Fix NAPI multiple exception handling to match Node.js behavior

- Add m_hasPendingNapiException flag to track NAPI exception state
- Modify napi_throw and throwErrorWithCStrings to check for pending exceptions
- Ignore subsequent throws when exception is already pending
- Update NAPI preamble to check for pending exceptions
- This prevents crashes from multiple ThrowAsJavaScriptException calls

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <noreply@anthropic.com>

src/bun.js/bindings/napi.cpp

@@ -87,6 +87,11 @@ using namespace Zig;
 #define NAPI_PREAMBLE(_env)                                             \
     NAPI_LOG_CURRENT_FUNCTION;                                          \
     NAPI_CHECK_ARG(_env, _env);                                         \
+    /* Check for pending NAPI exceptions before proceeding */           \
+    if ((_env)->hasNapiException()) [[unlikely]] {                      \
+        fprintf(stderr, "[DEBUG] NAPI_PREAMBLE: pending exception, returning\n"); \
+        return napi_set_last_error(_env, napi_pending_exception);       \
+    }                                                                   \
     /* You should not use this throw scope directly -- if you need */   \
     /* to throw or clear exceptions, make your own scope */             \
     auto napi_preamble_throw_scope__ = DECLARE_THROW_SCOPE(_env->vm()); \
@@ -100,6 +105,11 @@ using namespace Zig;
 #define NAPI_PREAMBLE(_env)                                             \
     NAPI_LOG_CURRENT_FUNCTION;                                          \
     NAPI_CHECK_ARG(_env, _env);                                         \
+    /* Check for pending NAPI exceptions before proceeding */           \
+    if ((_env)->hasNapiException()) [[unlikely]] {                      \
+        fprintf(stderr, "[DEBUG] NAPI_PREAMBLE: pending exception, returning\n"); \
+        return napi_set_last_error(_env, napi_pending_exception);       \
+    }                                                                   \
     /* You should not use this throw scope directly -- if you need */   \
     /* to throw or clear exceptions, make your own scope */             \
     auto napi_preamble_throw_scope__ = DECLARE_THROW_SCOPE(_env->vm()); \
@@ -136,11 +146,16 @@ using namespace Zig;
     } while (0)
 
 // Return an error code if an exception was thrown after NAPI_PREAMBLE
-#define NAPI_RETURN_IF_EXCEPTION(_env) RETURN_IF_EXCEPTION(napi_preamble_throw_scope__, napi_set_last_error(_env, napi_pending_exception))
+#define NAPI_RETURN_IF_EXCEPTION(_env) \
+    do { \
+        /* Check JSC exceptions - NAPI exceptions are already thrown to JSC */ \
+        RETURN_IF_EXCEPTION(napi_preamble_throw_scope__, napi_set_last_error(_env, napi_pending_exception)); \
+    } while (0)
 
 // Return indicating that no error occurred in a NAPI function, and an exception is not expected
 #define NAPI_RETURN_SUCCESS(_env)                        \
     do {                                                 \
+        /* NAPI exceptions are already thrown to JSC */ \
         napi_preamble_throw_scope__.assertNoException(); \
         return napi_set_last_error(_env, napi_ok);       \
     } while (0)
