Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 14 additions & 4 deletions Core/Node-API/Source/js_native_api_quickjs.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1586,7 +1586,14 @@ napi_status napi_throw(napi_env env, napi_value error) {
JSContext* targetCtx = env->current_context ? env->current_context : env->context;
JS_Throw(targetCtx, JS_DupValue(targetCtx, jsError));

return napi_set_last_error(env, napi_pending_exception);
// Returning napi_pending_exception here would report that the throw itself
// failed. node-addon-api's Error::ThrowAsJavaScriptException reacts to that by
// re-throwing `Error::New(env)`, which clears the exception it just set and
// lets the C++ exception escape the callback wrapper. The escaped error is
// then stringified by ExternalCallback::Callback after its handle scope is
// gone, which reads freed memory.
napi_clear_last_error(env);
return napi_ok;
}

// Throw error
Expand All @@ -1602,23 +1609,26 @@ napi_status napi_throw_error(napi_env env, const char* code, const char* msg) {
}

JS_Throw(env->context, error);
return napi_set_last_error(env, napi_pending_exception);
napi_clear_last_error(env);
return napi_ok;
}

// Throw type error
napi_status napi_throw_type_error(napi_env env, const char* code, const char* msg) {
CHECK_ENV(env);

JS_ThrowTypeError(env->context, "%s", msg ? msg : "");
return napi_set_last_error(env, napi_pending_exception);
napi_clear_last_error(env);
return napi_ok;
}

// Throw range error
napi_status napi_throw_range_error(napi_env env, const char* code, const char* msg) {
CHECK_ENV(env);

JS_ThrowRangeError(env->context, "%s", msg ? msg : "");
return napi_set_last_error(env, napi_pending_exception);
napi_clear_last_error(env);
return napi_ok;
}

// Create error
Expand Down
20 changes: 20 additions & 0 deletions Tests/UnitTests/Scripts/tests.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1359,6 +1359,26 @@ describe("URLSearchParams", function () {
expect(() => paramsSet.set()).to.throw();
});

it("should preserve the type and message of an error thrown from native code", function () {
// A native throw must reach JS unchanged. When napi_throw reported failure, the
// error was replaced by an InternalError reading "Uncaught C++ exception: ...",
// built by stringifying an error whose handle scope had already closed.
let caught: any;
try {
// @ts-expect-error
paramsSet.set();
} catch (e) {
caught = e;
}
expect(caught).to.be.an.instanceOf(Error);
expect(caught.name).to.equal("Error");
// Not an equality check: the JSI backend prefixes "Exception in HostFunction: ".
expect(caught.message).to.contain(
"Failed to execute 'set' on 'URLSearchParams': 2 arguments required, but only 0 present"
);
expect(caught.message).to.not.contain("Uncaught C++ exception");
});

it("should add a number and retrieve it as a string from searchParams", function () {
// Set Number
paramsSet.set("foo", 400 as any);
Expand Down
Loading