Handle Errors from execution without killing the channel loop #132 - #133
Merged
Merged
Conversation
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 #132.
An
Errorescaping evaluation pierced bothcatch (Exception)layers (BaseKernel.handleExecuteRequestand theShellChanneldispatcher), reachedLoop.run's catch — which rethrows when noonErrorcallback is registered — and killed the shell-channel loop thread: noexecute_reply, kernel permanently unresponsive. For JShell evaluation this is largely masked (user throwables arrive wrapped inEvalException), but kernel-side code (rendering, magics, extensions) and non-JShell kernels built onjjava-jupyterhit it directly.Two defensive layers, per the issue:
BaseKernel: the execute/inspect/complete handlers now catchThrowable, so the user sees a proper error (PublishError+ error reply) and the kernel keeps running.ErrorReply.of,PublishError.ofandErrorFormatter.formatwiden fromExceptiontoThrowableaccordingly (source-compatible for existing callers;ErrorFormatterimplementors need the wider signature).ShellChannel: the dispatcher's per-message guard widens toThrowableas a last resort, so nothing a handler throws can take down the loop.New
BaseKernelErrorHandlingTestdriveshandleExecuteRequestwith an evaluator throwingAssertionError,NoClassDefFoundError, and (as the control)RuntimeException, asserting an error is published and replied in each case. Verified the new tests fail against the previous code (the twoErrorcases escape raw) and pass with the fix; fulljjava-jupytersuite: 154 tests green.