Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add a onClose() method to the client to indicate the connection has been closed #1283

Open
jamezp opened this issue Sep 9, 2024 · 5 comments
Labels
enhancement New feature or request spec

Comments

@jamezp
Copy link
Contributor

jamezp commented Sep 9, 2024

It would be nice for users, and implementations, to have something like an onClose() method on the Client API. It could look something like:

CompletionStage<Void> onClose();

The CompletionStage would be completed when the close() method is invoked. While in most cases the user is in control of when the Client gets closed, there are cases like CDI where the client may be injected and the user wants to know then the client has been closed.

Implementations could also use this if the connection has been reset and/or a reconnect is not possible. In this case the CompletionStage can end exceptionally.

@jamezp jamezp added enhancement New feature or request spec labels Sep 9, 2024
@jamezp
Copy link
Contributor Author

jamezp commented Sep 9, 2024

If there is interest, this is something I can look at adding/implementing in RESTEasy. I'll be on PTO for starting Thursday through the following week, but I can likely look in late September or early October.

@spericas
Copy link
Contributor

spericas commented Sep 16, 2024

Closing a connection is an event like many others. Would it make sense to consider the addition of a more generic event listener instead?

@NicoNes
Copy link
Contributor

NicoNes commented Sep 16, 2024

I agree with @jamezp that adding a method to indicate the connection has been closed will be a nice feature.
Maybe the method to do so should be more like : SseBroadcaster.onClose(Consumer<SseEventSink> onClose) (https://github.com/jakartaee/rest/blob/main/jaxrs-api/src/main/java/jakarta/ws/rs/sse/SseBroadcaster.java#L62).

@spericas, is that what you have in mind when you said "more generic event listener instead" ?

-- Nicolas

@spericas
Copy link
Contributor

@NicoNes Not necessary, my point is that if we identify other useful events to listen for, these should all be grouped in a proper listener instead of creating a method for each one. I'm also not a big fan of using CompletionStage for this type of event reporting.

@jamezp
Copy link
Contributor Author

jamezp commented Sep 16, 2024

@spericas I would be fine with some kind of event listener too. The idea with a CompletionStage is just the API was already there.

Did the top of my head the events I could see are:

  • Connected
  • Closed
  • Connection Failed
  • Reconnecting (possibly)

Closed and Connection Failed seem like the most important though which is why I thought of a CompletionStage.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request spec
Projects
None yet
Development

No branches or pull requests

3 participants