node:module: check for an exception after wrapping a runMain override's return value

[robobun]

Problem

• A --require preload that replaces Module.runMain with a function that does not call the original makes bun wrap the override's return value in a promise. That wrap is never checked for an exception. On a debug build this aborts under JSC's exception-check validator with the existing test fixtures as-is:

  cd test/js/node/module
  BUN_JSC_validateExceptionChecks=1 bun-debug --require ./overwrite-module-run-main-3.cjs ./overwrite-module-run-main-2.cjs
  ERROR: Unchecked JS exception:
      This scope can throw a JS exception: promiseResolve @ vendor/WebKit/Source/JavaScriptCore/runtime/JSPromise.cpp:1154
      But the exception was unchecked as of this scope: <rust> @ src/jsc/VirtualMachine.rs:607
  ASSERTION FAILED: exception check validation failed
  

• When the wrap really does throw, release builds crash instead. An override returning a promise whose constructor getter throws gives panic: opaque_deref: null FFI handle (bun 1.4.0 canary, linux x64), because the null result is stored as the entry point promise and then dereferenced in load_entry_point / Run::start.
• Cause: JSC__JSInternalPromise__resolvedPromise (src/jsc/bindings/bindings.cpp:4072) is a bare wrapper over JSC::JSPromise::resolvedPromise, which runs promiseResolve and returns null if it throws. Its only caller, the has_patched_run_main branch of reload_entry_point (src/jsc/VirtualMachine.rs:2714), used the result directly. The override call two lines above it is wrapped in from_js_host_call_generic; this call was not.
• CI does not see it: the ASAN lanes skip the validator for test/js/node/module/node-module-module.test.js (test/no-validate-exceptions.txt), and the Node fixture monkey-patch-run-main.js calls through to the original runMain, which takes the other branch.

Fix

• Mark JSC__JSInternalPromise__resolvedPromise [[ZIG_EXPORT(check_slow)]], matching the JSC__JSInternalPromise__resolve / __reject bindings next to it, and have reload_entry_point call the generated crate::cpp:: wrapper, which opens a scope around the call and returns Err when an exception is pending. The hand-written extern "C" declaration goes away (src/jsc/cpp.rs asks new code to use the generated wrappers instead of redeclaring).
• The error is mapped to CrateError::JSError, exactly like the override call on the line above, so a throw while adopting the return value is reported the same way as a throw inside the override (Error occurred loading entry point, exit 1) instead of a crash. Making that report print the underlying error, and a non-callable runMain override crashing, are separate defects being handled separately; both sites will pick up whatever that change does.
• Why check_slow rather than null_is_throw: JSC's own callers of JSPromise::resolvedPromise do RETURN_IF_EXCEPTION and then trust the pointer (JSPromiseConstructor.cpp, JSWebAssembly.cpp). check_slow is that exact shape, and it also covers the case where promiseResolve returns a promise but leaves a termination exception pending. Once the wrapper returns Ok, the pointer is non-null by the same contract JSC relies on.
• Non-throwing behaviour is unchanged: the same promise is created and stored; the only addition is the exception check.
• Verified with test/js/node/module/node-module-module.test.js:
  • Module.runMain / Module.runMain 2 now run their fixtures under BUN_JSC_validateExceptionChecks=1 and assert empty stderr.
  • New Module.runMain override returning ... table covers each way promiseResolve can go: a plain value, a pending native promise, a fulfilling thenable, a rejecting thenable (error reported, exit 1), and a promise whose constructor getter throws (exit 1, no crash).
  • Unfixed debug build: 6 of the 7 fail (validator abort with the report above; the constructor getter case crashes). Fixed debug build: all 7 pass, whole file 44 pass.
  • test/js/node/test/parallel/test-module-run-main-monkey-patch.js passes on the fixed build, with and without the validator.
  • The file stays in test/no-validate-exceptions.txt: running it under the validator still trips the Module.wrap site owned by node:module: check exceptions from jsString/jsSubstring in Module.wrap and new Module() #34745 (and node:module: check exceptions from the parent lookup in the native require resolver #38099 covers the _resolveFilename path). Once those land the entry can be dropped.
  • BUN_JSC_validateExceptionChecks is a no-op on release builds, so there the new table only adds the constructor getter case as a before/after difference.

Background

• runMain override path: during preloads, assigning Module.runMain sets vm.has_patched_run_main. reload_entry_point then calls the override instead of loading the entry itself. If the override called the original runMain, that stored the module promise as pending_internal_promise; otherwise bun adopts the override's return value with Promise.resolve semantics and stores that, so an async override's rejection is reported as the entry point failing.
• promiseResolve: the spec's PromiseResolve. For a promise argument it reads .constructor (observable user code, can throw) to decide whether to return the promise as-is; a thenable is adopted via a microtask; anything else becomes a fulfilled promise. A throwing then getter does not throw here (it rejects the new promise); the constructor getter is the throwing case.
• Exception-check validator (BUN_JSC_validateExceptionChecks=1, debug builds only): every JSC throw scope records on destruction that its caller owes an exception check; creating the next scope before that check happens aborts and prints both locations. That is why even the non-throwing plain-value fixture aborts: nothing between the wrap and the next scope ever looked at the exception state.
• [[ZIG_EXPORT(mode)]]: attribute read by src/codegen/cppbind.ts, which generates a Rust wrapper in bun_jsc::cpp for each annotated C++ function. check_slow means the return value says nothing about whether the call threw, so the wrapper opens a TopExceptionScope, makes the call, and converts a pending exception into Err(JsError::Thrown); null_is_throw instead asserts that a null return and a pending exception always coincide.