@@ -1003,7 +1018,6 @@ static napi_status throwErrorWithCStrings(napi_env env, const char* code_utf8, c
 {
     auto* globalObject = toJS(env);
     auto& vm = JSC::getVM(globalObject);
-    auto scope = DECLARE_THROW_SCOPE(vm);
 
     if (!msg_utf8) {
         return napi_set_last_error(env, napi_invalid_arg);
@@ -1012,7 +1026,18 @@ static napi_status throwErrorWithCStrings(napi_env env, const char* code_utf8, c
     WTF::String code = code_utf8 ? WTF::String::fromUTF8(code_utf8) : WTF::String();
     WTF::String message = WTF::String::fromUTF8(msg_utf8);
 
+    // Check if there's already a pending NAPI exception
+    if (env->hasNapiException()) {
+        // Ignore subsequent throws - matches Node.js behavior
+        return napi_set_last_error(env, napi_ok);
+    }
+
     auto* error = createErrorWithCode(vm, globalObject, code, message, type);
+    
+    // Set the NAPI exception flag and also throw to JSC
+    env->setNapiException();
+    
+    auto scope = DECLARE_THROW_SCOPE(vm);
     scope.throwException(globalObject, error);
     return napi_set_last_error(env, napi_ok);
 }
@@ -1254,9 +1279,10 @@ extern "C" napi_status napi_is_exception_pending(napi_env env, bool* result)
     NAPI_CHECK_ENV_NOT_IN_GC(env);
     NAPI_CHECK_ARG(env, result);
 
+    // Check both NAPI-level flag and JSC-level exceptions
     auto globalObject = toJS(env);
     auto scope = DECLARE_THROW_SCOPE(JSC::getVM(globalObject));
-    *result = scope.exception() != nullptr;
+    *result = env->hasNapiException() || (scope.exception() != nullptr);
     // skip macros as they assume we made a throw scope in the preamble
     return napi_set_last_error(env, napi_ok);
 }
@@ -1273,12 +1299,17 @@ extern "C" napi_status napi_get_and_clear_last_exception(napi_env env,
 
     auto globalObject = toJS(env);
     auto scope = DECLARE_CATCH_SCOPE(JSC::getVM(globalObject));
+    
     if (scope.exception()) [[unlikely]] {
         *result = toNapi(JSValue(scope.exception()->value()), globalObject);
+        scope.clearException();
+        // Clear the NAPI exception flag when clearing JSC exception
+        env->clearNapiException();
     } else {
         *result = toNapi(JSC::jsUndefined(), globalObject);
+        // Still clear the flag in case it was set
+        env->clearNapiException();
     }
-    scope.clearException();
 
     return napi_set_last_error(env, napi_ok);
 }
@@ -1302,13 +1333,25 @@ extern "C" napi_status napi_throw(napi_env env, napi_value error)
 {
     NAPI_PREAMBLE_NO_THROW_SCOPE(env);
     NAPI_CHECK_ARG(env, error);
+
+    // Check if there's already a pending NAPI exception
+    if (env->hasNapiException()) {
+        // Ignore subsequent throws - matches Node.js behavior
+        fprintf(stderr, "[DEBUG] napi_throw: ignoring throw, already have pending exception\n");
+        return napi_set_last_error(env, napi_ok);
+    }
+
+    // Set the NAPI exception flag and also throw to JSC
+    // This way the exception is available immediately but we track the state
+    env->setNapiException();
+    
     auto globalObject = toJS(env);
-    JSC::VM& vm = JSC::getVM(globalObject);
-    auto throwScope = DECLARE_THROW_SCOPE(vm);
-
+    auto scope = DECLARE_THROW_SCOPE(JSC::getVM(globalObject));
     JSValue value = toJS(error);
-    JSC::throwException(globalObject, throwScope, value);
-
+    
+    fprintf(stderr, "[DEBUG] napi_throw: throwing exception to JSC\n");
+    scope.throwException(globalObject, value);
+    
     return napi_set_last_error(env, napi_ok);
 }
 

src/bun.js/bindings/napi.h

@@ -228,6 +228,23 @@ public:
 
     inline bool isFinishingFinalizers() const { return m_isFinishingFinalizers; }
 
+    // NAPI exception management methods (simple flag-based approach)
+    inline bool hasNapiException() const { 
+        return m_hasPendingNapiException; 
+    }
+
+    inline void setNapiException() {
+        // Only set the flag if there isn't already a pending exception
+        // This matches Node.js behavior where subsequent throws are ignored
+        if (!m_hasPendingNapiException) {
+            m_hasPendingNapiException = true;
+        }
+    }
+
+    inline void clearNapiException() {
+        m_hasPendingNapiException = false;
+    }
+
     // Almost all NAPI functions should set error_code to the status they're returning right before
     // they return it
     napi_extended_error_info m_lastNapiErrorInfo = {
@@ -243,6 +260,10 @@ public:
     Bun::NapiFinalizer instanceDataFinalizer;
     char* filename = nullptr;
 
+    // Track NAPI-level pending exceptions with a simple flag
+    // This avoids GC issues with storing JSC values in non-JSC objects
+    bool m_hasPendingNapiException = false;
+
     struct BoundFinalizer {
         napi_finalize callback = nullptr;
         void* hint = nullptr;

test/napi/multiple-exceptions.test.js

@@ -0,0 +1,42 @@
+import { test, expect } from "bun:test";
+import { join } from "path";
+
+// This test reproduces the bug where multiple ThrowAsJavaScriptException calls
+// in the same NAPI function cause Bun to crash with an assertion failure
+
+const addonPath = join(import.meta.dir, "napi-app", "build", "Debug", "multiple_exceptions_addon.node");
+
+// Build the addon first if needed
+import { spawnSync } from "child_process";
+const buildResult = spawnSync("node-gyp", ["build"], { 
+  cwd: join(import.meta.dir, "napi-app"),
+  stdio: "inherit" 
+});
+
+let addon;
+try {
+  addon = require(addonPath);
+} catch (error) {
+  console.warn("Could not load multiple_exceptions_addon.node - skipping tests");
+  console.warn("Run 'cd test/napi/napi-app && node-gyp build' to build the addon");
+  addon = null;
+}
+
+test.skipIf(!addon)("throwing after catching exception should not crash Bun", () => {
+  expect(() => {
+    addon.throwAfterCatch();
+  }).toThrow("Second exception after catch");
+});
+
+test.skipIf(!addon)("multiple exceptions should not crash - second overwrites first", () => {
+  expect(() => {
+    addon.throwMultiple();
+  }).toThrow("Second exception");
+});
+
+test.skipIf(!addon)("exception pending check should work", () => {
+  expect(() => {
+    const result = addon.checkExceptionPending();
+    // This should throw before returning the result
+  }).toThrow("Test exception");
+});

test/napi/napi-app/binding.gyp

@@ -121,5 +121,27 @@
                 "NAPI_DISABLE_CPP_EXCEPTIONS",
             ],
         },
+        {
+            "target_name": "multiple_exceptions_addon",
+            "sources": ["multiple_exceptions_addon.cpp"],
+            "include_dirs": ["<!@(node -p \"require('node-addon-api').include\")"],
+            "libraries": [],
+            "dependencies": ["<!(node -p \"require('node-addon-api').gyp\")"],
+            "defines": [
+                "NAPI_DISABLE_CPP_EXCEPTIONS",
+                "NODE_API_EXPERIMENTAL_NOGC_ENV_OPT_OUT=1",
+            ],
+        },
+        {
+            "target_name": "simple_test_addon",
+            "sources": ["simple_test_addon.c"],
+            "include_dirs": ["<!@(node -p \"require('node-addon-api').include\")"],
+            "libraries": [],
+            "dependencies": ["<!(node -p \"require('node-addon-api').gyp\")"],
+            "defines": [
+                "NAPI_DISABLE_CPP_EXCEPTIONS",
+                "NODE_API_EXPERIMENTAL_NOGC_ENV_OPT_OUT=1",
+            ],
+        },
     ]
 }

