[RESTEASY-2985] Only run the complete listeners if a 204 response is …

[jamezp]

…received. Note that an SseEventSink.close() in RESTEasy will result in a 204. In other implementations, this results in a 200. Add a way to override this with a system property or configuration parameter. See jakartaee/rest#1007 for details.

https://issues.redhat.com/browse/RESTEASY-2985

This potentially replaces #3554. However, there is an attempt to not full undo #1361.

@radcortez Would this work for you as a replacement for #3722?

Note the TCK will fail on ee.jakarta.tck.ws.rs.jaxrs21.ee.sse.sseeventsource.JAXRSClientIT.connectionLostFor1500msTest. Locally I've got a simple change to fix this, but I'd like to get an approval on this approach before I send a PR up to fix that test.

[radcortez]

Sure, I'm OK with the approach. I was trying to pass the TCK. Can you share the fix with me so I can also try it? Are you fixing the TCK directly?

[jamezp]

Sure, I'm OK with the approach. I was trying to pass the TCK. Can you share the fix with me so I can also try it? Are you fixing the TCK directly?

Oops, sorry yes. The fix will just be setting the system property dev.resteasy.sse.closed.response.code to 200. For a Wildfly run that just means adding -Ddev.resteasy.sse.closed.response.code=200 in the arquillian.xml file.

[radcortez]

Ah, I should have checked the code. Thanks!

Unfortunately, even with the switch, the TCK fails :(

[jamezp]

Ah, I should have checked the code. Thanks!

Unfortunately, even with the switch, the TCK fails :(

Okay. I was just hoping it would work. The change in #3722 is definitely different. I can look into that issue a bit more as well.

[radcortez]

Yes, the issue here is that with Vert.x the response gets closed way sooner, so when the status is updated, you don't see it in the response.

If you want to try it with minimal effort, you can run: https://github.com/quarkiverse/quarkus-microprofile/tree/main/tck/rest

And: mvn -pl tck/rest -Dtest=ee.jakarta.tck.ws.rs.jaxrs21.ee.sse.sseeventsource.JAXRSClientIT verify

You do need to update the RESTEasy dependencies to use the SNAPSHOT. I've used 6.2 branch and the commit applied cleanly.

[jamezp]

Perfect, thank you! I'll give that a try, possibly tomorrow.

[radcortez]

Is this now working with Quarkus? :)

[jamezp]

Is this now working with Quarkus? :)

I would guess not, but maybe. I think the Quarkus error was different with a race of some sort in Vert.x.

[PrimevalCoder]

@jamezp Would you please revisit the issue described here? I also provided steps on how to reproduce it. RESTEasy (6.2.6) never realizes that a sink was closed by the client, doesn't complete the SseEventSink's .send(e) CompletionStage exceptionally when trying to send an event to an already-closed sink, as the documentation seems to imply:

If there is a problem during sending of an event, completion stage will be completed exceptionally.

and the CompletionStage seems to never, ever complete in this situation where the sink was closed by the client.
As a side note, Quarkus' RESTEasy Reactive seems to behave correctly in this regard. I tried and bound an EventSource, then closed the browser tab that opened it, and at that point .isClosed() of the corresponding sink correctly returns true.
Also, it would be great to have some kind of event to know when a particular sink was closed.

[jamezp]

@PrimevalCoder Do you use an SseBroadcaster to add the sinks to?

[PrimevalCoder]

No, because my use case is similar to the one of the person who asked that StackOverflow question: I need to send specific events to specific client sinks, not broadcast to all of them. I know WebSockets might solve the problem easier, but our team decided to settle for SSE in this case. So we're keeping the sinks into a ConcurrentHashMap and send the events when needed. We receive data from another service and need to live notify users when some data they require is available.
Imagine you had a web application that displayed documents for a user on a page, and it receives the documents from another service; and you needed to notify the user when a document they need was received, without reloading the page or periodic polling: you can bind to SSEs and send the notification right away – this is more or less what we had to do. But since I couldn't find a solution to determine whether the sink is actually closed (by the client), I had to find some workarounds, at least until we switch to RESTEasy Reactive.

[jamezp]

So far from what I see when the EventSource.close() is being invoked, RESTEasy is not seeing the request at all. I need to track down why it's not seeing it. I'm testing with WildFly FWIW.

[PrimevalCoder]

Thanks a lot. Please try to find out why it doesn't realize the sink is closed when the browser/tab (that created the EventSource) is closed, either, (without necessarily calling .close() on the EventSource) and why it gets stuck when calling .send(e) on such a sink that is actually closed. So far, I tried:

CompletableFuture<?> cf = sink.send(sse.newEvent("alive?")).toCompletableFuture(); 
cf.get(5, TimeUnit.SECONDS);

