cleaning up after a websocket goes aways is not clear

[hiredman]

the spec says on-close is guaranteed to be called and can be used for finalization logic.

if you run a little ring-jetty server like:

;; clj -Sdeps '{:deps {ring/ring {:mvn/version "1.12.1"}}}'

(require '[ring.adapter.jetty :refer [run-jetty]]
         '[ring.websocket :as ws]
         '[ring.websocket.protocols :as wsp])

;; patch in logging to see that upgrade-to-websocket is called
(alter-var-root #'ring.adapter.jetty/upgrade-to-websocket
                 (fn [orig]
                   (fn [& args]
                     (prn 'upgrade-to-websocket 'called)
                     (apply orig args))))

(def jetty
  (run-jetty
   (fn [req]
     (prn req)
     {::ws/listener
      (reify wsp/Listener
        (on-open [_ sock]
          (println "on open")
          (ws/send sock "Hello")
          (ws/send sock (java.nio.ByteBuffer/wrap (.getBytes "World"))))
        (on-message [_ sock msg]
          (prn "on close"))
        (on-pong [_ _ _]
          (prn "on pong"))
        (on-error [_ _ _]
          (prn "on error"))
        (on-close [_ _ _ _]
          (prn "on close")))})
   {:port 4395
    :join? false}))

and then hit it with curl like curl -i -N -H "Connection: Upgrade" -H "Upgrade: websocket" http://localhost:4395 you get some interesting behavior:

1. no valid websocket connection is established of course
2. on-open is never called
3. on-close is never called
4. on-error is never called

With some thought you can come to the conclusion that the fact that on-open was never called is the signal to the handler that the websocket is not valid, but that is not at all clear from the spec, and the spec for on-close doesn't say it is guaranteed to be called if on-open is called, it just says it is guaranteed to be called.

Additionally the jetty websocket impl just forwards the jetty WebSocketListener onClose events to the handler's on-close function, and there is nothing in the docs for WebSocketListener that says onClose is guaranteed to be called. I don't have a reproducer at the moment, but I believer it should be relatively straightforward to replicate, and that it happens a lot in practice, that sometimes when the underlying tcp connection is closed, onClose is never called and there are no exceptions.

[hiredman]

I think I was too hasty in assuming just forwarding jetty's onClose wouldn't result in it always getting called, but hard to tell, I haven't found docs that say it is always called, but I found a stackoverflow post that suggested it was (but now have lost the link to that post)

[weavejester]

Interesting. I hadn't considered how Jetty would behave under those circumstances. Thank for for bringing it to my attention.

My instinct is to adjust the SPEC and say that on-close is guaranteed to be called if and only if on-open is called, as if the WebSocket is not correctly established and never "opened", it makes no sense for it to then be "closed". This seems the most correct behavior, at least to my mind.

With regard to the Jetty documentation on WebSocketConnectionListener, it says that onWebSocketClose is called when "A Close Event was received." Given that the close status includes events where the WebSocket was closed abnormally - i.e. the TCP connection is terminated without a valid close frame - this implies that once open, a close event is guaranteed. However, I can confirm this on the Jetty mailing list to be sure.

[weavejester]

The Jetty users mailing list confirms that on-close is guaranteed to be called if on-open is: https://www.eclipse.org/lists/jetty-users/msg10694.html

[hiredman]

interesting, definitely not at all clear to me that "A Close Event was received" in the docs encompasses abnormally termination, but if the jetty mailing list says it is so it must be so. And I didn't realize there was specific reason code to synthesize an onClose call even if no close frame with a reason code came.