jarohen / chord

A library designed to bridge the gap between the triad of CLJ/CLJS, web-sockets and core.async.
439 stars 40 forks source link

Adding on-client-close callback option. #35

Closed antoniogarrote closed 9 years ago

antoniogarrote commented 9 years ago

Small change that adds a callback that will be invoked when the client closes the connection from the browser.

My use case is to be able to do some kind of resource cleaning for the closed ws connection if required.

As far as I know, core.async does not provide a way of checking if the channel has been closed and it is recommended for the producer to be the one closing the channel, but for this use case, in an unreliable web connection, the callback can be useful.

jarohen commented 9 years ago

Hi Antonio - thanks for the PR :)

Actually - core.async does provide a way of checking if the channel has been closed - a take on a channel will return nil iff the channel is closed. I usually use something along these lines to perform any cleanup:

(:require [clojure.core.async :as a])

(defn process-messages! [ch]
  (a/go-loop []
    (if-let [msg (a/<! ch)]
      (do
        (process-message! msg)
        (recur))

      (clean-up! ch))))

Would this work for your use case?

Regarding whose responsibility it is to close a channel - Chord's channels are actually bi-directional. Unlike a conventional core.async channel, where a 'take' reads the messages that have been put onto that channel, Chord in fact sets up a pair of channels, where a take on one reads messages that have been put onto the other, and vice versa. This extends to closing the channel, too - if the server closes the channel, the client can watch for that close by checking for nil; likewise, if the client closes their channel, the server can watch for that by checking for nil. Not sure if this makes sense - let me know if it's unclear!

How's your holiday, btw? :)

James

antoniogarrote commented 9 years ago

Hi James, thanks for your reply!

I have a use case where I think I could really make use of the callback.

I'm pushing events from a server-side event source into the client. For each opened websocket there is a handler receiving server side notifications and writing the events into the websocket.

With the current API I need to add the conditional logic and in the case of a closed connection, find a way of re-enqueuing the event in the source and then close the handler. This can be hard depending on the way events are processed, for example if you need to keep the FIFO processing order of the events.

In this case, closing the handler in the very moment the websocket is closed would make things really easy: no conditional logic, no event to re-enqueue in the right order.

I understand it is kind of an edge case and the callback breaks the core.async API design. You can always fall-back to httpkit channels if you need it, so please feel free to ignore the pull-request.

Thanks a lot!

On Apr 11, 2015, at 8:07 AM, James Henderson notifications@github.com wrote:

Hi Antonio - thanks for the PR :)

Actually - core.async does provide a way of checking if the channel has been closed - a take on a channel will return nil iff the channel is closed. I usually use something along these lines to perform any cleanup:

(:require [clojure.core.async :as a])

(defn process-messages! [ch] (a/go-loop [] (if-let [msg (a/<! ch)](do %28process-message! msg%29 %28recur%29)

  (clean-up! ch))))

Would this work for your use case?

Regarding whose responsibility it is to close a channel - Chord's channels are actually bi-directional. Unlike a conventional core.async channel, where a 'take' reads the messages that have been put onto that channel, Chord in fact sets up a pair of channels, where a take on one reads messages that have been put onto the other, and vice versa. This extends to closing the channel, too - if the server closes the channel, the client can watch for that close by checking for nil; likewise, if the client closes their channel, the server can watch for that by checking for nil. Not sure if this makes sense - let me know if it's unclear!

How's your holiday, btw? :)

James

— Reply to this email directly or view it on GitHub.

jarohen commented 9 years ago

Hi Antonio - great to pair up the other day :)

What do you think's the best thing to do with this PR now, btw?

James

antoniogarrote commented 9 years ago

Probably, ignore it.

I think you showed me that if you are looking for this callback you are not writing your code in the core.async style and your best chance is to have another look at how to rewrite it using core.async primitives.

Maybe a mention about how to detect the closing of the websocket from the client in the readme might be useful for some users, I have the impression you are going to see this question in the future.

Thanks a lot.

On Thu, Apr 16, 2015 at 8:39 AM, James Henderson notifications@github.com wrote:

Hi Antonio - great to pair up the other day :)

What do you think's the best thing to do with this PR now, btw?

James

— Reply to this email directly or view it on GitHub https://github.com/james-henderson/chord/pull/35#issuecomment-93667257.