fix(jupyter): report cell errors instead of failing silently#35328
Merged
Conversation
The JS kernel rewrite (#34083) read the evaluate op result under a `.value` wrapper that the Rust side never sends. `op_jupyter_repl_evaluate` returns a `cdp::EvaluateResponse` directly (`{ result, exceptionDetails }`), so `evalResult?.value?.exceptionDetails` was always `undefined`: a throwing cell silently became `status: "ok"` with no error broadcast, and the last-expression value / completion objectId lookups were broken the same way. Drop the bogus `.value` indirection in the three affected reads. Adds a regression test asserting a throwing cell replies with `status: "error"` and broadcasts an `error` message with a traceback. Fixes #35290
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
1 participant
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.
deno jupyterstopped surfacing cell errors in 2.8.2: a cell that throwsjust produced an empty
status: "ok"reply with no stack trace, where2.8.1 showed the error. The regression came in with the JS-kernel rewrite
in #34083.
The cause is a data-shape mismatch between the kernel JS and the Rust op
it calls.
op_jupyter_repl_evaluatereturns acdp::EvaluateResponsedirectly, which serializes to
{ result, exceptionDetails }at the toplevel. The JS kernel, however, read it through a
.valuewrapper that theRust side never sends, so
evalResult?.value?.exceptionDetailswas alwaysundefined. The error branch never fired and the throwing cell silentlyreported success with no
errorbroadcast. The same wrong indirectionalso broke the last-expression display value and the completion objectId
lookup.
This drops the bogus
.valuein the three affected reads so the kernelreads
exceptionDetails,result, andresult.objectIdoff the responsewhere they actually live.
Adds a regression test that evaluates a throwing cell and asserts the
execute_replycomes back withstatus: "error"plus a populatedtraceback, and that an
errormessage is broadcast on iopub.Fixes #35290