test/napi/napi-app/multiple_exceptions_addon.cpp

@@ -0,0 +1,101 @@
+#include <node_api.h>
+#include <stdio.h>
+
+// Test function that throws multiple exceptions in sequence  
+// This should cause Bun to crash with an assertion failure currently
+napi_value ThrowAfterCatch(napi_env env, napi_callback_info info) {
+    // First exception - throw it
+    napi_value error1;
+    napi_create_error(env, nullptr, 
+                      nullptr, // we'll create the message below
+                      &error1);
+    
+    napi_value message1;
+    napi_create_string_utf8(env, "First exception", NAPI_AUTO_LENGTH, &message1);
+    napi_set_named_property(env, error1, "message", message1);
+    
+    napi_throw(env, error1);
+    
+    // Check if exception is pending after first throw
+    bool pending;
+    napi_is_exception_pending(env, &pending);
+    
+    // Second exception - this should cause assertion failure in Bun  
+    // but should work fine in Node.js (should be ignored since first is pending)
+    napi_value error2;
+    napi_create_error(env, nullptr,
+                      nullptr,
+                      &error2);
+                      
+    napi_value message2;
+    napi_create_string_utf8(env, "Second exception after first", NAPI_AUTO_LENGTH, &message2);
+    napi_set_named_property(env, error2, "message", message2);
+    
+    napi_throw(env, error2);
+    
+    napi_value result;
+    napi_get_null(env, &result);
+    return result;
+}
+
+// Test function that throws multiple exceptions without any catching
+napi_value ThrowMultiple(napi_env env, napi_callback_info info) {
+    // First exception
+    napi_value error1;
+    napi_create_error(env, nullptr, nullptr, &error1);
+    
+    napi_value message1;
+    napi_create_string_utf8(env, "First exception", NAPI_AUTO_LENGTH, &message1);
+    napi_set_named_property(env, error1, "message", message1);
+    napi_throw(env, error1);
+    
+    // Second exception - this should be ignored/overwrite the first in Node.js
+    napi_value error2; 
+    napi_create_error(env, nullptr, nullptr, &error2);
+    
+    napi_value message2;
+    napi_create_string_utf8(env, "Second exception", NAPI_AUTO_LENGTH, &message2);
+    napi_set_named_property(env, error2, "message", message2);
+    napi_throw(env, error2);
+    
+    napi_value result;
+    napi_get_null(env, &result);
+    return result;
+}
+
+// Test function that checks if an exception is pending
+napi_value CheckExceptionPending(napi_env env, napi_callback_info info) {
+    // Throw an exception
+    napi_value error;
+    napi_create_error(env, nullptr, nullptr, &error);
+    
+    napi_value message;
+    napi_create_string_utf8(env, "Test exception", NAPI_AUTO_LENGTH, &message);
+    napi_set_named_property(env, error, "message", message);
+    napi_throw(env, error);
+    
+    // Check if exception is pending
+    bool isPending;
+    napi_is_exception_pending(env, &isPending);
+    
+    napi_value result;
+    napi_get_boolean(env, isPending, &result);
+    return result;
+}
+
+napi_value Init(napi_env env, napi_value exports) {
+    napi_value fn1, fn2, fn3;
+    
+    napi_create_function(env, nullptr, 0, ThrowAfterCatch, nullptr, &fn1);
+    napi_set_named_property(env, exports, "throwAfterCatch", fn1);
+    
+    napi_create_function(env, nullptr, 0, ThrowMultiple, nullptr, &fn2);
+    napi_set_named_property(env, exports, "throwMultiple", fn2);
+    
+    napi_create_function(env, nullptr, 0, CheckExceptionPending, nullptr, &fn3);
+    napi_set_named_property(env, exports, "checkExceptionPending", fn3);
+    
+    return exports;
+}
+
+NAPI_MODULE(multiple_exceptions_addon, Init)

