Clojurians Log v2
ring
Hi, I am working with Server Sent Events and using ring-jetty. I have a question about the way disconnections from the client are handled/detected. Right now if I keep the HTTP connection around (meaning I hold onto the OutputStream handed to me in the write-body-to-stream ) and the client disconnects I need to send 2 small events to get an IOException with a Broken pipe message. Note that I flush the OuputStream every event. It seems like the first event stays stuck in jetty's buffer until I try to send another one. Only then jetty tells me the connection is dead by throwing. I can get the exception on the first small event if I specify :output-buffer-size to a small number or if I send a big event. I believe touching jetty's buffer isn't a good idea since I imagine it would means having a small buffers for all HTTP responses... Is there a way to get the broken pipe exception on the first event regardless of the size without touching jetty's buffer size?
Could you explain what you mean by holding onto the OutputStream? Also, are you operating in synchronous or asynchronous mode?
I use asynchronous mode. By holding onto the output stream I mean keeping the output stream in an atom for later use, pushing events form the sever when they arrive. For some context I am maintaining the Clojure SDK for Datastar which relies on SSE. You helped me there a while back btw!
I thought I recognized the name! Thanks for clarifying. It makes sense that this would be caused by some sort of buffering on Jetty's behalf, though it seems incorrect behavior - you'd expect Jetty to throw an error for any new data if the connection was closed. The third-party rj9a adapter uses Jetty 12. Would it be possible to check if this bug exists when using that? I ask because Jetty 11 is deprecated now (except for security updates), and it's just a matter of me finding the time to upgrade Ring's adapter.
It should be a drop-in replacement. The project site is: https://github.com/sunng87/ring-jetty9-adapter
Well I can't try with rj9a because the it closes the output stream when the initial server thread it done processing even in async mode. It's this issue https://github.com/sunng87/ring-jetty9-adapter/issues/122
Oh, well that's no good.
I tested with pedestal 7.2 however and the behavior is the same, on jetty 12 I believe. Pedestal uses a heartbeat under the hood which I'd like to avoid if possible.
So upgrading Ring to 12 likely won't fix the underlying issue.
Although with pedestal it's worse since I detect a closed connection in 3 events, the first stays stuck, the second throws and the core.async channel gets closed and only then on the third event I get a closed channel and now the connection is broken...
Yes maybe there is a way to force jetty to flush and throw on the first event but I don't know how...
I've had a quick poke around, but I haven't found anything in Jetty's issues, or a way of definitively testing whether a client has disconnected via the Servlet API.
Thanks for taking the time,
Presumably you're testing a graceful TCP disconnection, not a sudden loss of connection.
I guess you must be otherwise there'd be a timeout involved.
Ideally yes, http-kit does it on its own but since it's custom and eschew the servlet api, that's not a valid comparison
nor fair
My suggestion would be to open an issue on the Jetty issues, but they may want a Java example so that they can confirm the bug. That sounds like something of a rabbithole, though.
There is an isClosed method on the HttpOutput class that Jetty uses, though. You could try checking to see if the OutputStream for the Jetty adapter responds to the isClosed method.
If it does, I wonder if that would give the correct answer regarding the OutputStream. Obviously it's not a very portable or future-proof solution, but it might be able to be used as part of a workaround in the Jetty adapter.
Yes I try that, basic java OutputSteam doesn't have this method I believe.
I'll check it out!
It might also be being proxied by Ring in the async case (https://github.com/ring-clojure/ring/blob/dde9716e23f85497df4ea0deba148fd22ac99c6c/ring-jakarta-servlet/src/ring/util/jakarta/servlet.clj#L72), which may prevent direct access.
Possibly you might try it with a synchronous handler first. Though I'd imagine that if writes are detecting the closed socket immediately, then I don't know if the isClosed method would.
the thing that bugs me is the buffering behavior. Tiny buffer or big event results in an execption first try. It is like jetty is saying "this event isn't big, I'll wait a bit to actually declare the connection is lost"
The HttpOutput class appears to only set the closed state after a write has been completed (https://github.com/jetty/jetty.project/blob/24e5ec7ed04508c070f1fdb15898ecdbdf5a4b27/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/HttpOutput.java#L218). It's weird that flush doesn't work.
It doesn't work only in this case. If the connection is alive I can flush tiny events without them getting stuck in jetty's buffer.
It does sound like a bug with Jetty.
It does.
I'll investigate a bit further later. Maybe I'll try to get a repro working in java at some point and open an issue on the jetty repo. Thank you 🙏
No problem. Sorry I couldn't help further.
No problem, now I know I didn't miss something obvious in the docs!
I just implemented SSE events with rj9a, so I'm an expert now, right? 😏🙄
The thing I found didn't work was if the body of write-body-to-stream returned immediately. So I couldn't have that body wrapped in go or future or something like that. I'm curious about that incident #122 because the body there is wrapped in a future, so I wouldn't think it would work at all.
@jeremys what does your implementation of write-body-to-stream look like?
Oh, one other thing. I implemented a heartbeat that sends ":heartbeat\n\n" (which is treated like a comment on the client side) and my connections are closing just fine when the next heartbeat comes along after client disconnect. And if I need to shut things down server-side, I close the channel that is feeding the output stream, and things shut down nicely.
As long as write-body-to-stream doesn't return the connection stays open. When it does return there are 2 scenarios according to the spec. If you run the server with :async? false the OutputStream will be closed. If you run with :async? true the server should leave it to you to close it. As far as I understand rj9a closes in both cases. It means to prevent the adapter closing the connection you need to do what you do, that is blocking in the write-body-to-stream function and not returning. It may be a valid way to do this if you use virtual thread. If not please don't I believe you'll end up blocking the whole thread pool. A heartbeat is a way to "force jetty" to take the closed connection into account. It works but at some point I'd like to figure out a way where I don't have to have one.
Just to chime in and say that's all correct.
In synchronous mode, the response is "complete" when the handler function returns. In asynchronous mode, it may not be.
Yes this makes sense
Also my specific implementation is there https://github.com/starfederation/datastar/blob/develop/sdk/clojure/adapter-ring/src/main/starfederation/datastar/clojure/adapter/ring/impl.clj You can check out the whole project here https://github.com/starfederation/datastar/tree/develop/sdk/clojure Not to plug my thing here but if you are into html over the wire Datastar is really nice.