Out of curiosity, have any of you experienced browsers not closing SSE connections when navigating away from pages (or closing the browser)? My :on-client-disconnect callback in the options to start-stream is never triggered, and my code can keeps sending to the channels.
After doing some more digging, even closing the event source object in the client does not trigger cleaning up of the sse channel. Next up is to see if this is also the case with the Jetty connector.
Okay, with Jetty the channel is closed as expected, but the EofException that is caught because of the disconnect is printed to the logs.
Making the test case is proving very difficult. I have made an interceptor wheee I can supply a disconnect channel from the outside, but it seems like regardless what I do the disconnect always happens after my test ends; and if I wait for it to happen it never does.
Testing with Http-Kit’s own as-channel, and it seems the connection is just never closed by Http-Kit. Is that expected?
After spending some time on this, I think my conclusion is that Httk-Kit does not properly instruct the client to close the connection when it is done. Although should it, given HTTP2 using a single multiplexed connection per domain? Hm…
This is with Pedestal 0.8 and http-kit. I’m wondering if this is a browser issue. I see it in Chrome and Firefox, haven’t tried others.
You're welcome 🙂 And thanks for all the work you are doing on Pedestal! gratitude
When I get time again I want to see if I can add an SSE end-to-end test case in Pedestal that tests the client disconnect behaviour. I'm not sure how to force a Java HttpClient to disconnect yet, if the close method actually does that.
I have tried to make a test case that shows the problem, but I haven’t been able to force a disconnect with the java.net.HttpClient. Calling .shutdownNow has no effect, so the approach looks like it has to be different. I will let it rest for now, hoping that I find a solution when not actively looking for it. hammock
And it worked! I stumbled upon an SSE client in Clojure using the same building blocks as the one in the Pedestal tests (https://github.com/cjohansen/clj-event-source/), which led me to see that the Pedestal test client would always pull all messages immediately. After making it pull one message at a time, I can now consistently make it fail by sending the shutdownNow message to the HttpClient.
Next up is actually making a test case that shows the problem, and fixing the existing case that broke because of my change to the SSE client.
Thanks for testing this, I'm currently travelling and not available to assist.
@hlship When you are back from travelling I could use some assistance on this. I have forked Pedestal to work on the test (see the diff of my changes here: https://github.com/pedestal/pedestal/compare/master...mdiin:pedestal:master), and am running into problems with the thread synchronization.
What I have discovered so far:
• Jetty correctly calls the on-client-disconnect callback and delivers the promise
• Http-Kit does not call the on-client-disconnect
While the promise is correctly delivered in the Jetty test, it is only delivered once the test is no longer in scope, i.e. after the deref timeout.