ping event ids: server-side vs client-side

[deridex]

I found Sente via pragprog's Web Development with Clojure (2ed). Great library! While updating the book project from sente-1.8.0 to 1.12.0, I hit a situation that lead to a question about pings.

Updating the version broke the book's project client-side event handling because of pings. Based on the doc, I expected the ping events to have :id :chsk/ws-ping:

On the server side, I see this is so:

But on the client side, the events have :id :chsk/recv, instead. And deeper inside the event map, I see :chsk/ws-ping:

Am I doing something wrong that would make the ping events have :id :chsk/recv instead of :id :chsk/ws-ping? Right now, the clojurescript :chsk/recv handling includes logic to identify and ignore ping events.

[theasp]

It is normal for :chsk/ping to come inside a :chsk/recv event. I'm not sure the reason behind this, but I agree the docs imply that you should be getting it directly rather than wrapped in a :chsk/recv.

I deal with it like this:

(defrecord SenteClientHandler [send! state api handler]                                                                                                        
  ISenteClientHandler                                                                                                                                          
  (unhandled-event [this {:keys [event] :as ev-msg}]
    (warnf "Unhandled event: %s" event))                                                                                                                                                               

  (chsk-handshake-event [this {:keys [send-fn] :as ev-msg}]
    (debugf "Handshake"))

  (chsk-state-event [this {:keys [?data] :as ev-msg}]
    (let [[old new] ?data]
      (debugf "Channel socket state change: %s" new)))

  (chsk-recv-event [this {:keys [?data] :as ev-msg}]
    (let [[cmd data] ?data]
      (condp = cmd
        :chsk/ws-ping nil
        :my/event    (handle-my-event this data)  
        (warnf "Unhandled push event from server: %s" ?data))))

  (event-msg-handler [this ev-msg]
    #_(debugf "Sente event: %s" ev-msg)
    (condp = (:id ev-msg)
      :chsk/handshake (chsk-handshake-event this ev-msg) 
      :chsk/state     (chsk-state-event this ev-msg)
      :chsk/recv      (chsk-recv-event this ev-msg)
      (unhandled-event this ev-msg)))                                                                                                                          

  component/Lifecycle                                                                                                                                          
  (start [this]
    (infof "Starting")
    (assoc this :handler (partial event-msg-handler this)))
                                                                                                                                                               
  (stop [this]
    (infof "Stopping")
    (dissoc this :handler)))

[deridex]

It is normal for :chsk/ping to come inside a :chsk/recv event.

Oh, ok. I'm glad to learn that it's not just me.

I agree the docs imply that you should be getting it directly rather than wrapped in a :chsk/recv.

So should this github issue result in a change to the documentation? Or a change to the implementation (which would seem to be a breaking change)?

[jjttjj]

Also note that the :chsk/recv wrapping can be disabled by setting wrap-recv-evs? to false in the options map passed to make-channel-socket! on the client.

Here is the relevant lines in the source:
https://github.com/ptaoussanis/sente/blob/master/src/taoensso/sente.cljc#L1391

Here is an earlier issue that discusses this:
#151

And slightly more info in comments in the commit that added this than currently exists in the code now:
64463fa#diff-2148a5a3305b14a6f9087b63ecb8bae4R1049

[SVMBrown]

Is ping the only standard event that will get wrapped in recv?
Also, will ping always be received in the wrapped form (if wrap-recv-evs? is true)?

[SVMBrown]

Also, out of curiosity, why is wrap-recv-evs? enabled by default? I can't see a reason for this difference in behaviour between client and server personally. What is the rationale behind this?

[ptaoussanis]

@SVMBrown Your questions are addressed by the comment above by @jjttjj (thanks Justin!).

Default val is currently true for backwards compatibility. Default will be changed to false in a future breaking release.
In the meantime, folks can change the behaviour manually.

Leaving this issue open for now as a reminder to change the default in a future release.

[ptaoussanis]

Closing since the default will be changed in forthcoming v1.18 👍