search

taoensso / sente

Realtime web comms library for Clojure/Script
https://www.taoensso.com/sente
Eclipse Public License 1.0
1.73k stars 193 forks source link

Update Jetty9 server adapter to ring-jetty9-adapter's new websocket fix #408

Closed surferHalo closed 1 year ago

surferHalo commented 1 year ago

Currently, ring-Jetty9-adapter does not regard websocket as a separate ring handler. https://github.com/sunng87/ring-jetty9-adapter/pull/59

ptaoussanis commented 1 year ago

@surferHalo Hi Jian, thank you for your PR - and sorry for the long delay replying on this!

In your PR, you have the following code:

(defn- server-ch-resp
  [websocket? {:keys [on-open on-close on-msg on-error]}]
  (jetty/ws-upgrade-response
   {:on-connect (fn [sch]          (on-open  sch websocket?))
    :on-text    (fn [sch msg]      (on-msg   sch websocket? msg))
    :on-error   (fn [sch error]    (on-error sch websocket? error))
    :on-close   (fn [sch status _] (on-close sch websocket? status))}))

I'm not familiar with this Jetty9 adapter, but it looks strange to me that the server-ch-resp return value will use jetty/ws-upgrade-response even if the websocket? argument is false.

Is this really correct? Ajax will work correctly even when websocket? is false?

Also, you mention in your commit message:

Currently, I can't find a way to response a {:body sch} inside ring-req->server-ch-resp.

I don't understand. Can you explain a bit more why you wrote this?

Thanks!

ptaoussanis commented 1 year ago

Closing to consolidate efforts at https://github.com/ptaoussanis/sente/issues/424