From ecf100155986ad5a26a1da90b10593a71a6c975f Mon Sep 17 00:00:00 2001 From: Matt Hargett Date: Wed, 16 Sep 2026 17:42:57 -0700 Subject: [PATCH 1/3] Node-API-JSI: rethrow native errors from host functions as their JS value 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 (#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 --- Core/Node-API-JSI/Include/napi/napi-inl.h | 65 ++++++++++++------- .../UnitTests/Source/Scripts/tests.nodeApi.ts | 26 ++++++++ 2 files changed, 69 insertions(+), 22 deletions(-) diff --git a/Core/Node-API-JSI/Include/napi/napi-inl.h b/Core/Node-API-JSI/Include/napi/napi-inl.h index 47120224..5f81bd7f 100644 --- a/Core/Node-API-JSI/Include/napi/napi-inl.h +++ b/Core/Node-API-JSI/Include/napi/napi-inl.h @@ -1324,11 +1324,25 @@ inline const T* TypedArrayOf::Data() const { namespace details { + // 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()), which loses the class (TypeError, RangeError, + // DOMException...), the exact message and any extra properties. Rethrow it as the JS value, + // which is what every other Node-API backend delivers, so scripts observe the same exception. + template + inline jsi::Value CallHost(jsi::Runtime& rt, Call&& call) { + try { + return call(); + } catch (const Napi::Error& error) { + throw jsi::JSError{rt, jsi::Value{rt, static_cast(error.Value())}}; + } + } + template struct Function { static inline jsi::Value Callback(napi_env env, const jsi::Value& thisVal, const jsi::Value* args, size_t count, void* data, Callable cb) { CallbackInfo callbackInfo{env, thisVal, args, count, {}, data}; - return {env->rt, cb(callbackInfo)}; + return CallHost(env->rt, [&]() -> jsi::Value { return {env->rt, cb(callbackInfo)}; }); } }; @@ -1336,8 +1350,7 @@ namespace details struct Function { static inline jsi::Value Callback(napi_env env, const jsi::Value& thisVal, const jsi::Value* args, size_t count, void* data, Callable cb) { CallbackInfo callbackInfo{env, thisVal, args, count, {}, data}; - cb(callbackInfo); - return {}; + return CallHost(env->rt, [&]() -> jsi::Value { cb(callbackInfo); return {}; }); } }; } @@ -2298,8 +2311,10 @@ ObjectWrap::DefineClass(napi_env env, [env, newTarget, data](jsi::Runtime& rt, const jsi::Value& thisVal, const jsi::Value* args, size_t count) -> jsi::Value { CallbackInfo callbackInfo{env, thisVal, args, count, *newTarget, data}; // TODO: use prototype to wrap object? - thisVal.getObject(rt).setProperty(rt, env->native_name, jsi::Object::createFromHostObject(rt, std::make_shared(callbackInfo))); - return {}; + return details::CallHost(rt, [&]() -> jsi::Value { + thisVal.getObject(rt).setProperty(rt, env->native_name, jsi::Object::createFromHostObject(rt, std::make_shared(callbackInfo))); + return {}; + }); })).getObject(rt).getFunction(rt)}; *newTarget = {rt, static_cast(constructor)}; @@ -2319,28 +2334,26 @@ ObjectWrap::DefineClass(napi_env env, if (p.staticVoidMethod != nullptr) { descriptor.setProperty(rt, "value", jsi::Function::createFromHostFunction(rt, name, 0, - [env, method{p.staticVoidMethod}, data{p.data}](jsi::Runtime& /*rt*/, const jsi::Value& thisVal, const jsi::Value* args, size_t count) -> jsi::Value { - (*method)({env, thisVal, args, count, nullptr, data}); - return {}; + [env, method{p.staticVoidMethod}, data{p.data}](jsi::Runtime& rt, const jsi::Value& thisVal, const jsi::Value* args, size_t count) -> jsi::Value { + return details::CallHost(rt, [&]() -> jsi::Value { (*method)({env, thisVal, args, count, nullptr, data}); return {}; }); })); } else if (p.staticMethod != nullptr) { descriptor.setProperty(rt, "value", jsi::Function::createFromHostFunction(rt, name, 0, [env, method{p.staticMethod}, data{p.data}](jsi::Runtime& rt, const jsi::Value& thisVal, const jsi::Value* args, size_t count) -> jsi::Value { - return {rt, (*method)({env, thisVal, args, count, nullptr, data})}; + return details::CallHost(rt, [&]() -> jsi::Value { return {rt, (*method)({env, thisVal, args, count, nullptr, data})}; }); })); } else if (p.staticGetter != nullptr || p.staticSetter != nullptr) { if (p.staticGetter != nullptr) { descriptor.setProperty(rt, "get", jsi::Function::createFromHostFunction(rt, name, 0, [env, getter{p.staticGetter}, data{p.data}](jsi::Runtime& rt, const jsi::Value& thisVal, const jsi::Value* args, size_t count)-> jsi::Value { - return {rt, (*getter)({env, thisVal, args, count, nullptr, data})}; + return details::CallHost(rt, [&]() -> jsi::Value { return {rt, (*getter)({env, thisVal, args, count, nullptr, data})}; }); })); } if (p.staticSetter != nullptr) { descriptor.setProperty(rt, "set", jsi::Function::createFromHostFunction(rt, name, 0, [env, setter{p.staticSetter}, data{p.data}](jsi::Runtime& rt, const jsi::Value& thisVal, const jsi::Value* args, size_t count)-> jsi::Value { - (*setter)({env, thisVal, args, count, nullptr, data}, {env, {rt, args[0]}}); - return {}; + return details::CallHost(rt, [&]() -> jsi::Value { (*setter)({env, thisVal, args, count, nullptr, data}, {env, {rt, args[0]}}); return {}; }); })); } } else if (!p.staticValue.IsEmpty()) { @@ -2349,30 +2362,38 @@ ObjectWrap::DefineClass(napi_env env, } else if (p.instanceVoidMethod != nullptr) { descriptor.setProperty(rt, "value", jsi::Function::createFromHostFunction(rt, name, 0, [env, method{p.instanceVoidMethod}, data{p.data}](jsi::Runtime& rt, const jsi::Value& thisVal, const jsi::Value* args, size_t count) -> jsi::Value { - T* nativeObject{Unwrap(env, thisVal.getObject(rt))}; - (nativeObject->*method)({env, thisVal, args, count, nullptr, data}); - return {}; + return details::CallHost(rt, [&]() -> jsi::Value { + T* nativeObject{Unwrap(env, thisVal.getObject(rt))}; + (nativeObject->*method)({env, thisVal, args, count, nullptr, data}); + return {}; + }); })); } else if (p.instanceMethod != nullptr) { descriptor.setProperty(rt, "value", jsi::Function::createFromHostFunction(rt, name, 0, [env, method{p.instanceMethod}, data{p.data}](jsi::Runtime& rt, const jsi::Value& thisVal, const jsi::Value* args, size_t count) -> jsi::Value { - T* nativeObject{Unwrap(env, thisVal.getObject(rt))}; - return {rt, (nativeObject->*method)({env, thisVal, args, count, nullptr, data})}; + return details::CallHost(rt, [&]() -> jsi::Value { + T* nativeObject{Unwrap(env, thisVal.getObject(rt))}; + return {rt, (nativeObject->*method)({env, thisVal, args, count, nullptr, data})}; + }); })); } else if (p.instanceGetter != nullptr || p.instanceSetter != nullptr) { if (p.instanceGetter != nullptr) { descriptor.setProperty(rt, "get", jsi::Function::createFromHostFunction(rt, name, 0, [env, getter{p.instanceGetter}, data{p.data}](jsi::Runtime& rt, const jsi::Value& thisVal, const jsi::Value* args, size_t count) -> jsi::Value { - T* nativeObject{Unwrap(env, thisVal.getObject(rt))}; - return {rt, (nativeObject->*getter)({env, thisVal, args, count, nullptr, data})}; + return details::CallHost(rt, [&]() -> jsi::Value { + T* nativeObject{Unwrap(env, thisVal.getObject(rt))}; + return {rt, (nativeObject->*getter)({env, thisVal, args, count, nullptr, data})}; + }); })); } if (p.instanceSetter != nullptr) { descriptor.setProperty(rt, "set", jsi::Function::createFromHostFunction(rt, name, 0, [env, setter{p.instanceSetter}, data{p.data}](jsi::Runtime& rt, const jsi::Value& thisVal, const jsi::Value* args, size_t count) -> jsi::Value { - T* nativeObject{Unwrap(env, thisVal.getObject(rt))}; - (nativeObject->*setter)({env, thisVal, args, count, nullptr, data}, {env, {rt, args[0]}}); - return {}; + return details::CallHost(rt, [&]() -> jsi::Value { + T* nativeObject{Unwrap(env, thisVal.getObject(rt))}; + (nativeObject->*setter)({env, thisVal, args, count, nullptr, data}, {env, {rt, args[0]}}); + return {}; + }); })); } } else if (!p.instanceValue.IsEmpty()) { diff --git a/Tests/UnitTests/Source/Scripts/tests.nodeApi.ts b/Tests/UnitTests/Source/Scripts/tests.nodeApi.ts index 28fa8c16..0c896eda 100644 --- a/Tests/UnitTests/Source/Scripts/tests.nodeApi.ts +++ b/Tests/UnitTests/Source/Scripts/tests.nodeApi.ts @@ -1,5 +1,31 @@ import { expect } from "chai"; +describe("native exceptions", function () { + it("reach scripts as the thrown error object, with its class and message, on every engine", function () { + // A polyfill throwing Napi::TypeError from native code must arrive as that TypeError. The JSI + // adapter used to deliver a generic Error("Exception in HostFunction: ...") instead. + let fromMethod: any; + try { + URL.createObjectURL({} as any); + } catch (e) { + fromMethod = e; + } + expect(fromMethod).to.be.an.instanceof(TypeError); + expect(fromMethod.message).to.match(/not a Blob/); + expect(fromMethod.message).to.not.match(/HostFunction/); + + let fromConstructor: any; + try { + new URL("not a url"); + } catch (e) { + fromConstructor = e; + } + expect(fromConstructor).to.be.an.instanceof(TypeError); + expect(fromConstructor.message).to.match(/Invalid URL/); + expect(fromConstructor.message).to.not.match(/HostFunction/); + }); +}); + describe("napi class prototype isolation (#172)", function () { // Regression coverage for #172: napi-defined class constructors must each // get a fresh per-class object as their `.prototype` property. Previously From b42ab989dcc2132b7e85214116b22154f28fb748 Mon Sep 17 00:00:00 2001 From: Matt Hargett Date: Wed, 16 Sep 2026 22:04:15 -0700 Subject: [PATCH 2/3] Node-API-JSI: propagate pending host callback exceptions 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 --- Core/Node-API-JSI/Include/napi/napi-inl.h | 40 ++++++++++--------- Tests/UnitTests/Source/Scripts/globals.d.ts | 1 + .../UnitTests/Source/Scripts/tests.nodeApi.ts | 7 ++++ Tests/UnitTests/Source/Tests.JavaScript.cpp | 5 +++ 4 files changed, 35 insertions(+), 18 deletions(-) diff --git a/Core/Node-API-JSI/Include/napi/napi-inl.h b/Core/Node-API-JSI/Include/napi/napi-inl.h index 5f81bd7f..3c8ca001 100644 --- a/Core/Node-API-JSI/Include/napi/napi-inl.h +++ b/Core/Node-API-JSI/Include/napi/napi-inl.h @@ -1324,15 +1324,19 @@ inline const T* TypedArrayOf::Data() const { namespace details { - // 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()), which loses the class (TypeError, RangeError, - // DOMException...), the exact message and any extra properties. Rethrow it as the JS value, - // which is what every other Node-API backend delivers, so scripts observe the same exception. + // Native callbacks report JavaScript errors either by throwing Napi::Error or by leaving an + // exception pending after ThrowAsJavaScriptException(). Convert both forms at the host-function + // boundary so JSI observes the original JavaScript value, matching the other Node-API backends. template - inline jsi::Value CallHost(jsi::Runtime& rt, Call&& call) { + inline jsi::Value CallHost(napi_env env, jsi::Runtime& rt, Call&& call) { try { - return call(); + auto result = call(); + if (!env->last_exception.isUndefined()) { + auto error = std::move(env->last_exception); + env->last_exception = jsi::Value::undefined(); + throw jsi::JSError{rt, jsi::Value{rt, error}}; + } + return result; } catch (const Napi::Error& error) { throw jsi::JSError{rt, jsi::Value{rt, static_cast(error.Value())}}; } @@ -1342,7 +1346,7 @@ namespace details struct Function { static inline jsi::Value Callback(napi_env env, const jsi::Value& thisVal, const jsi::Value* args, size_t count, void* data, Callable cb) { CallbackInfo callbackInfo{env, thisVal, args, count, {}, data}; - return CallHost(env->rt, [&]() -> jsi::Value { return {env->rt, cb(callbackInfo)}; }); + return CallHost(env, env->rt, [&]() -> jsi::Value { return {env->rt, cb(callbackInfo)}; }); } }; @@ -1350,7 +1354,7 @@ namespace details struct Function { static inline jsi::Value Callback(napi_env env, const jsi::Value& thisVal, const jsi::Value* args, size_t count, void* data, Callable cb) { CallbackInfo callbackInfo{env, thisVal, args, count, {}, data}; - return CallHost(env->rt, [&]() -> jsi::Value { cb(callbackInfo); return {}; }); + return CallHost(env, env->rt, [&]() -> jsi::Value { cb(callbackInfo); return {}; }); } }; } @@ -2311,7 +2315,7 @@ ObjectWrap::DefineClass(napi_env env, [env, newTarget, data](jsi::Runtime& rt, const jsi::Value& thisVal, const jsi::Value* args, size_t count) -> jsi::Value { CallbackInfo callbackInfo{env, thisVal, args, count, *newTarget, data}; // TODO: use prototype to wrap object? - return details::CallHost(rt, [&]() -> jsi::Value { + return details::CallHost(env, rt, [&]() -> jsi::Value { thisVal.getObject(rt).setProperty(rt, env->native_name, jsi::Object::createFromHostObject(rt, std::make_shared(callbackInfo))); return {}; }); @@ -2335,25 +2339,25 @@ ObjectWrap::DefineClass(napi_env env, if (p.staticVoidMethod != nullptr) { descriptor.setProperty(rt, "value", jsi::Function::createFromHostFunction(rt, name, 0, [env, method{p.staticVoidMethod}, data{p.data}](jsi::Runtime& rt, const jsi::Value& thisVal, const jsi::Value* args, size_t count) -> jsi::Value { - return details::CallHost(rt, [&]() -> jsi::Value { (*method)({env, thisVal, args, count, nullptr, data}); return {}; }); + return details::CallHost(env, rt, [&]() -> jsi::Value { (*method)({env, thisVal, args, count, nullptr, data}); return {}; }); })); } else if (p.staticMethod != nullptr) { descriptor.setProperty(rt, "value", jsi::Function::createFromHostFunction(rt, name, 0, [env, method{p.staticMethod}, data{p.data}](jsi::Runtime& rt, const jsi::Value& thisVal, const jsi::Value* args, size_t count) -> jsi::Value { - return details::CallHost(rt, [&]() -> jsi::Value { return {rt, (*method)({env, thisVal, args, count, nullptr, data})}; }); + return details::CallHost(env, rt, [&]() -> jsi::Value { return {rt, (*method)({env, thisVal, args, count, nullptr, data})}; }); })); } else if (p.staticGetter != nullptr || p.staticSetter != nullptr) { if (p.staticGetter != nullptr) { descriptor.setProperty(rt, "get", jsi::Function::createFromHostFunction(rt, name, 0, [env, getter{p.staticGetter}, data{p.data}](jsi::Runtime& rt, const jsi::Value& thisVal, const jsi::Value* args, size_t count)-> jsi::Value { - return details::CallHost(rt, [&]() -> jsi::Value { return {rt, (*getter)({env, thisVal, args, count, nullptr, data})}; }); + return details::CallHost(env, rt, [&]() -> jsi::Value { return {rt, (*getter)({env, thisVal, args, count, nullptr, data})}; }); })); } if (p.staticSetter != nullptr) { descriptor.setProperty(rt, "set", jsi::Function::createFromHostFunction(rt, name, 0, [env, setter{p.staticSetter}, data{p.data}](jsi::Runtime& rt, const jsi::Value& thisVal, const jsi::Value* args, size_t count)-> jsi::Value { - return details::CallHost(rt, [&]() -> jsi::Value { (*setter)({env, thisVal, args, count, nullptr, data}, {env, {rt, args[0]}}); return {}; }); + return details::CallHost(env, rt, [&]() -> jsi::Value { (*setter)({env, thisVal, args, count, nullptr, data}, {env, {rt, args[0]}}); return {}; }); })); } } else if (!p.staticValue.IsEmpty()) { @@ -2362,7 +2366,7 @@ ObjectWrap::DefineClass(napi_env env, } else if (p.instanceVoidMethod != nullptr) { descriptor.setProperty(rt, "value", jsi::Function::createFromHostFunction(rt, name, 0, [env, method{p.instanceVoidMethod}, data{p.data}](jsi::Runtime& rt, const jsi::Value& thisVal, const jsi::Value* args, size_t count) -> jsi::Value { - return details::CallHost(rt, [&]() -> jsi::Value { + return details::CallHost(env, rt, [&]() -> jsi::Value { T* nativeObject{Unwrap(env, thisVal.getObject(rt))}; (nativeObject->*method)({env, thisVal, args, count, nullptr, data}); return {}; @@ -2371,7 +2375,7 @@ ObjectWrap::DefineClass(napi_env env, } else if (p.instanceMethod != nullptr) { descriptor.setProperty(rt, "value", jsi::Function::createFromHostFunction(rt, name, 0, [env, method{p.instanceMethod}, data{p.data}](jsi::Runtime& rt, const jsi::Value& thisVal, const jsi::Value* args, size_t count) -> jsi::Value { - return details::CallHost(rt, [&]() -> jsi::Value { + return details::CallHost(env, rt, [&]() -> jsi::Value { T* nativeObject{Unwrap(env, thisVal.getObject(rt))}; return {rt, (nativeObject->*method)({env, thisVal, args, count, nullptr, data})}; }); @@ -2380,7 +2384,7 @@ ObjectWrap::DefineClass(napi_env env, if (p.instanceGetter != nullptr) { descriptor.setProperty(rt, "get", jsi::Function::createFromHostFunction(rt, name, 0, [env, getter{p.instanceGetter}, data{p.data}](jsi::Runtime& rt, const jsi::Value& thisVal, const jsi::Value* args, size_t count) -> jsi::Value { - return details::CallHost(rt, [&]() -> jsi::Value { + return details::CallHost(env, rt, [&]() -> jsi::Value { T* nativeObject{Unwrap(env, thisVal.getObject(rt))}; return {rt, (nativeObject->*getter)({env, thisVal, args, count, nullptr, data})}; }); @@ -2389,7 +2393,7 @@ ObjectWrap::DefineClass(napi_env env, if (p.instanceSetter != nullptr) { descriptor.setProperty(rt, "set", jsi::Function::createFromHostFunction(rt, name, 0, [env, setter{p.instanceSetter}, data{p.data}](jsi::Runtime& rt, const jsi::Value& thisVal, const jsi::Value* args, size_t count) -> jsi::Value { - return details::CallHost(rt, [&]() -> jsi::Value { + return details::CallHost(env, rt, [&]() -> jsi::Value { T* nativeObject{Unwrap(env, thisVal.getObject(rt))}; (nativeObject->*setter)({env, thisVal, args, count, nullptr, data}, {env, {rt, args[0]}}); return {}; diff --git a/Tests/UnitTests/Source/Scripts/globals.d.ts b/Tests/UnitTests/Source/Scripts/globals.d.ts index 78e1a8ce..a651e384 100644 --- a/Tests/UnitTests/Source/Scripts/globals.d.ts +++ b/Tests/UnitTests/Source/Scripts/globals.d.ts @@ -1,4 +1,5 @@ declare const hostPlatform: string; declare const hostEngine: string; +declare const throwPendingAfterCallback: (callback: () => void) => void; declare const napiGetPropertyNames: (object: any) => string[]; declare const napiGetPropertyNamesRaw: (object: any) => string[]; diff --git a/Tests/UnitTests/Source/Scripts/tests.nodeApi.ts b/Tests/UnitTests/Source/Scripts/tests.nodeApi.ts index 0c896eda..cda8123b 100644 --- a/Tests/UnitTests/Source/Scripts/tests.nodeApi.ts +++ b/Tests/UnitTests/Source/Scripts/tests.nodeApi.ts @@ -24,6 +24,13 @@ describe("native exceptions", function () { expect(fromConstructor.message).to.match(/Invalid URL/); expect(fromConstructor.message).to.not.match(/HostFunction/); }); + + it("propagate from the pending exception slot after a reentrant callback", function () { + let callbackRan = false; + expect(() => throwPendingAfterCallback(() => { callbackRan = true; })) + .to.throw(TypeError, "pending exception after callback"); + expect(callbackRan).to.equal(true); + }); }); describe("napi class prototype isolation (#172)", function () { diff --git a/Tests/UnitTests/Source/Tests.JavaScript.cpp b/Tests/UnitTests/Source/Tests.JavaScript.cpp index c5b38b40..13300d11 100644 --- a/Tests/UnitTests/Source/Tests.JavaScript.cpp +++ b/Tests/UnitTests/Source/Tests.JavaScript.cpp @@ -95,6 +95,11 @@ TEST(JavaScript, All) "setExitCode"); env.Global().Set("setExitCode", setExitCodeCallback); + env.Global().Set("throwPendingAfterCallback", Napi::Function::New(env, [](const Napi::CallbackInfo& info) { + info[0].As().Call({}); + Napi::TypeError::New(info.Env(), "pending exception after callback").ThrowAsJavaScriptException(); + })); + env.Global().Set("hostPlatform", Napi::Value::From(env, JSRUNTIMEHOST_PLATFORM)); env.Global().Set("hostEngine", Napi::Value::From(env, NAPI_JAVASCRIPT_ENGINE)); From cd396cae0662294062742e65acb1b90b773e42bd Mon Sep 17 00:00:00 2001 From: Gary Hsu Date: Mon, 12 Oct 2026 18:17:37 -0700 Subject: [PATCH 3/3] Clear pending exceptions when a JSI host callback throws 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 --- Core/Node-API-JSI/Include/napi/napi-inl.h | 1 + Tests/UnitTests/Source/Tests.NodeApi.cpp | 65 +++++++++++++++++++++++ 2 files changed, 66 insertions(+) diff --git a/Core/Node-API-JSI/Include/napi/napi-inl.h b/Core/Node-API-JSI/Include/napi/napi-inl.h index 3c8ca001..8cd76f58 100644 --- a/Core/Node-API-JSI/Include/napi/napi-inl.h +++ b/Core/Node-API-JSI/Include/napi/napi-inl.h @@ -1338,6 +1338,7 @@ namespace details } return result; } catch (const Napi::Error& error) { + env->last_exception = jsi::Value::undefined(); throw jsi::JSError{rt, jsi::Value{rt, static_cast(error.Value())}}; } } diff --git a/Tests/UnitTests/Source/Tests.NodeApi.cpp b/Tests/UnitTests/Source/Tests.NodeApi.cpp index 69d88deb..09ba92a9 100644 --- a/Tests/UnitTests/Source/Tests.NodeApi.cpp +++ b/Tests/UnitTests/Source/Tests.NodeApi.cpp @@ -1,10 +1,75 @@ #include #include +#include #include #include +#include #include #include +#if defined(JSRUNTIMEHOST_NAPI_ENGINE_JSI) +TEST(NodeApi, ThrownHostErrorClearsPendingException) +{ + Babylon::AppRuntime runtime{}; + std::promise> result; + + runtime.Dispatch([&result](Napi::Env env) { + env.Global().Set("throwFromVoid", Napi::Function::New(env, [](const Napi::CallbackInfo& info) { + napi_env rawEnv{info.Env()}; + Napi::Error{rawEnv, facebook::jsi::Value{rawEnv->rt, static_cast(info[1])}}.ThrowAsJavaScriptException(); + throw Napi::Error{rawEnv, facebook::jsi::Value{rawEnv->rt, static_cast(info[0])}}; + })); + env.Global().Set("throwFromValue", Napi::Function::New(env, [](const Napi::CallbackInfo& info) -> Napi::Value { + napi_env rawEnv{info.Env()}; + Napi::Error{rawEnv, facebook::jsi::Value{rawEnv->rt, static_cast(info[1])}}.ThrowAsJavaScriptException(); + throw Napi::Error{rawEnv, facebook::jsi::Value{rawEnv->rt, static_cast(info[0])}}; + })); + env.Global().Set("nextCallback", Napi::Function::New(env, [](const Napi::CallbackInfo&) {})); + + std::array passed{true, true, true}; + for (const char* callback : {"throwFromVoid", "throwFromValue"}) + { + env.Global().Set("throwingCallback", env.Global().Get(callback)); + for (bool sameError : {true, false}) + { + env.Global().Set("sameError", sameError); + const bool originalErrorCaught = Napi::Eval(env, R"( + (() => { + const thrown = new TypeError("thrown"); + const pending = sameError ? thrown : new RangeError("pending"); + try { + throwingCallback(thrown, pending); + } catch (error) { + return error === thrown; + } + return false; + })() + )", "").As().Value(); + const bool exceptionCleared{!env.IsExceptionPending()}; + const bool nextCallbackSucceeded = Napi::Eval(env, R"( + (() => { + try { + return nextCallback() === undefined; + } catch (error) { + return false; + } + })() + )", "").As().Value(); + passed[0] = passed[0] && originalErrorCaught; + passed[1] = passed[1] && exceptionCleared; + passed[2] = passed[2] && nextCallbackSucceeded; + } + } + result.set_value(passed); + }); + + const auto passed{result.get_future().get()}; + EXPECT_TRUE(passed[0]) << "The JavaScript catch must receive the thrown error object"; + EXPECT_TRUE(passed[1]) << "The caught host error must not remain pending"; + EXPECT_TRUE(passed[2]) << "The next host callback must return without throwing"; +} +#endif + // The V8JSI Node-API shim does not implement napi_create_dataview / // napi_get_dataview_info (its DataView::New throws "TODO"), so this native test // only builds on the Chakra, V8, and JavaScriptCore backends. The size_t-width