[#859]:Allow SseEventSink.close() to throw IOException

[jimma]

No description provided.

[andymc12]

LGTM - thanks @jimma!

[andymc12]

Ack! I spoke too soon. There is an error - this breaks one of the examples. CI output:

[INFO] -------------------------------------------------------------
615[ERROR] COMPILATION ERROR : 
616[INFO] -------------------------------------------------------------
617[ERROR] /home/travis/build/eclipse-ee4j/jaxrs-api/examples/src/main/java/jaxrs/examples/sse/ServerSentEventsResource.java:[99,32] unreported exception java.io.IOException; must be caught or declared to be thrown
618[INFO] 1 error
619[INFO] -------------------------------------------------------------

Can you also change this example and any unit test cases that may not be handling the IOException?

[jimma]

@andymc12 Done. Thanks Andy.

[andymc12]

Looks good, thanks again for the changes!

[jansupol]

Please see my comment.

[ronsigal]

We had a longish discussion related to this proposal. On one side, someone suggested that calling close() should put an end to any further activity. My concern, on the other side, was that a failure to flush the stream upon closing was at least worth noting, whatever you choose to do about it. So I vote +1.

But I think that @jansupol is right that this is a breaking change. So, +1 for merging to 4..0.

[chkal]

I agree that the change should be done in a major version. Unfortunately we don't have a branch for 4.0 yet. And I don't think that we should create one now. So maybe we should park this one for a while?

[mkarg]

I added this PR to the milestone, so at least we don't forget it. :-)

[spericas]

@jimma Please re-target this PR to branch 4.0-SNAPSHOT

[mkarg]

@jimma Can you please fix the conflicts please? Thanks!

The base branch was changed.

[jimma]

@mkarg Done. Thanks.

[mkarg]

I personally would rather go with UncheckedIOException instead of IOException, so the caller still can catch if he likes, but is not forced to catch-or-throw.

[chkal]

I prefer IOException over UncheckedIOException TBH. I guess in most cases, users will catch and ignore (and maybe log) it. And with UncheckedIOException it is too easy to miss catching it.

[spericas]

This PR should include updates to the TCKs now (in our workspace). They won't compile with the addition of the new exception. Elsewhere our CI pipeline needs to be updated as well.

[jimma]

This PR should include updates to the TCKs now (in our workspace). They won't compile with the addition of the new exception. Elsewhere our CI pipeline needs to be updated as well.

Fixed the tck. Thanks for the review @spericas

[asoldano]

The TCK changes are here: 23f86b3 can these be merged now?

[spericas]

The TCK changes are here: 23f86b3 can these be merged now?

Thanks for the TCK changes