AsyncContext completion attempt after AsyncListener.onError is called #5675 #5773
Conversation
| LOGGER.log(Level.SEVERE, LocalizationMessages.ERROR_WRITING_RESPONSE_ENTITY(), ex); | ||
| if (ex.getCause() instanceof IOException) { | ||
| skipFinally = true; | ||
| LOGGER.log(Level.WARNING, "Response was sent, but there is IO issue", ex); |
There was a problem hiding this comment.
There was an issue after the response was committed.
You don't know if the response was sent or not. (it could be sitting in a buffer somewhere locally, not even sent yet)
You just know you cannot change the response anymore.
| * let's just log it. | ||
| */ | ||
| LOGGER.log(Level.SEVERE, LocalizationMessages.ERROR_WRITING_RESPONSE_ENTITY(), ex); | ||
| if (ex.getCause() instanceof IOException) { |
There was a problem hiding this comment.
The ex instanceof IOException is relevant here too.
There was a problem hiding this comment.
Not sure if you want to check all nested causes and suppressed throwables for IOException too?
| @@ -669,11 +669,18 @@ public OutputStream getOutputStream(final int contentLength) throws IOException | |||
|
|
|||
| } catch (final Throwable ex) { | |||
| if (response.isCommitted()) { | |||
There was a problem hiding this comment.
This if (response.isCommitted()) { check is actually kinda racy.
The response could be failed in a different thread (eg: from an HTTP/2 goaway), causing the response to be flagged as committed as part of its stream close handling.
This could be false when you check, and then a microsecond later its true.
There was a problem hiding this comment.
I think if there is IOException it does not really matter if the response was sent or not, because we cannot respond. Probably we need to skip the finally block in any case for this type of exception.
jbescos
force-pushed
the
issue5675
branch
2 times, most recently
from
f5fda1d
to
e1da103
Compare
…clipse-ee4j#5675 Signed-off-by: Jorge Bescos Gascon <[email protected]>
… #5675
Draft for now