Repository navigation
Preserve native error values across JSI host callbacks - #264
Merged
bghgary merged 3 commits intoOct 13, 2026
Merged
Conversation
…alue
Native code throws Napi::Error, a C++ exception carrying the JavaScript
error object. JSI reports any std::exception that escapes a host
function as a fresh Error("Exception in HostFunction: " + what()), so
on the JSI backend a polyfill's TypeError reached scripts as a plain
Error with a prefixed message: `instanceof TypeError` was false and
messages no longer compared equal, unlike every other backend.
Route every host-function trampoline (Function::New callbacks, class
constructors, static and instance methods and accessors) through a
helper that catches Napi::Error and rethrows jsi::JSError with the
original value, the same conversion Napi::Eval already does in the
other direction (BabylonJS#246).
Regression test: "native exceptions reach scripts as the thrown error
object, with its class and message, on every engine".
Move the regression into tests.nodeApi.ts to match the current split test suite.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 27231fa6-6686-460a-86dc-642f10ba32b5
JS-to-native callbacks can report an error either by throwing Napi::Error or by calling ThrowAsJavaScriptException, which records an exception in the environment. The JSI trampoline converted only the first form, allowing pending exceptions to escape their JavaScript call site and surface later as uncaught dispatcher failures. Check and clear the pending slot before every JSI host callback returns, then throw its original JavaScript value through JSI. Add a reentrant callback regression that exercises the pending-exception form without depending on a particular polyfill. Resolve the test-suite split by registering the callback in Tests.JavaScript.cpp and declaring it in globals.d.ts, with the regression in tests.nodeApi.ts. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 27231fa6-6686-460a-86dc-642f10ba32b5
Contributor
There was a problem hiding this comment.
Warning
Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.
Copilot review overview
2 open findings
What changed in this PR
Improves the Node-API JSI backend’s exception propagation so native-thrown errors preserve their original JavaScript value/type (e.g., TypeError) and pending exceptions from re-entrant callbacks are surfaced at the correct boundary.
Changes:
- Add unit tests asserting native exceptions reach scripts as the originally-thrown JS error object/value.
- Add a helper
CallHostwrapper to consistently translateNapi::Errorthrows and pending exceptions (last_exception) intojsi::JSErrorat host-function boundaries. - Introduce a test-only global (
throwPendingAfterCallback) to reproduce “pending exception after callback” behavior.
| File | Description |
|---|---|
| Tests/UnitTests/Source/Tests.JavaScript.cpp | Adds a native helper global used by JS tests to validate pending-exception propagation. |
| Tests/UnitTests/Source/Scripts/tests.nodeApi.ts | Adds regression tests ensuring error class/message/value fidelity and pending-exception behavior. |
| Tests/UnitTests/Source/Scripts/globals.d.ts | Declares the new test helper global for TypeScript compilation. |
| Core/Node-API-JSI/Include/napi/napi-inl.h | Adds details::CallHost and wraps host callbacks to preserve native error values across the JSI boundary. |
🧠 Review effort: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
A callback can record a pending JavaScript exception and also throw Napi::Error. Converting the thrown error without clearing the pending slot leaves the error to be thrown again by the next host callback, even after JavaScript catches the original. Clear the slot before propagating the thrown error. Cover void- and value-returning callbacks with matching and distinct pending/thrown objects, checking error identity, the pending slot, and the next callback. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 27231fa6-6686-460a-86dc-642f10ba32b5
Copilot stopped reviewing on behalf of
bghgary due to an error
October 13, 2026 01:52
bkaradzic-microsoft
approved these changes
Oct 13, 2026
bghgary
added a commit
to BabylonJS/BabylonNative
that referenced
this pull request
Oct 15, 2026
*[Created by Copilot on behalf of @bghgary]* Updates JsRuntimeHost to include host-controlled loading of packaged JavaScript chunks from [JsRuntimeHost#259](BabylonJS/JsRuntimeHost#259). Hosts must supply a resource resolver; this update does not enable unrestricted file or network loading. The dependency update also includes native-error preservation across JSI host callbacks from [JsRuntimeHost#264](BabylonJS/JsRuntimeHost#264). Copilot-Session: f699c13e-9c11-4caa-b86f-eb8003761f2b
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.
[Created by Copilot on behalf of @bghgary]
Native errors on the JavaScript Interface (JSI) backend reach scripts as generic errors instead of retaining their original class, message, and value. Pending exceptions from reentrant callbacks can also escape the call site.
Supersedes #252, originally authored by @matthargett. Both original commits retain Matt Hargett's authorship; the tests have been relocated to match current
main. This replacement is needed because maintainers cannot update the original branch in its organization-owned fork.