rest icon indicating copy to clipboard operation
rest copied to clipboard

Allow SseEventSink.close() to throw IOException

Open jimma opened this issue 5 years ago • 5 comments

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.

jimma avatar Apr 16 '20 02:04 jimma

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!

andymc12 avatar Apr 17 '20 22:04 andymc12

@andymc12 PR is sent : https://github.com/eclipse-ee4j/jaxrs-api/pull/863. Please review, Andy. Thanks.

jimma avatar Apr 20 '20 03:04 jimma

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 avatar May 25 '20 19:05 jansupol

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

jansupol avatar May 25 '20 21:05 jansupol

it is a backward-incompatible change

Agree.

ronsigal avatar May 27 '20 18:05 ronsigal