napi: check exceptions between NapiClass::create and defineOwnProperty

[robobun]

Repro

Any N-API addon whose Init calls napi_define_class or napi_define_properties with a method, getter, or setter descriptor aborts under BUN_JSC_validateExceptionChecks=1:

NAPI_MODULE_INIT() {
  napi_property_descriptor prop = {"m", NULL, Method, NULL, NULL, NULL, napi_default, NULL};
  napi_value cls;
  napi_define_class(env, "Thing", NAPI_AUTO_LENGTH, Ctor, NULL, 1, &prop, &cls);
  return cls;
}

ERROR: Unchecked JS exception:
    This scope can throw a JS exception: finishCreation @ src/jsc/bindings/NapiClass.cpp:120
        (ExceptionScope::m_recursionDepth was 12)
    But the exception was unchecked as of this scope: defineOwnNonIndexProperty @ JavaScriptCore/runtime/JSObject.cpp:4062
        (ExceptionScope::m_recursionDepth was 12)
ASSERTION FAILED: exception check validation failed

Hit by every test/napi/node-napi-tests/test/js-native-api/* suite that defines a class or a function property (33 entries in the # 3rd party napi section of test/no-validate-exceptions.txt).

Cause

Napi::defineProperty (shared by napi_define_class and napi_define_properties) builds method/getter/setter descriptors by calling NapiClass::create, which opens and closes its own ThrowScope in finishCreation. On return that scope's destructor simulates a throw. The very next call is defineOwnProperty, which enters defineOwnNonIndexProperty and constructs a new ThrowScope; its constructor finds the unsatisfied check and RELEASE_ASSERTs. This surfaced after #34134 routed every descriptor kind through defineOwnProperty.

Fix

Add RETURN_IF_EXCEPTION(scope, napi_pending_exception) after each of the three NapiClass::create calls in Napi::defineProperty (getter, setter, method), so the exception state is observed before defineOwnProperty. The same pattern is already used after toPropertyKey and after defineOwnProperty in the same function.

Verification

test/napi/napi-define-property-exception-check.test.ts compiles two tiny C addons with the system cc against the in-repo headers (no node-gyp) and require()s each in a child process with BUN_JSC_validateExceptionChecks=1: one exercising napi_define_class with method/getter/setter/accessor properties, one exercising napi_define_properties with the same shapes. Without the fix both abort (exit 134) with the finishCreation/defineOwnNonIndexProperty report above; with it, both load cleanly. On release builds the validator is compiled out, so the option is a no-op and the tests pass trivially; the gate runs a debug build.

Also ran against the fixed debug build: test/napi/node-napi-tests/test/js-native-api/{6_object_wrap,test_constructor,test_properties}/do.test.ts (9 pass) and test/napi/napi.test.ts -t napi_define (the [[DefineOwnProperty]] test from #34134 still passes).

Relationship to #32911

#32911 (open) addresses the broader question of addon C code not being able to satisfy JSC's exception-check discipline between consecutive napi_* calls, via a NapiBoundaryScope at the preamble level, and as a secondary fix adds the getter/setter checks here (but not the method one). This PR is the narrow Napi::defineProperty half of that and stands on its own: a single napi_define_class call is enough to abort, regardless of what the addon does before or after it. The no-validate-exceptions.txt entries are intentionally left in place since most of them still need the preamble-side fix from #32911 (or the dlopen path fix it mentions) before the validator can be enabled for them.

[robobun]

^{Updated 7:21 PM PT - Jul 19th, 2026}

❌ @robobun, your commit f140d3e has 1 failures in Build #75915 (All Failures):

• test/js/node/test/parallel/test-net-connect-memleak.js - code 1 on 🐧 3.23 x64-baseline
• test/js/node/test/parallel/test-net-connect-memleak.js - code 1 on 🐧 3.23 x64

---

🧪   To try this PR locally:

bunx bun-pr 34750

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

bun-34750 --bun

[robobun]

Reproduced locally on a debug+ASAN build:

$ git stash push -- src/ && bun bd test test/napi/napi-define-property-exception-check.test.ts
(fail) napi_define_class with method/getter/setter runs under BUN_JSC_validateExceptionChecks
  exitCode: 134
  stderr: finishCreation @ NapiClass.cpp:120 ... defineOwnNonIndexProperty @ JSObject.cpp:4062
(fail) napi_define_properties with method/getter/setter runs under BUN_JSC_validateExceptionChecks
  exitCode: 134

$ git stash pop && bun bd test test/napi/napi-define-property-exception-check.test.ts
(pass) napi_define_class with method/getter/setter runs under BUN_JSC_validateExceptionChecks
(pass) napi_define_properties with method/getter/setter runs under BUN_JSC_validateExceptionChecks

CI (build #75915): the new test passed on every lane. Remaining red is unrelated to this diff: test-net-connect-memleak.js on alpine x64 is a pre-existing main failure, and the two darwin 14 aarch64 lanes never booted (guest never up after retry / The number of VMs exceeds the system limit). Ready for review.

[coderabbitai]

Warning

Review limit reached

@robobun, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 19 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 1aa04e1f-a7c9-4dc9-9506-7854a0f2f54d

📥 Commits

Reviewing files that changed from the base of the PR and between 99fc2f8 and f140d3e.

📒 Files selected for processing (2)

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

---

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

[claude]

LGTM — three mechanical RETURN_IF_EXCEPTION insertions matching the pattern already used elsewhere in the same function.

What was reviewed:

• Confirmed NapiClass::finishCreation declares a ThrowScope (NapiClass.cpp:120), so the check is required before the subsequent defineOwnProperty.
• Checked the other NapiClass::create call sites: napi_define_class already has NAPI_RETURN_IF_EXCEPTION; napi_create_function has no ThrowScope-constructing call between create and return, so it doesn't trip the validator.
• Test drains pipes concurrently, skips when cc is unavailable / on Windows, doesn't assert exact stderr, and asserts addon-defined properties without depending on loader-added ones.
• The lone CI failure (test-net-connect-memleak.js) is unrelated to N-API.

Extended reasoning...

Overview

This PR adds three RETURN_IF_EXCEPTION(scope, napi_pending_exception) calls in Napi::defineProperty (src/jsc/bindings/napi.cpp), one after each NapiClass::create call for getter, setter, and method descriptors. It also adds a new test file that compiles minimal C addons against the in-repo N-API headers and loads them under BUN_JSC_validateExceptionChecks=1 to prove the validator no longer aborts.

Security risks

None. This is purely JSC exception-scope discipline in the N-API property definition path — no user input parsing, no auth/crypto, no boundary validation changes.

Level of scrutiny

Low. The native change is three identical one-liners that mirror the exact pattern already used at napi.cpp:345 (after toPropertyKey) and napi.cpp:389 (after defineOwnProperty) in the same function. NapiClass::finishCreation at NapiClass.cpp:120 does declare a ThrowScope, so the checks are correctly placed. The return value napi_pending_exception matches every other exception-check site in this function.

Other factors

• I checked for sibling NapiClass::create sites that might need the same treatment. napi_define_class (napi.cpp:2179) already has NAPI_RETURN_IF_EXCEPTION immediately after. napi_create_function (napi.cpp:969) does not, but there is no ThrowScope-constructing call between create and NAPI_RETURN_SUCCESS, so the validator cannot trip there — and the PR description explicitly scopes the broader boundary-scope work to #32911.
• The test follows harness conventions well: tempDir, bunEnv spread, concurrent pipe drain, test.concurrent, skipIf(isWindows || !cc), and it asserts {stdout, exitCode} via toMatchObject while including stderr in the received object for diagnostics without asserting it exactly (debug builds emit noise). The second test intentionally filters to addon-defined own properties rather than asserting the full set, avoiding coupling to loader implementation details.
• The one CI failure so far is test/js/node/test/parallel/test-net-connect-memleak.js, which touches nothing in the N-API path and is clearly unrelated flake.
• The PR description demonstrates the test fails without the fix (exit 134) and passes with it, satisfying the "fails for the right reason" requirement.