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.

Sign up for GitHub

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

Jump to bottom

Allow SseEventSink.close() to throw IOException #859

Open
jimma opened this issue Apr 16, 2020 · 5 comments
Open

Allow SseEventSink.close() to throw IOException #859

jimma opened this issue Apr 16, 2020 · 5 comments

Comments

@jimma
Copy link
Contributor

jimma commented Apr 16, 2020

Is it better allowing SseEventSink.close() to throw IOException, then the underlying IOException doesn't need to be wrapped in RuntimeException to throw up ? The caller can decide what to do when catch this IOException.
@andymc12
Copy link
Contributor

Hi @jimma I think this makes sense. Would you code up a pull request with this change?

Please code it against the 3.1-SNAPSHOT branch as we cannot change APIs in the master (3.0) branch.

Thanks!

jimma added a commit to jimma/jaxrs-api that referenced this issue Apr 20, 2020
@jimma
[jakartaee#859]:Allow SseEventSink.close() to throw IOException
d87a0df
@jimma
Copy link
Contributor Author

jimma commented Apr 20, 2020

@andymc12 PR is sent : #863. Please review, Andy. Thanks.

@jimma jimma mentioned this issue Apr 20, 2020
Merged
jimma pushed a commit to jimma/jaxrs-api that referenced this issue Apr 22, 2020
@maerqiang
[jakartaee#859]:Allow SseEventSink.close() to throw IOException
72999c5
jimma added a commit to jimma/jaxrs-api that referenced this issue Apr 22, 2020
@jimma
[jakartaee#859]:Allow SseEventSink.close() to throw IOException
b8de0d5
jimma added a commit to jimma/jaxrs-api that referenced this issue Apr 22, 2020
@jimma
[jakartaee#859]:Allow SseEventSink.close() to throw IOException
e9b84dc
jimma added a commit to jimma/jaxrs-api that referenced this issue Apr 22, 2020
@jimma
[jakartaee#859]:Allow SseEventSink.close() to throw IOException
c51066e
@jansupol
Copy link
Contributor

I do not have a strong opinion about this, but it would not align with other close methods such as on a Client and on a Response (that may close the entity input stream that throws IOException)

@jansupol
Copy link
Contributor

Anyway, it is a backward-incompatible change, and as such, it should go into a major version, not minor 3.1, imho.

@jansupol jansupol mentioned this issue May 25, 2020
Merged
@ronsigal
Copy link
Contributor

it is a backward-incompatible change

Agree.

jimma added a commit to jimma/jaxrs-api that referenced this issue Sep 7, 2022
@jimma
[jakartaee#859]:Allow SseEventSink.close() to throw IOException
ca898df
jimma added a commit to jimma/jaxrs-api that referenced this issue Sep 23, 2022
@jimma
[jakartaee#859]:Allow SseEventSink.close() to throw IOException
23f86b3
spericas pushed a commit that referenced this issue Oct 24, 2022
@jimma
[#859]:Allow SseEventSink.close() to throw IOException (#863)
57216fe
mkarg pushed a commit to mkarg/rest that referenced this issue Aug 15, 2023
@jimma @mkarg
[jakartaee#859]:Allow SseEventSink.close() to throw IOException (jaka…
5e51295 
…rtaee#863)
@jamezp jamezp mentioned this issue Feb 13, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
4 participants
@jimma @ronsigal @jansupol @andymc12