napi: don't require addon code to satisfy JSC's exception-check discipline

[robobun]

Repro

The canonical two-line Init that every N-API tutorial and the node-addon-api template emit is enough:

NAPI_MODULE_INIT() {
  napi_value fn;
  napi_create_function(env, NULL, 0, Method, NULL, &fn);
  napi_set_named_property(env, exports, "hello", fn);
  return exports;
}

On an assert-enabled build with BUN_JSC_validateExceptionChecks=1, require()ing that addon aborts at module load:

ERROR: Unchecked JS exception:
    This scope can throw a JS exception: napi_create_function @ src/jsc/bindings/napi.cpp:968
        (ExceptionScope::m_recursionDepth was 10)
    But the exception was unchecked as of this scope: napi_set_named_property @ src/jsc/bindings/napi.cpp:607
        (ExceptionScope::m_recursionDepth was 10)
ASSERTION FAILED: exception check validation failed
vendor/WebKit/Source/JavaScriptCore/runtime/VM.cpp(1561) : void JSC::VM::verifyExceptionCheckNeedIsSatisfied(...)

This means no native addon can load at all under exception-scope validation. Bun's CI already runs the validator on the linux x64 ASAN test shard (scripts/runner.node.mjs sets BUN_JSC_validateExceptionChecks=1 for every test file not listed in test/no-validate-exceptions.txt), and the entire N-API section of that list (node-napi-tests/, uv.test.ts, napi-finalizer-delete-ref.test.ts, ...) is a casualty of this: those tests cannot currently run with the validator on. Release builds compile ENABLE(EXCEPTION_SCOPE_VERIFICATION) out, so they do not abort, but the underlying discipline violation is real: a pending exception raised inside Init (for example by a getter on exports) can be dropped or surface at an unrelated later point.

Cause

Two instances of the same mistake: calling two scope-opening JSC operations on the N-API path with nothing observing the exception state in between.

1. NAPI_PREAMBLE declared a JSC::ThrowScope. A ThrowScope destructor always calls simulateThrow() toward its caller unless the caller is LLInt/JIT, which sets VM::m_needExceptionCheck. JSC's discipline then requires the caller to observe exception() before the next scope is constructed. But the caller of a N-API entry point is the addon's C code, which cannot participate in that discipline, so the very next napi_* call's scope constructor finds the unsatisfied check and RELEASE_ASSERTs. Any two back-to-back napi_* calls from native code trigger it; module Init is just the most universal instance.

2. Napi::defineProperty creates two NapiClass objects back to back for a property descriptor with both a getter and a setter (NapiClass::create for the getter, then again for the setter). Each NapiClass::finishCreation opens and closes its own ThrowScope, so the second one's constructor aborts on the first one's simulated throw:

    This scope can throw a JS exception: finishCreation @ src/jsc/bindings/NapiClass.cpp:116
    But the exception was unchecked as of this scope: finishCreation @ src/jsc/bindings/NapiClass.cpp:116

That is the shape of every node-addon-api ObjectWrap accessor, and it is what was aborting Node's upstream js-native-api/6_object_wrap conformance test even with fix 1 applied.

Fix

For 1, replace the preamble's ThrowScope with a NapiBoundaryScope, a minimal JSC::ExceptionScope subclass for the C ABI boundary. It reads vm.exception() on entry (acknowledging the simulated throw left by a sibling napi_* call) and on exit (acknowledging inner scopes'), and never simulates a throw toward its native caller. NAPI_PREAMBLE_NO_THROW_SCOPE now declares the same boundary scope, covering entry points such as napi_get_and_clear_last_exception that construct an inner TopExceptionScope (whose constructor verifies) and would otherwise hit the identical abort.

This does not weaken validation inside Bun's own code: every DECLARE_THROW_SCOPE inside a N-API function body still verifies and simulates normally against its own nesting, NAPI_RETURN_SUCCESS still RELEASE_ASSERTs that no real exception is pending, and a real pending exception is still reported to the addon as napi_pending_exception. JSC's own C API makes the same choice at its boundary (DECLARE_TOP_EXCEPTION_SCOPE in JSObjectRef.cpp); TopExceptionScope itself is not usable here because it verifies the pending-check bit in its constructor. In release builds the class collapses to the trivial non-verification ExceptionScope, so there is no behavior or codegen change.

For 2, add the missing RETURN_IF_EXCEPTION checks in Napi::defineProperty: after the property-name lookup (which can genuinely throw via toPropertyKey) and after each NapiClass creation in the accessor branch.