and it results in a TimeoutException (completes normally and very fast with sinks that are actually open).
(We're using Quarkus 3.6 with RESTEasy Classic, for the moment)

[dedalusMohantyMa]

No, because my use case is similar to the one of the person who asked that StackOverflow question: I need to send specific events to specific client sinks, not broadcast to all of them. I know WebSockets might solve the problem easier, but our team decided to settle for SSE in this case. So we're keeping the sinks into a ConcurrentHashMap and send the events when needed. We receive data from another service and need to live notify users when some data they require is available. Imagine you had a web application that displayed documents for a user on a page, and it receives the documents from another service; and you needed to notify the user when a document they need was received, without reloading the page or periodic polling: you can bind to SSEs and send the notification right away – this is more or less what we had to do. But since I couldn't find a solution to determine whether the sink is actually closed (by the client), I had to find some workarounds, at least until we switch to RESTEasy Reactive.

I have a similar use-case and it seems that I also have some weird behavior.
Also not being able to use Reasteasy Reactive right away.

[jamezp]

I don't really have a good Quarkus reproducer, I'll have to see if I can find some time, but I've not been able to reproduce this in WildFly or with the SeBootstrap.

[PrimevalCoder]

No, because my use case is similar to the one of the person who asked that StackOverflow question: I need to send specific events to specific client sinks, not broadcast to all of them. I know WebSockets might solve the problem easier, but our team decided to settle for SSE in this case. So we're keeping the sinks into a ConcurrentHashMap and send the events when needed. We receive data from another service and need to live notify users when some data they require is available. Imagine you had a web application that displayed documents for a user on a page, and it receives the documents from another service; and you needed to notify the user when a document they need was received, without reloading the page or periodic polling: you can bind to SSEs and send the notification right away – this is more or less what we had to do. But since I couldn't find a solution to determine whether the sink is actually closed (by the client), I had to find some workarounds, at least until we switch to RESTEasy Reactive.

I have a similar use-case and it seems that I also have some weird behavior. Also not being able to use Reasteasy Reactive right away.

On top of the RESTEasy bug, the specification is quite silly by not mandating a callback for the SSE connection closure event (e.g. Node.js has this out-of-the-box), so if you have a use case where you don't close the sink explicitly on the server side after performing some operations, you could probably save a lot of headache by using websockets instead, where you have the @OnClose listener. These seem to work correctly on RESTEasy Classic too.
PS: Another thing I noticed with our server using RESTEasy Classic is that when calling sink.close() the corresponding front-end EventSource interprets that message as a failure and fires the onerror handler instead of closing the connection. Probably something's not in line with the HTML SSE spec.

[dedalusMohantyMa]

I am sadly bound to resteasy v4.7.9.Final by company restrictions. There is work being done to migrate to Quarkus v3.x but it will not happen before autumn.

What I am doing now is implementing an endpoint to only close sinks on the backend.
And in addition using @Timeout(value = 5, unit = ChronoUnit.SECONDS) and @Fallback(fallbackMethod = "closeSink") annotated on my sending method, where closeSink is a callback method, to simply close the sink, if there are any exceptions thrown.

Thanks for the insight @PrimevalCoder

[PrimevalCoder]

Good to hear you found a solution that works for you. In our situation, connections have to remain open for very long periods of time, even hours, while the server waits for data from another service, so unfortunately we couldn't go for an algorithm where we perform some operations synchronously, and close the sink at the end; we had to store the sink objects and send the events to each when that other service sends us the data.

[jamezp]

From what I understand there is nothing in the SSE Spec that indicates when an EventSource client is closed, that a message is sent to the server to indicate the client is closed. I do need to test this again, but I don't recall seeing a request going to the server if the client closes say the browser. In that case RESTEasy doesn't really know it's closed until another request is made and an IOException is thrown.

If anyone has a reproducer, I'd be happy to have a look though. When I created one, I was not seeing any issues. However, I was also using WildFly not Quarkus.

[PrimevalCoder]

I left some instructions on how to reproduce it in my first replies here. I will try to provide a complete Quarkus project maybe during the weekend. But in any case, it would be normal to at least have .isClosed() working correctly (i.e., return true, which does not happen) after either the EventSource is closed explicitly, or the browser is closed. With Quarkus' RESTEasy Reactive this works as expected.
PS: my observation above regarding the HTML spec is that when one calls .close() on the SseEventSink, it fires the error handler of the EventSource instead of making it close the connection. The interface seems to be covered by step 15 in the section 9.2.2 of the spec I linked. This I haven't tried on RESTEasy Reactive to see how it behaves.

[PrimevalCoder]

Hi again, @jamezp. I made this demo for you to see what I mean regarding the isClosed() behavior. If you want, I can push the code for both the server and front-end to a repo to share it with you. The Quarkus server here uses RESTEasy Classic, with RESTEasy Reactive this works fine. Sorry for the delay, I've been quite busy.

resteasy.bug.demo.conv.mp4

[jamezp]

Thank you @PrimevalCoder. A reproducer would be great, thank you! And no need to apologize, I completely understand :)

[PrimevalCoder]

Thanks for your time, too. I sent you an invite to the repo.

[jamezp]

@PrimevalCoder Here's what I see locally and possibly what I'd expect. If I connect a client and then close it, on the next request the sink.isClosed() will return true and the client will not show up. However, if I don't make another request the client appears to be open. This makes sense to me because there is no communication when the close is done on the client side to tell the server the connection has been closed. I'm not as familiar as I should be with Quarkus RESTEasy so I'd need to look at why it behaves differently there.