Report AsyncFile handler failures to the context - #6322
Open
jnbdz wants to merge 1 commit into
Open
Conversation
When the data handler or the end handler of an AsyncFile read stream throws, the exception is caught by the InboundBuffer and, since the file does not register an exception handler on its queue, silently dropped. Dispatch both handlers through the context so that such failures are reported to the context exception handler, consistently with the other Vert.x streams and event-bus consumers. The stream's exceptionHandler keeps its I/O failure semantic and reading proceeds to the end handler. Fixes eclipse-vertx#5343
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #5343
Motivation
When the data handler or the end handler of an
AsyncFileread stream throws, nothing happens: theInboundBuffercatches the throwable inhandleEventand, sinceAsyncFileImplnever registers an exception handler on its queue, the failure is silently dropped. Neither the streamexceptionHandlernor the context ever sees it (the issue's reproducer prints nothing).As discussed on the issue, the correct behaviour is to report such failures to the context, like event-bus consumers do.
Changes
AsyncFileImpl.handleBuffer/handleEndnow dispatch the handler through the context (ContextInternal.dispatch), so an exception thrown by the handler is reported viaContext.reportException(context exception handler →Vertx.exceptionHandler→ "Unhandled exception" log). The streamexceptionHandlerkeeps its I/O failure semantic and is not invoked for a handler failure; reading is not interrupted and the end handler is still called.FileSystemTest.testReadStreamHandlerExceptionReportedToContext/testReadStreamEndHandlerExceptionReportedToContext(fail without the change).Note:
NettyFileUpload(multipart uploads) has the same silent-swallow pattern with itsInboundBuffer; left out of this PR since the issue is aboutAsyncFile, happy to address it here or separately if wanted.