Behaviour of each return shape on the unfixed build (debug + validator unless noted)

override returns	unfixed	fixed
plain value (overwrite-module-run-main-3.cjs)	validator abort at promiseResolve	exit 0
async () => { await 0; ... } (pending native promise)	validator abort	exit 0
fulfilling thenable	validator abort	then runs, exit 0
rejecting thenable	validator abort	error printed, exit 1
promise with throwing constructor getter	panic: opaque_deref: null FFI handle (release too)	Error occurred loading entry point, exit 1
already-rejected promise, or thenable whose then getter throws	error printed, exit 1 (passes by accident: load_entry_point returns early on an already-rejected promise and the reporting path checks the exception state)	unchanged
override calls the original runMain (overwrite-module-run-main-1.cjs, Node's monkey-patch-run-main.js)	clean, other branch	unchanged

[robobun]

Status: fix and tests pushed, waiting on CI.

Reproduced on the unfixed build two ways:

• debug build + BUN_JSC_validateExceptionChecks=1 on the existing overwrite-module-run-main-3.cjs fixture: validator abort at promiseResolve (report in the PR body)
• release and debug builds with an override returning a promise whose constructor getter throws: panic: opaque_deref: null FFI handle

With the fix, bun bd test test/js/node/module/node-module-module.test.js passes (44 pass); the 6 runMain cases that fail on the unfixed build pass.

[coderabbitai]

Warning

Review limit reached

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

Next review available in: 4 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: 35f874f3-d819-494a-82da-7baa227c9bdb

📥 Commits

Reviewing files that changed from the base of the PR and between b7a0431 and 71f5d7b.

📒 Files selected for processing (3)

• src/jsc/VirtualMachine.rs
• src/jsc/bindings/bindings.cpp
• test/js/node/module/node-module-module.test.js

---

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

[claude]

LGTM — targeted exception-check fix that mirrors the sibling __resolve/__reject bindings.

What was reviewed:

• Confirmed JSC__JSInternalPromise__resolvedPromise's only caller is the one updated in reload_entry_point; the removed hand-written extern has no other users.
• Verified the generated check_slow wrapper's return type (JsResult<*mut JSPromise>) is compatible with pending_internal_promise: Option<*mut JSInternalPromise> — JSInternalPromise is a re-export alias of JSPromise (src/jsc/lib.rs:206).
• Error mapping to CrateError::JSError matches the override call two lines above; the throwing case now exits 1 instead of null-derefing.
• New tests drain both pipes concurrently, assert the combined {stdout, stderr, exitCode} object, and cover each promiseResolve outcome including the throwing .constructor getter.

Extended reasoning...

Overview

Three files changed. In bindings.cpp, JSC__JSInternalPromise__resolvedPromise gains [[ZIG_EXPORT(check_slow)]] (plus a two-line comment), so cppbind.ts generates a crate::cpp:: wrapper that opens a top_scope!, calls the extern, and returns Err(JsError::Thrown) if an exception is pending. In VirtualMachine.rs, the hand-written extern "C" declaration is removed and the single call site in the has_patched_run_main branch of reload_entry_point switches to the generated wrapper with .map_err(|_| CrateError::JSError)?. In node-module-module.test.js, the two existing Module.runMain subprocess tests now set BUN_JSC_validateExceptionChecks=1 and assert empty stderr, and a new five-entry test.each covers plain values, native promises, fulfilling/rejecting thenables, and a promise with a throwing constructor getter.

Security risks

None. The change adds an exception check where one was missing; no new user-controlled data reaches native code, and no validation is loosened.

Level of scrutiny

Low-to-moderate. The fix is mechanical: it applies the same check_slow attribute already on the adjacent JSC__JSInternalPromise__resolve / __reject bindings (bindings.cpp:4032, 4066) and routes through the codegen'd wrapper the way src/jsc/cpp.rs documents. The only behaviour change is that a pending exception after promiseResolve now returns CrateError::JSError instead of storing a null pointer that later panics in opaque_deref. I checked that JSInternalPromise is pub use JSPromise as JSInternalPromise (src/jsc/lib.rs:206), so the generated *mut JSPromise return type is assignable to pending_internal_promise without a cast. The removed extern had exactly one caller (grep confirms only bindings.cpp, headers.h, and this call site reference the symbol).

Other factors

The tests follow harness conventions well: tempDir for fixtures, await using on the spawned process, concurrent Promise.all drain of stdout/stderr/exited, and a single combined-object assertion. The validateExceptions env is a no-op on release builds (per the comment and PR description), so release CI still exercises the throwing-constructor case as a real behaviour change while debug lanes additionally guard against validator regressions. The PR description explains why check_slow was chosen over null_is_throw (matches JSC's own callers of resolvedPromise, and covers termination exceptions), which was the one non-obvious choice.

[robobun]

^{Updated 7:45 AM PT - Aug 13th, 2026}

✅ @robobun, your commit 71f5d7b07e6048532f95300e893d3e0170e4b0ad passed in Build #94400! 🎉

---

🧪   To try this PR locally:

bunx bun-pr 38113

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

bun-38113 --bun