napi: guard string creators and buffer accessors against pending VM exception

[robobun]

Repro

napi_throw(env, err);                         // E stashed on env
napi_call_function(env, g, fn, 0, NULL, &r);  // returns 10, E now in vm.m_exception
napi_create_string_utf8(env, "x", 1, &out);   // debug/assert: SIGABRT

ASSERTION FAILED: Unexpected exception observed ...
!exception()
ExceptionScope.h(62) : void JSC::ExceptionScope::releaseAssertNoException()

Release builds return napi_ok and keep running; the assertion is compiled out there. The same abort is reachable without an addon throw via napi_create_bigint_words past the engine cap (throws a RangeError into the VM, returns 10), then any of the eight calls below.

Cause

The Rust napi_create_string_{utf8,latin1,utf16} and napi_{is_arraybuffer,get_arraybuffer_info,get_typedarray_info,get_dataview_info,get_buffer_info} entry points used the bare get_env! prologue and then called into C++ helpers (BunString__transferToJS / BunString__createUTF8ForJS / JSC__JSValue__asArrayBuffer) whose exception-validation scope asserts the VM has no pending exception when they return a value. With vm.m_exception already set, that assertion fails.

Fix

Add preamble_no_pending_check! in napi_body.rs (the Rust mirror of NAPI_PREAMBLE_NO_PENDING_CHECK): it returns napi_pending_exception when vm.m_exception is set but leaves an env-stashed napi_throw* exception alone. Use it at all eight sites.

Only the VM slot is gated because (a) a stashed exception never reaches the asserting helpers, and (b) Node.js allows these calls with a stashed exception pending: the existing test_deferred_exceptions uses napi_create_string_utf8 immediately after a ThrowAsJavaScriptException() and expects napi_ok, via checkSameOutput. The fuller preamble! (which also gates on the stash) would regress that. With a VM exception pending Node.js still returns napi_ok from these eight; Bun now returns napi_pending_exception, which joins the existing status divergence shared with napi_create_object et al rather than aborting.

Test

test_vm_pending_exception_no_abort in test/napi/napi-app/standalone_tests.cpp arms a VM exception via napi_throw + napi_call_function, calls all eight entry points, and asserts the original Error: E1 is still pending and catchable afterwards. It then re-arms via the napi_create_bigint_words overflow route and calls napi_create_string_utf8 again. Output matches Node byte-for-byte via checkSameOutput.

Fail-before (src/ stashed, bun bd test test/napi/napi.test.ts -t 'does not abort when a VM exception'): aborts with releaseAssertNoException, exit 134.
After: passes; test_deferred_exceptions and test_pending_exception_gate unchanged.

Related: #36091 takes the alternative node-parity approach (suspend the exception and return napi_ok) for the three string creators only.

---

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

[coderabbitai]

Warning

Review limit reached

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

Next review available in: 28 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: 9d559aea-9c3e-47e6-a64d-f0a728d823a9

📥 Commits

Reviewing files that changed from the base of the PR and between 4eb6f99 and c43ba8b.

📒 Files selected for processing (4)

• src/jsc/bindings/napi.cpp
• src/runtime/napi/napi_body.rs
• test/napi/napi-app/standalone_tests.cpp
• test/napi/napi.test.ts

---

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

[github-actions]

Found 1 issue this PR may fix:

1. panic: Error::New napi_get_last_error_info #20663 - napi-rs's Error::New calls napi_create_string_utf8 internally; if a VM exception is already pending, this triggers the exact releaseAssertNoException abort that this PR guards against

If this is helpful, copy the block below into the PR description to auto-close this issue on merge.

Fixes #20663

🤖 Generated with Claude Code

[github-actions]

This PR may be a duplicate of:

1. napi: tolerate pending VM exception in napi_create_string_* (debug/asan abort) #36091 - Also tolerates pending VM exceptions in napi_create_string_* functions; PR napi: guard string creators and buffer accessors against pending VM exception #36095 is a superset that extends the fix to buffer accessors
2. napi: tolerate pending VM exception in ArrayBuffer/typed-array info accessors (debug/asan abort) #36093 - Guards ArrayBuffer/typed-array info accessors against pending VM exceptions, which PR napi: guard string creators and buffer accessors against pending VM exception #36095 also covers