Verification

test/napi/napi-exception-check.test.ts compiles two addons with the system C compiler against the in-repo N-API headers (no node-gyp, so it is a separate file from napi.test.ts and does not depend on that suite's toolchain setup) and require()s each in a child process with BUN_JSC_validateExceptionChecks=1:

• the canonical two-call Init (fix 1), and
• a napi_define_class with a getter+setter property (fix 2).

Without the src/ changes both abort (SIGABRT, exit 134) with the Unchecked JS exception reports above; with them, both load and run. With only fix 1 applied, the second test still aborted with the finishCreation pair, so each test is pinned to its own fix. On release builds the validator is compiled out, so the option is a no-op and the tests still pass.

Also ran, with BUN_JSC_validateExceptionChecks=1 against the fixed debug build: test/napi/napi-finalizer-delete-ref.test.ts and Node's upstream js-native-api/2_function_arguments, 3_callbacks, and 6_object_wrap suites (6_object_wrap aborted before fix 2), plus test/napi/napi.test.ts and the napi_get_value_string_utf8 / napi_define_class / napi_wrap groups without the validator.

Not addressed here

Some node-napi-tests entries still abort under the validator on a separate, pre-existing unchecked scope in the module-load path, before any napi_* call runs (for example js-native-api/4_object_factory):

    This scope can throw a JS exception: putInlineSlow @ JavaScriptCore/runtime/JSObject.cpp:836
    But the exception was unchecked as of this scope: Process_functionDlopen @ src/jsc/bindings/BunProcess.cpp:393

That is in the require/process.dlopen path, not in the N-API entry points, so it is left for a separate change, and the N-API entries in test/no-validate-exceptions.txt are intentionally not removed yet.

Rebase note

This branch was rebased over two upstream refactors of the same code and the fix was re-applied into their new shapes:

• NAPI_PREAMBLE on main now ends with NAPI_RETURN_IF_EXCEPTION (also checks the env-stashed napi_throw* exception); the rebased preamble keeps that behavior and declares a NapiBoundaryScope. The new third preamble macro, NAPI_PREAMBLE_NO_PENDING_CHECK, received the same boundary-scope conversion since it has the identical ThrowScope pattern.
• Napi::defineProperty was rewritten on main (now returns napi_status and uses a PropertyDescriptor). The upstream rewrite already checks after toPropertyKey, so that part of the original change is subsumed; the remaining back-to-back NapiClass::create calls in the accessor branch still lacked a check between them, which is re-applied with the new return value.

After the rebase, both tests still fail on main's napi.cpp (with the same napi_create_function/napi_set_named_property and finishCreation/finishCreation pairs at updated line numbers) and pass with the fix.

---

no test proof · iteration 4 · Platform-specific test(s) that do not run on this machine. Deferring to CI, which covers all platforms: test/napi/napi-exception-check.test.ts

[coderabbitai]

Walkthrough

NapiBoundaryScope replaces the N-API boundary throw scope in NAPI_PREAMBLE and NAPI_PREAMBLE_NO_THROW_SCOPE, and defineProperty gains extra exception checks around property-name and class creation steps. A new test suite compiles two native addons and verifies they load under BUN_JSC_validateExceptionChecks=1.

Changes

NapiBoundaryScope implementation and validation

Layer / File(s)	Summary
NapiBoundaryScope and preamble macros src/jsc/bindings/napi.cpp	Defines NapiBoundaryScope as a JSC::ExceptionScope subclass that reads vm.exception() on entry and exit without simulating throws, adds DECLARE_NAPI_BOUNDARY_SCOPE, and updates the NAPI preamble macros to use the new scope while keeping the existing boundary variable alive.
defineProperty exception checks src/jsc/bindings/napi.cpp	Adds RETURN_IF_EXCEPTION checks in Napi::defineProperty after deriving the property name and around NapiClass::create calls used for getter and setter functions.
N-API exception-check validation test test/napi/napi-exception-check.test.ts	Adds a test harness that discovers a C compiler, compiles temporary N-API addons with the in-repo headers, loads them through Bun with BUN_JSC_validateExceptionChecks=1, and asserts successful output without Unchecked JS exception in stderr. Skipped on Windows or when no compiler is found.

Suggested reviewers

• dylan-conway

🚥 Pre-merge checks | ✅ 4

✅ Passed checks (4 passed)

Check name	Status	Explanation
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Title check	✅ Passed	The title is concise and accurately summarizes the main N-API exception-check fix.
Description check	✅ Passed	The description thoroughly explains the bug, fix, and verification, though it uses different headings than the template.

---

_{Comment @coderabbitai help to get the list of available commands.}

[robobun]

^{Updated 7:49 PM PT - Jul 14th, 2026}

❌ @robobun, your commit d839d6a has 2 failures in Build #73079 (All Failures):

• test/js/third_party/grpc-js/test-tonic.test.ts - code 1 on 🍎 14 aarch64
• test/js/node/test/parallel/test-net-connect-memleak.js - code 1 on 🐧 3.23 x64
• test/js/node/test/parallel/test-net-connect-memleak.js - code 1 on 🐧 3.23 x64-baseline

---

🧪   To try this PR locally:

bunx bun-pr 32911

That installs a local version of the PR into your bun-32911 executable, so you can run:

bun-32911 --bun

[claude]

Thanks — both earlier points are addressed. No further issues from me, but the NapiBoundaryScope / ExceptionScope subclassing and the preamble macro changes touch every N-API entry point, so I'd like a human familiar with JSC's exception-scope machinery to sign off.

Extended reasoning...

Overview

This PR changes src/jsc/bindings/napi.cpp to introduce NapiBoundaryScope, a custom JSC::ExceptionScope subclass, and rewrites the NAPI_PREAMBLE / NAPI_PREAMBLE_NO_THROW_SCOPE macros to use it instead of DECLARE_THROW_SCOPE. These macros run at the entry of essentially every napi_* C ABI function. It also adds test/napi/napi-exception-check.test.ts, which compiles a minimal addon with the system C compiler and loads it under BUN_JSC_validateExceptionChecks=1.

Since my last review, commit 0718bfd addressed the remaining nit (stderr is now carried in the final assertion's received object so a regression's abort report shows in the failure diff). The earlier macOS -undefined dynamic_lookup fix is also in place. Both of my prior inline comments are resolved in the current diff.

Security risks

None identified. The change is confined to debug-build exception-scope verification bookkeeping; in release builds ENABLE(EXCEPTION_SCOPE_VERIFICATION) is off and NapiBoundaryScope collapses to the trivial base ExceptionScope. No new inputs are parsed and no trust boundaries change.

Level of scrutiny

High. While the rationale is well-argued and mirrors JSC's own TopExceptionScope precedent, this is a hand-rolled ExceptionScope subclass whose constructor/destructor interact with VM::m_needExceptionCheck, and the NAPI_PREAMBLE_NO_THROW_SCOPE macro changes shape from a do..while(0) block to a variable-declaring statement sequence across ~19 call sites. Getting the scope nesting or lifetime subtly wrong here could mask real exception-handling bugs across the entire N-API surface. This warrants review from someone with JSC internals knowledge rather than bot approval.

Other factors

• No CODEOWNERS entry covers src/jsc/bindings/napi.cpp.
• The bug-hunting system found no issues this round.
• The new test exercises the fix on debug builds and is a no-op pass on release builds, which is reasonable.
• napi_preamble_throw_scope__ keeps its name despite no longer being a ThrowScope; harmless but worth noting if a reviewer wants to rename it.

[coderabbitai]

Actionable comments posted: 3

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/jsc/bindings/napi.cpp`:
- Around line 82-87: The new explanatory comments around the N-API exception
scope are too long and exceed the repo’s 3-line comment limit. Trim the prose in
the block near the C ABI boundary in napi.cpp, keeping only the durable
invariant about reading vm.exception() on entry/exit and not simulating throws,
and apply the same shortening to the matching comment block later in the file.

In `@test/napi/napi-exception-check.test.ts`:
- Around line 35-76: The standalone validator-only N-API test is not reliable
because BUN_JSC_validateExceptionChecks=1 is a no-op on release/system Bun, so
it can miss the regression under USE_SYSTEM_BUN=1. Move this addon-load
assertion into the existing N-API coverage in the same test suite and have the
exception-check behavior exercised by the validator-enabled CI shard instead of
relying on a separate test. Keep the coverage anchored around the existing addon
load path in napi-exception-check.test.ts and the load.js/addon.node scenario,
but make sure the test still fails in the system Bun path when the boundary bug
is present.
- Around line 11-17: Shorten the rationale comment in the N-API exception check
test so it fits the 3-line comment limit while preserving the validator
invariant. Trim the explanatory block near the module init example in
napi-exception-check.test.ts, keeping only the key point about
napi_create_function and napi_set_named_property exception scopes and the
BUN_JSC_validateExceptionChecks=1 abort behavior. Use the existing test
description context to condense wording without changing the meaning.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 11fd28a6-5f1c-4972-a407-3028cd21829d

📥 Commits

Reviewing files that changed from the base of the PR and between 8706328 and 91b1d67.

📒 Files selected for processing (2)

• src/jsc/bindings/napi.cpp
• test/napi/napi-exception-check.test.ts

[claude]

Both earlier comments are addressed and I didn't find anything new, but this introduces a custom JSC::ExceptionScope subclass and rewires the preamble macros used by every N-API entry point — worth a human with JSC familiarity signing off.

Extended reasoning...

Overview

The PR replaces the DECLARE_THROW_SCOPE in NAPI_PREAMBLE with a new NapiBoundaryScope (a hand-rolled JSC::ExceptionScope subclass) and adds the same boundary scope to NAPI_PREAMBLE_NO_THROW_SCOPE, which previously declared no scope at all. A new test compiles a minimal addon with the system C compiler and loads it under BUN_JSC_validateExceptionChecks=1. Both of my earlier inline comments (macOS -undefined dynamic_lookup, and carrying stderr into the final assertion's received object) have been applied and resolved.

Security risks

None identified. This is debug-build exception-scope bookkeeping; in release builds ENABLE(EXCEPTION_SCOPE_VERIFICATION) is off and NapiBoundaryScope collapses to the trivial base. No new trust boundaries, parsing, or external input handling.

Level of scrutiny

High. NAPI_PREAMBLE / NAPI_PREAMBLE_NO_THROW_SCOPE are expanded at the top of essentially every napi_* entry point, and napi_preamble_throw_scope__ is consumed by NAPI_RETURN_IF_EXCEPTION, NAPI_RETURN_IF_VM_EXCEPTION, and NAPI_RETURN_SUCCESS (assertNoException()). Subclassing JSC::ExceptionScope directly and relying on specific constructor/destructor semantics (reading exception() to clear m_needExceptionCheck, deliberately not simulating a throw toward the native caller) is the kind of JSC-internals reasoning a human reviewer should validate — including the ~19 NAPI_PREAMBLE_NO_THROW_SCOPE callsites that now gain an outer scope and changed from a do {} while(0) statement to a bare declaration.

Other factors

The design rationale is thorough and cites JSC's own DECLARE_TOP_EXCEPTION_SCOPE precedent for the same boundary problem, the regression test is targeted and platform-aware, and the broader napi test suites were reportedly re-run. Nothing looks wrong to me; it's just not a mechanical change I'm comfortable approving without a human look at the JSC-side assumptions.

[claude]

No bugs found on this revision — my earlier two points are addressed. Leaving this for a human reviewer since it subclasses JSC::ExceptionScope and changes the exception-scope semantics at every N-API entry point, which is the kind of JSC-internals decision someone familiar with WebKit's exception discipline should sign off on.

Extended reasoning...

Overview

This PR replaces the DECLARE_THROW_SCOPE in NAPI_PREAMBLE / NAPI_PREAMBLE_NO_THROW_SCOPE with a new NapiBoundaryScope class that subclasses JSC::ExceptionScope. The new scope reads vm.exception() on entry and exit (clearing the validator's pending-check bit) but never simulates a throw toward its caller, so back-to-back napi_* calls from addon C code no longer trip JSC's exception-check validator. NAPI_PREAMBLE_NO_THROW_SCOPE is also unwrapped from a do { } while (0) block into open statements so the boundary scope outlives any inner ThrowScope. A new test compiles a minimal addon with the system C compiler and loads it under BUN_JSC_validateExceptionChecks=1.

Security risks

None identified. This is debug-build exception-scope bookkeeping; in release builds ENABLE(EXCEPTION_SCOPE_VERIFICATION) is compiled out and NapiBoundaryScope collapses to the trivial base ExceptionScope, so there is no codegen change there. No auth, crypto, permissions, or untrusted-input parsing is touched.

Level of scrutiny

High. The macro changed here runs at the top of every N-API entry point, and the fix works by subclassing a JSC-internal type and relying on specific behavior of ExceptionScope's constructor/destructor and the m_needExceptionCheck machinery. The reasoning in the PR description is detailed and matches my reading of JSC's ThrowScope/CatchScope/TopExceptionScope, and the downstream macros (RETURN_IF_EXCEPTION, assertNoException) only use ExceptionScope members so the type change is compatible. But this is exactly the kind of subtle runtime/VM-boundary design decision that a maintainer with JSC context should confirm rather than a bot.

Other factors

• My two earlier inline comments (macOS -undefined dynamic_lookup link flag; carrying stderr into the failure diff) were both addressed in e4e5a58 and 0718bfd.
• I checked all NAPI_PREAMBLE_NO_THROW_SCOPE call sites: each is a standalone statement at function-body scope, so dropping the do/while(0) wrapper doesn't change control flow anywhere.
• There is an unresolved CodeRabbit comment about the test being a no-op under USE_SYSTEM_BUN=1 / release builds; the PR description already explains this is by design (the validator is compiled out in release), but the author may want to respond or resolve it.
• No bugs surfaced in this review pass.

[claude]

All my earlier feedback is addressed and I found nothing further in feee1a6, but introducing a custom JSC::ExceptionScope subclass at the N-API boundary and swapping the scope type used by every napi_* preamble is the kind of JSC-internals design decision that should get a human look.

Extended reasoning...

Overview

This PR touches two files: src/jsc/bindings/napi.cpp and a new test test/napi/napi-exception-check.test.ts. The substantive change introduces NapiBoundaryScope, a minimal JSC::ExceptionScope subclass that acknowledges pending exception checks on entry/exit without simulating a throw toward its (native C addon) caller, and uses it in NAPI_PREAMBLE / NAPI_PREAMBLE_NO_THROW_SCOPE in place of the previous ThrowScope. It also adds RETURN_IF_EXCEPTION checks between back-to-back NapiClass::create calls in Napi::defineProperty. The new test compiles two tiny addons with cc and loads them under BUN_JSC_validateExceptionChecks=1.

Security risks

None identified. The change is to debug-build exception-scope bookkeeping; in release builds ENABLE(EXCEPTION_SCOPE_VERIFICATION) is compiled out and NapiBoundaryScope collapses to the trivial base ExceptionScope, so there is no release-observable behavior change. The added RETURN_IF_EXCEPTION calls in defineProperty are strictly safer (early-return on a real pending exception rather than proceeding). No auth, crypto, permissions, or untrusted-input parsing is involved.

Level of scrutiny

Medium-high. While release behavior is unchanged, NAPI_PREAMBLE is expanded at the top of every napi_* entry point, and NapiBoundaryScope is a new bespoke participant in JSC's ExceptionScope/ThrowScope/CatchScope hierarchy. The PR description's reasoning (mirroring JSC's own DECLARE_TOP_EXCEPTION_SCOPE choice at the C API boundary, and why TopExceptionScope itself doesn't fit) is sound and I verified the downstream macro uses (RETURN_IF_EXCEPTION, .assertNoException()) are all base-ExceptionScope methods, so the type swap is compatible. But validating that this is the right design — versus, say, a CatchScope or some other JSC idiom — is a judgment call for someone who owns the JSC bindings.

Other factors

I left four rounds of feedback on this PR (macOS link flags, stderr-in-failure-diff, test.concurrent, async Bun.spawn), all of which the author addressed; CodeRabbit's comments are likewise all resolved. The bug-hunting pass on feee1a6 found nothing. The NAPI_PREAMBLE_NO_THROW_SCOPE macro changed shape from a do { } while(0) statement to a variable declaration, but that's intentional (the boundary scope must outlive any inner ThrowScope) and it's only ever used at function-body start. CI build #66255 is in progress for the head commit.

[robobun]

CI note for reviewers: the red lanes on this PR's builds are not related to the diff.

On build 73079 (head d839d6a, after the rebase), 283 jobs passed and 3 failed:

• alpine 3.23 x64 and x64-baseline test-bun: test/js/node/test/parallel/test-net-connect-memleak.js (retried four times on both lanes), plus the agent's docker coordinator failing to start the MySQL test services. The most recent completed main build (73064) passed these lanes.
• darwin 14 aarch64 test-bun (one shard; its sibling shard passed): test/js/bun/test/test-retry-repeats-basic.test.ts and test/js/third_party/grpc-js/test-tonic.test.ts. The test-retry-repeats-basic failure has recurred on a darwin lane in every prior build of this PR.

Both tests added in this PR, test/napi/napi-exception-check.test.ts, passed on all five darwin test shards (darwin 26 aarch64 both shards, darwin 14 x64 both shards, and the darwin 14 aarch64 shards), on the linux x64 ASAN shard, and everywhere else they ran.

Earlier builds of this PR were red only on the same categories: a buildkite artifact-download timeout that ran zero tests, test/cli/install/GHSA-pfwx-36v6-832x.test.ts hitting a live codeload.github.com URL returning 400, test/cli/hot/hot.test.ts, test-retry-repeats-basic.test.ts, the alpine docker-services failure, and test/cli/install/bun-install.test.ts on Windows.

This change compiles to a no-op in release builds, which is what every CI test lane runs, so it cannot affect those failures. The napi suites (including Node's upstream js-native-api tests) were run locally against the debug build with BUN_JSC_validateExceptionChecks=1 as described in the PR body. Ready for review.

[claude]

All four of my earlier comments are addressed and I didn't find anything further, but I'm leaving this for a human reviewer: introducing a custom JSC::ExceptionScope subclass and swapping the scope type in all three NAPI_PREAMBLE* macros is a design-level change to how the N-API boundary interacts with JSC's exception-check machinery, and a maintainer should sign off on that choice.

Checked: NAPI_PREAMBLE_NO_THROW_SCOPE losing its do..while wrapper is safe — every call site is a plain top-of-function statement. RETURN_IF_EXCEPTION / assertNoException() on the boundary scope resolve via the ExceptionScope base. The defineProperty accessor branch now checks after each NapiClass::create; the method branch was examined and doesn't need one (single create, existing check follows).

Extended reasoning...

Overview

The PR touches two files: src/jsc/bindings/napi.cpp (introduces NapiBoundaryScope, a JSC::ExceptionScope subclass; rewrites the three NAPI_PREAMBLE* macros to declare it instead of a ThrowScope; adds RETURN_IF_EXCEPTION between the getter/setter NapiClass::create calls in Napi::defineProperty) and a new test file test/napi/napi-exception-check.test.ts that compiles two minimal C addons with the system cc and loads them under BUN_JSC_validateExceptionChecks=1.

Security risks

None identified. The change is to debug-build exception-scope bookkeeping; in release builds ENABLE(EXCEPTION_SCOPE_VERIFICATION) is off and NapiBoundaryScope collapses to the same trivial ExceptionScope wrapper that ThrowScope does, so there is no release codegen or behavior change. No new user input parsing, no auth/crypto/permissions surface.

Level of scrutiny

High. The NAPI_PREAMBLE* macros gate essentially every napi_* entry point, and the fix works by subclassing a JSC internal (ExceptionScope) with custom constructor/destructor semantics that deliberately deviate from ThrowScope (read vm.exception() on entry/exit, never simulateThrow()). The PR description argues this mirrors JSC's own C-API boundary (DECLARE_TOP_EXCEPTION_SCOPE) and explains why TopExceptionScope itself isn't usable. That reasoning looks sound to me, but it is exactly the kind of JSC-internals design call a maintainer familiar with the exception-scope discipline should confirm — including whether assertNoException() on a non-throw scope in NAPI_RETURN_SUCCESS is the intended contract, and whether removing simulateThrow() toward the caller has any downside for the (non-addon) internal callers of these entry points.

Other factors

I left four inline comments over the PR's lifetime (macOS -undefined dynamic_lookup, stderr in the failure diff, test.concurrent, async spawn for the compile step) — all addressed. CodeRabbit's comment-length nits were addressed; its "must fail under USE_SYSTEM_BUN" objection was withdrawn after the author explained the validator is compiled out of release builds. The bug hunter raised and refuted whether the method branch of defineProperty needs the same check (it doesn't; only one NapiClass::create there). I additionally verified that dropping the do..while(0) from NAPI_PREAMBLE_NO_THROW_SCOPE doesn't break any call site (all ~18 uses are plain statements at function-body top level), and that the macros referencing napi_preamble_throw_scope__ (NAPI_RETURN_IF_EXCEPTION, NAPI_RETURN_SUCCESS, etc.) still compile against a NapiBoundaryScope since those members live on the ExceptionScope base. The PR was also rebased over two upstream refactors of the same code, which is another reason a human familiar with those refactors should take a look.

[robobun]

Closing as part of a cleanup of stale pull requests. This PR has had no new commits since 2026-07-15, it conflicts with main, and its last CI run failed. This is not a judgment on the fix itself. If the problem still reproduces on a current build, reopen this PR after a rebase or open a new one against main.