diff --git a/src/jsc/bindings/napi.cpp b/src/jsc/bindings/napi.cpp index 54c289e47f5..443b605e0e8 100644 --- a/src/jsc/bindings/napi.cpp +++ b/src/jsc/bindings/napi.cpp @@ -1070,8 +1070,6 @@ static napi_status createErrorWithNapiValues(napi_env env, napi_value code, napi { auto* globalObject = toJS(env); auto& vm = JSC::getVM(globalObject); - auto scope = DECLARE_THROW_SCOPE(vm); - RETURN_IF_EXCEPTION(scope, napi_pending_exception); NAPI_CHECK_ARG(env, result); NAPI_CHECK_ARG(env, message); @@ -1081,15 +1079,23 @@ static napi_status createErrorWithNapiValues(napi_env env, napi_value code, napi js_message.isString() && (js_code.isEmpty() || js_code.isString()), napi_string_expected); + // napi_create_error and friends must succeed even when a VM-level exception + // is already pending (matching Node.js behavior). Our operations (getString + // on already-validated strings, createErrorWithCode) do not consult + // vm.m_exception and run correctly in its presence. We only detect a NEW + // exception that appeared when there was none before. + JSC::Exception* preExistingException = vm.exception(); + auto wtf_code = js_code.isEmpty() ? WTF::String() : js_code.getString(globalObject); - RETURN_IF_EXCEPTION(scope, napi_set_last_error(env, napi_pending_exception)); auto wtf_message = js_message.getString(globalObject); - RETURN_IF_EXCEPTION(scope, napi_set_last_error(env, napi_pending_exception)); *result = toNapi( createErrorWithCode(vm, globalObject, wtf_code, wtf_message, type), globalObject); - RETURN_IF_EXCEPTION(scope, napi_set_last_error(env, napi_pending_exception)); + + if (!preExistingException && vm.exception()) { + return napi_set_last_error(env, napi_pending_exception); + } return napi_set_last_error(env, napi_ok); } diff --git a/test/napi/napi-app/bun.lock b/test/napi/napi-app/bun.lock index 605f6d47777..099875ae95c 100644 --- a/test/napi/napi-app/bun.lock +++ b/test/napi/napi-app/bun.lock @@ -1,5 +1,6 @@ { "lockfileVersion": 1, + "configVersion": 0, "workspaces": { "": { "name": "napi-buffer-bug", diff --git a/test/napi/napi-app/standalone_tests.cpp b/test/napi/napi-app/standalone_tests.cpp index 23050e9d2fc..b6b74ebba4b 100644 --- a/test/napi/napi-app/standalone_tests.cpp +++ b/test/napi/napi-app/standalone_tests.cpp @@ -2351,6 +2351,189 @@ static napi_value test_napi_create_tsfn_async_context_frame(const Napi::Callback return env.Undefined(); } +// Regression test for https://github.com/oven-sh/bun/issues/22259 +// napi_create_error and friends must succeed even when a VM-level exception is +// pending. Before the fix, createErrorWithNapiValues had a local +// DECLARE_THROW_SCOPE with RETURN_IF_EXCEPTION checks that returned +// napi_pending_exception when vm.m_exception was set. +// +// We create a VM-level exception via napi_call_function: calling a C++ +// callback that throws sets vm.m_exception through JSC::call. +static napi_value thrower_cb(napi_env env, napi_callback_info) { + napi_throw_error(env, nullptr, "vm-level"); + return nullptr; +} + +static napi_value test_issue_22259(const Napi::CallbackInfo &info) { + napi_env env = info.Env(); + napi_status status; + + // Prepare string values before triggering a VM-level exception. + napi_value error_msg; + status = napi_create_string_utf8(env, "test error", NAPI_AUTO_LENGTH, &error_msg); + if (status != napi_ok) { + printf("napi_create_string_utf8 (error_msg) failed: %d\n", status); + return nullptr; + } + + napi_value error_code; + status = napi_create_string_utf8(env, "ERR_TEST", NAPI_AUTO_LENGTH, &error_code); + if (status != napi_ok) { + printf("napi_create_string_utf8 (error_code) failed: %d\n", status); + return nullptr; + } + + // Create a JS function that throws, to produce a VM-level exception + // when called via napi_call_function. + napi_value throw_fn; + status = napi_create_function(env, "thrower", NAPI_AUTO_LENGTH, thrower_cb, nullptr, &throw_fn); + if (status != napi_ok) { + printf("napi_create_function failed: %d\n", status); + return nullptr; + } + + napi_value global; + status = napi_get_global(env, &global); + if (status != napi_ok) { + printf("napi_get_global failed: %d\n", status); + return nullptr; + } + + // Call the throwing function to set vm.m_exception. + // napi_call_function first propagates any pending N-API exception to + // VM level via throwPendingException(), then calls JSC::call() which + // sets vm.m_exception when the function throws. + napi_value call_result; + status = napi_call_function(env, global, throw_fn, 0, nullptr, &call_result); + if (status != napi_pending_exception) { + printf("napi_call_function unexpected status: %d (expected %d)\n", status, napi_pending_exception); + return nullptr; + } + + // Verify VM exception is pending + bool is_pending = false; + status = napi_is_exception_pending(env, &is_pending); + if (status != napi_ok || !is_pending) { + printf("napi_is_exception_pending: expected true (status %d)\n", status); + return nullptr; + } + + // Now with a VM-level exception pending, napi_create_error must still succeed. + // Before the fix, RETURN_IF_EXCEPTION would return napi_pending_exception here. + napi_value error_val; + status = napi_create_error(env, error_code, error_msg, &error_val); + if (status != napi_ok) { + printf("napi_create_error failed with VM-level exception pending: %d\n", status); + return nullptr; + } + + napi_value type_error_val; + status = napi_create_type_error(env, error_code, error_msg, &type_error_val); + if (status != napi_ok) { + printf("napi_create_type_error failed with VM-level exception pending: %d\n", status); + return nullptr; + } + + napi_value range_error_val; + status = napi_create_range_error(env, error_code, error_msg, &range_error_val); + if (status != napi_ok) { + printf("napi_create_range_error failed with VM-level exception pending: %d\n", status); + return nullptr; + } + + napi_value syntax_error_val; + status = node_api_create_syntax_error(env, error_code, error_msg, &syntax_error_val); + if (status != napi_ok) { + printf("node_api_create_syntax_error failed with VM-level exception pending: %d\n", status); + return nullptr; + } + + puts("napi_create_error functions succeeded with VM-level exception pending"); + + // Loop test: call napi_create_error multiple times with VM exception still pending. + for (int i = 0; i < 5; i++) { + napi_value loop_error_val; + status = napi_create_error(env, error_code, error_msg, &loop_error_val); + if (status != napi_ok) { + printf("napi_create_error loop iteration %d failed: %d\n", i, status); + return nullptr; + } + napi_valuetype loop_type; + status = napi_typeof(env, loop_error_val, &loop_type); + if (status != napi_ok || loop_type != napi_object) { + printf("napi_create_error loop iteration %d: unexpected type %d (status %d)\n", i, loop_type, status); + return nullptr; + } + } + puts("napi_create_error loop test passed (5 iterations with VM exception pending)"); + + // Clear the pending exception so we can validate the created objects. + // Also validate that the pending exception is the VM-level throw from thrower_cb. + napi_value pending_exception; + status = napi_get_and_clear_last_exception(env, &pending_exception); + if (status != napi_ok) { + printf("napi_get_and_clear_last_exception failed: %d\n", status); + return nullptr; + } + { + napi_valuetype pending_type; + status = napi_typeof(env, pending_exception, &pending_type); + if (status != napi_ok || pending_type != napi_object) { + printf("pending_exception: expected object, got type %d (status %d)\n", pending_type, status); + return nullptr; + } + napi_value pending_message; + status = napi_get_named_property(env, pending_exception, "message", &pending_message); + if (status != napi_ok) { + printf("pending_exception.message: get failed with status %d\n", status); + return nullptr; + } + char pending_msg_buf[64]; + size_t pending_msg_len = 0; + status = napi_get_value_string_utf8(env, pending_message, pending_msg_buf, sizeof(pending_msg_buf), &pending_msg_len); + if (status != napi_ok) { + printf("pending_exception.message: get_value_string_utf8 failed with status %d\n", status); + return nullptr; + } + if (strcmp(pending_msg_buf, "vm-level") != 0) { + printf("pending_exception.message: expected 'vm-level', got '%s'\n", pending_msg_buf); + return nullptr; + } + puts("pending_exception confirmed: 'vm-level' error object"); + } + + // Verify each created error is a proper object + napi_valuetype type; + + status = napi_typeof(env, error_val, &type); + if (status != napi_ok || type != napi_object) { + printf("error_val: unexpected type %d (status %d)\n", type, status); + return nullptr; + } + + status = napi_typeof(env, type_error_val, &type); + if (status != napi_ok || type != napi_object) { + printf("type_error_val: unexpected type %d (status %d)\n", type, status); + return nullptr; + } + + status = napi_typeof(env, range_error_val, &type); + if (status != napi_ok || type != napi_object) { + printf("range_error_val: unexpected type %d (status %d)\n", type, status); + return nullptr; + } + + status = napi_typeof(env, syntax_error_val, &type); + if (status != napi_ok || type != napi_object) { + printf("syntax_error_val: unexpected type %d (status %d)\n", type, status); + return nullptr; + } + + puts("napi_create_error produced valid error objects"); + + return ok(env); +} + void register_standalone_tests(Napi::Env env, Napi::Object exports) { REGISTER_FUNCTION(env, exports, test_issue_7685); REGISTER_FUNCTION(env, exports, test_issue_11949); @@ -2394,6 +2577,7 @@ void register_standalone_tests(Napi::Env env, Napi::Object exports) { REGISTER_FUNCTION(env, exports, test_issue_25933); REGISTER_FUNCTION(env, exports, test_napi_make_callback_async_context_frame); REGISTER_FUNCTION(env, exports, test_napi_create_tsfn_async_context_frame); + REGISTER_FUNCTION(env, exports, test_issue_22259); } } // namespace napitests diff --git a/test/napi/napi.test.ts b/test/napi/napi.test.ts index cdf548e5a72..57f961dbd1b 100644 --- a/test/napi/napi.test.ts +++ b/test/napi/napi.test.ts @@ -612,6 +612,7 @@ describe.concurrent("napi", () => { it("behaves as expected when performing operations with an exception pending", async () => { await checkSameOutput("test_deferred_exceptions", []); + await checkSameOutput("test_issue_22259", []); }); it("behaves as expected when performing operations with numeric string keys", async () => {