QuickJS: report success from the napi_throw family

[bkaradzic-microsoft]

Problem

napi_throw, napi_throw_error, napi_throw_type_error and napi_throw_range_error returned napi_pending_exception after successfully scheduling the throw.

In Node-API that status means "this call failed because an exception was already pending", not "a throw is now pending". The upstream implementation returns napi_clear_last_error(env) (i.e. napi_ok).

Because the QuickJS port reported failure, Error::ThrowAsJavaScriptException in napi-inl.h took its failure branch on every native throw:

napi_status status = napi_throw(_env, Value());
#ifdef NAPI_CPP_EXCEPTIONS
if (status != napi_ok) {
throwError::New(_env); // consumes the exception that was just set
}
#endif

Error::New(env) calls napi_get_and_clear_last_exception, so the pending JS exception is discarded and a fresh C++ exception is thrown out of details::WrapCallback. ExternalCallback::Callback then catches it, observes !JS_HasException(ctx), and rebuilds the error from e.what().

By that point the HandleScope opened by ThrowAsJavaScriptException has been destroyed during unwinding, so stringifying the message reads freed memory.

Impact

Two symptoms, both of which reproduce today:

1. Wrong error surfaced to JS. The real error is replaced by InternalError: Uncaught C++ exception: <message>. Every native throw on QuickJS is affected, so err.name and err instanceof TypeError are wrong throughout.
2. Use-after-free. On Linux this segfaults. Backtrace from a BabylonNative CI core dump:

#0 js_dup quickjs.c:1628 <-- SIGSEGV
#1 js_force_tostring quickjs.c:4813
#3 JS_ToCStringLen
#4 napi_get_value_string_utf8 js_native_api_quickjs.cc:696
#5 Napi::String::Utf8Value napi-inl.h:1118
#7 Napi::Error::Message napi-inl.h:3087
#8 Napi::Error::what napi-inl.h:3157
#9 ExternalCallback::Callback js_native_api_quickjs.cc:164

The JSValue being stringified carries JS_TAG_STRING with an unaligned, freed pointer.

I instrumented the catch in ExternalCallback::Callback in a BabylonNative QuickJS build and confirmed that all ~50 native throws in that test run escaped WrapCallback with hasExc=0. After this change the count is 0.

Fix

Return napi_ok from the four throw entry points, matching upstream. The exception stays pending, WrapCallback returns normally, and the fragile e.what() fallback is never entered.

Test

Added a strict assertion to the existing URLSearchParams.set() arity throw, checking the error type and exact message rather than a substring. The pre-existing .to.throw() test could not catch this, because "Uncaught C++ exception: <msg>" still contains the expected substring.

Verified on Linux QuickJS (RelWithDebInfo):

	result
without the C++ change	expected 'InternalError' to equal 'Error' — 212 passing, 1 failing
with the C++ change	213 passing, 10/10 gtest

Also verified in a BabylonNative QuickJS build on Windows: 21/21 gtest, 49 JS assertions, exit 0, and zero escapes from WrapCallback.

[Copilot]

Pull request overview

This PR aligns the QuickJS Node-API (napi_*) throw APIs with upstream Node-API behavior by reporting success (napi_ok) after scheduling a JavaScript throw, preventing node-addon-api from taking an error-handling path that discards the real JS exception and can lead to use-after-free when building a fallback error message.

Changes:

• Update QuickJS napi_throw* functions to clear last error and return napi_ok after throwing, matching upstream semantics.
• Add a regression test ensuring native-thrown errors preserve their original JS type and exact message (no “Uncaught C++ exception …” fallback).

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File	Description
Tests/UnitTests/Scripts/tests.ts	Adds a regression assertion for preserved error type/message when native code throws.
Core/Node-API/Source/js_native_api_quickjs.cc	Makes napi_throw, napi_throw_error, napi_throw_type_error, and napi_throw_range_error report success after scheduling the exception.

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[bghgary]

[Reviewed by Copilot on behalf of @bghgary]

LGTM