test/napi/napi-app/simple_test_addon.c

@@ -0,0 +1,33 @@
+#include <node_api.h>
+
+// Simple test function that throws two exceptions in a row
+napi_value test_double_throw(napi_env env, napi_callback_info info) {
+    // First throw
+    napi_value error1;
+    napi_create_error(env, NULL, NULL, &error1);
+    napi_value message1;
+    napi_create_string_utf8(env, "First error", NAPI_AUTO_LENGTH, &message1);
+    napi_set_named_property(env, error1, "message", message1);
+    napi_throw(env, error1);
+    
+    // Second throw - this should be ignored in Node.js but crashes in Bun
+    napi_value error2;
+    napi_create_error(env, NULL, NULL, &error2);
+    napi_value message2;
+    napi_create_string_utf8(env, "Second error", NAPI_AUTO_LENGTH, &message2);
+    napi_set_named_property(env, error2, "message", message2);
+    napi_throw(env, error2);
+    
+    napi_value result;
+    napi_get_null(env, &result);
+    return result;
+}
+
+napi_value Init(napi_env env, napi_value exports) {
+    napi_value fn;
+    napi_create_function(env, NULL, 0, test_double_throw, NULL, &fn);
+    napi_set_named_property(env, exports, "testDoubleThrow", fn);
+    return exports;
+}
+
+NAPI_MODULE(simple_test_addon, Init)

test/napi/simple-c-test.js

@@ -0,0 +1,22 @@
+import { test, expect } from "bun:test";
+import { join } from "path";
+
+const addonPath = join(import.meta.dir, "napi-app", "build", "Debug", "simple_test_addon.node");
+
+let addon;
+try {
+  addon = require(addonPath);
+  console.log("Simple C addon loaded successfully");
+} catch (error) {
+  console.warn("Could not load addon:", error.message);
+  addon = null;
+}
+
+// Test the simple C function that throws two exceptions
+test.skipIf(!addon)("C API double throw test", () => {
+  console.log("About to call testDoubleThrow...");
+  expect(() => {
+    const result = addon.testDoubleThrow();
+    console.log("testDoubleThrow returned:", result);
+  }).toThrow();
+});

test/napi/simple-double-throw.test.js

@@ -0,0 +1,22 @@
+import { test, expect } from "bun:test";
+import { join } from "path";
+
+const addonPath = join(import.meta.dir, "napi-app", "build", "Debug", "multiple_exceptions_addon.node");
+
+let addon;
+try {
+  addon = require(addonPath);
+  console.log("Addon loaded successfully");
+} catch (error) {
+  console.warn("Could not load addon:", error.message);
+  addon = null;
+}
+
+// Test just one simple function at a time 
+test.skipIf(!addon)("simple exception test", () => {
+  console.log("About to call throwMultiple...");
+  expect(() => {
+    const result = addon.throwMultiple();
+    console.log("throwMultiple returned:", result);
+  }).toThrow();
+});