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

After upgrading to 1.18 event :chsk/uidport-close desn't work #430

Closed khmelevskii closed 1 year ago

ptaoussanis commented 1 year ago

@khmelevskii Hi Yuri, can you please explain in some more detail what you mean by "doesn't work"?

Is the behaviour reproducible with the reference example?

khmelevskii commented 1 year ago

@ptaoussanis this example dosn't use event :chsk/uidport-close.

So, I've just recognized that after upgrading to 1.18 my handler :chsk/uidport-close doesn't trigger when user close connection. The same code works well in 1.17

I'm doing somethings like this

[taoensso.sente.server-adapters.http-kit :refer [get-sch-adapter]]
...

(defonce ^:private ws-channel
  (sente/make-channel-socket-server!
   (get-sch-adapter)
   {:packer        :edn
    :csrf-token-fn nil}))

(def ^:private ws-handshake
  (:ajax-get-or-ws-handshake-fn ws-channel))

(def ^:private ws-send!
  (:send-fn ws-channel))

(def ^:private ws-recv
  (:ch-recv ws-channel))

(def ^:private connected-uids
  (:connected-uids ws-channel))

(defmulti -event-msg-handler
  "Multimethod to handle Sente `event-msg`s
   Dispatch on event-id"
  :id)

(defmethod -event-msg-handler
  :default ; Default/fallback case (no other matching handler)
  [_]
  nil)

(defmethod -event-msg-handler :chsk/uidport-open
  [_]
  (println "Works"))

(defmethod -event-msg-handler :chsk/uidport-close
  [_]
  (println "Not triggering"))

(defn event-msg-handler
  "Wraps `-event-msg-handler` with logging, error catching, etc."
  [ev-msg]
  (-event-msg-handler ev-msg))

(sente/start-server-chsk-router!
   ws-recv event-msg-handler)

Checked migration instruction and can't see any related to this issue things.

khmelevskii commented 1 year ago

When I enabled :debug log level I can see that server closed ws

DEBUG [taoensso.sente:896] - [ws/on-close] Server sch closed for 

But method :chsk/uidport-close didn't trigger

ptaoussanis commented 1 year ago

Thanks for the context 👍

this example dosn't use event :chsk/uidport-close.

The reference example logs every event, here.

Am investigating right now, will come back to you 👍

ptaoussanis commented 1 year ago

@khmelevskii Can confirm that this was an unintended regression, apologies for the trouble!

Should be fixed with [com.taoensso/sente "1.18.1"], now on Clojars 👍

Thanks for the report 🙏

khmelevskii commented 1 year ago

Hi Peter, I've just checked 1.8.1 and everything works great! Thank you for a quick fix and great library.

ptaoussanis commented 1 year ago

You're very welcome Yuri, cheers! :-)