🤖 Generated with Claude Code

[robobun]

On the overlap with #36091 and #36093: those two together cover the same eight entry points using JSC::SuspendExceptionScope, so they return napi_ok and match Node exactly. This PR takes the prologue route instead: return napi_pending_exception when vm.m_exception is set, which is the smaller change but leaves Bun diverging from Node on the return status for those eight calls (same divergence napi_create_object et al already have under NAPI_PREAMBLE_NO_PENDING_CHECK). Either approach resolves the abort; they're mutually exclusive.

[robobun]

^{Updated 6:53 AM PT - Jul 27th, 2026}

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

• test/js/bun/http/serve.test.ts - code 1 on 🍎 14 x64
• test/js/bun/http/serve.test.ts - code 1 on 🍎 14 aarch64

---

🧪   To try this PR locally:

bunx bun-pr 36095

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

bun-36095 --bun

[claude]

No issues found with the implementation itself, but this and #36091/#36093 are mutually-exclusive fixes for the same abort — prologue-gate (returns napi_pending_exception, diverging from Node) vs. SuspendExceptionScope (Node-parity napi_ok). A maintainer should pick which approach lands.

What was reviewed:

• NapiEnv__hasVMException / preamble_no_pending_check! correctly mirror the existing C++ NAPI_PREAMBLE_NO_PENDING_CHECK (DECLARE_TOP_EXCEPTION_SCOPE + NAPI_RETURN_IF_VM_EXCEPTION).
• The test only prints "survived" (not status codes), so checkSameOutput holds despite the status divergence; the bigint-overflow route is a no-op on Node so that line matches too.
• My earlier napi_create_array concern was refuted empirically and is now pinned in the test.

Extended reasoning...

Overview

Adds a Rust-side mirror of the C++ NAPI_PREAMBLE_NO_PENDING_CHECK macro: a new NapiEnv__hasVMException FFI (napi.cpp), has_vm_exception() wrapper, and preamble_no_pending_check! macro in napi_body.rs. Eight Rust NAPI entry points (napi_create_string_{utf8,latin1,utf16}, napi_is_arraybuffer, napi_get_{arraybuffer,typedarray,dataview,buffer}_info) swap get_env! for the new macro so they return napi_pending_exception instead of aborting on the JSC exception-validation assert when vm.m_exception is already set. A new test_vm_pending_exception_no_abort in the napi test addon exercises all eight plus napi_create_array{,_with_length} via checkSameOutput.

Security risks

None. This is a defensive early-return in NAPI entry points; no new attack surface, no untrusted input parsing, no memory ownership changes.

Level of scrutiny

Medium-high. The change is small and mechanically follows an established in-tree pattern (the C++ NAPI_PREAMBLE_NO_PENDING_CHECK at napi.cpp:113), but it touches exception-handling semantics in the NAPI layer and deliberately diverges from Node's return status for these eight functions. The correctness of the change itself is straightforward; the design question — whether Bun should return napi_pending_exception here (this PR) or suspend the exception and return napi_ok like Node (#36091/#36093) — is a maintainer-level decision.

Other factors

• Two open competing PRs (#36091, #36093) take the alternative SuspendExceptionScope approach; the author has already flagged them as mutually exclusive in the thread.
• My prior inline finding on napi_create_array was refuted with a traced explanation (constructEmptyArray hits RETURN_IF_EXCEPTION and returns null, so call_zero_is_throw takes the Err branch) and the author added those two calls to the test to pin the behavior.
• The test is carefully constructed to produce identical output on Node and Bun despite differing status codes (it prints "survived" rather than the status), and cleans up the pending exception at the end.
• The comment-cop bot flags on the doc comments were resolved; the remaining doc comments are the same length/style as neighboring has_pending_exception / preamble! docs.
• PR description reports fail-before/pass-after but notes "no test proof" locally (deferred to CI), so CI results should be checked before merge.

[robobun]

CI #83339: the only hard failure is test/js/bun/http/serve.test.ts "releases a paused request body when the handler responds without reading it" on darwin 14 x64, which also fails on main build #83238 (base commit 4eb6f99) and is unrelated to this NAPI change. test/napi/napi.test.ts passes on every lane. Ready for review.