Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
khmelevskii opened this issue Jul 4, 2023 · 7 comments
Closed

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

khmelevskii opened this issue Jul 4, 2023 · 7 comments
Assignees
Labels

Comments

@khmelevskii
Copy link

No description provided.

@ptaoussanis
Copy link
Member

@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
Copy link
Author

@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.

@ptaoussanis ptaoussanis self-assigned this Jul 4, 2023
@khmelevskii
Copy link
Author

khmelevskii commented Jul 4, 2023

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
Copy link
Member

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 added a commit that referenced this issue Jul 4, 2023
Faulty ownership check:

  The client connection `server-ch` entry in `conns_` will already
  have been set to nil by the earlier `upd-conn!` call above.
@ptaoussanis ptaoussanis added the bug label Jul 4, 2023
@ptaoussanis
Copy link
Member

@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
Copy link
Author

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

@ptaoussanis
Copy link
Member

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants