node:module: report a rejecting runMain override result once

[robobun]

Problem

• A --require preload that replaces Module.runMain with a function returning a promise (or thenable) that is still pending when it returns and rejects later gets the rejection reported twice: the error: ... block is printed twice before exiting 1, and with listeners installed both unhandledRejection and uncaughtException fire for the same error.
• An override whose promise is already rejected when it returns, and a plain entry point that throws after a top-level await, are reported once.
• Cause: reload_entry_point (src/jsc/VirtualMachine.rs:2714) adopts the override's return value with JSPromise::resolvedPromise and stores it as pending_internal_promise, which Run::start (src/runtime/cli/run_command.rs:1434) reports once it is rejected. Every other promise stored in that field comes from the module loader, which creates them marked as handled. The adopted promise was not, so when it rejected during the event loop spin in load_entry_point, the rejection tracker reported it at the end of that tick, and then Run::start reported it again.
• The same store is reached by a worker whose preload option overrides runMain; there the worker's own entry check (src/jsc/web_worker.rs, observe_entry, which also relies on the promise being handled) is the second reporter, so both listeners fire inside the worker as well.
• Repro:

  // preload.cjs
  require("module").runMain = async () => { await 0; throw new Error("boom"); };

  bun --require ./preload.cjs ./main.cjs prints error: boom twice. Same with () => ({ then(_, reject) { reject(new Error("boom")); } }).

Fix

• Mark the adopted promise as handled at the point it is stored as the entry point promise, so the tracker never sees it and Run::start stays the single reporter.
• Correct because the runtime does consume this promise: it waits for it and reports its rejection as the entry point failing. That is the same contract the module loader's promises already have (JSC marks them handled on creation for the same reason), and it is what Run::start does to the promise after reporting it anyway. The result is the already-rejected case and the late-rejecting case now behave identically: printed once, delivered once to uncaughtException listeners with origin unhandledRejection, and a passthrough override that calls the original runMain is unaffected (it stores the loader's promise, which was already handled).
• Behaviour change for anyone relying on the bug: an unhandledRejection listener no longer also fires for a late-rejecting override result; it did not fire for an already-rejected one before either.
• Independent of node:module: check for an exception after wrapping a runMain override's return value #38113 (exception check after resolvedPromise) and node:module: throw instead of crashing when a preload sets runMain to a non-callable, and report a throwing override #38119 (non-callable / throwing override); the three touch neighbouring lines and compose, whichever lands first.
• Verified with test/js/node/module/node-module-module.test.js: seven new cases (already rejected promise, promise that rejects later, rejecting thenable, each checked for the stderr count and for listener delivery on the main thread, plus the late-rejecting override installed by a worker preload). Five of them fail on the unfixed binary (errorsPrinted: 2, both listeners firing) and all pass with the fix; the two already-rejected cases pass both ways and pin the behaviour the other cases are aligned with.
• Also run with the fix: the rest of that file, test/js/node/test/parallel/test-module-run-main-monkey-patch.js, test/cli/run/preload-test.test.js, test/js/bun/resolve/bun-main-entry-point.test.ts.

Background

• Module.runMain override: if a preload assigns require("module").runMain, bun calls the override instead of evaluating the entry point itself. If the override calls the original runMain, the module loader's promise is stored as the entry point promise; otherwise the override's return value is adopted with Promise.resolve semantics (a native promise is used as is, a thenable is wrapped) and stored instead.
• pending_internal_promise: the VM field holding the entry point promise. load_entry_point spins the event loop until it settles, then Run::start prints a rejection through the uncaught exception path (uncaughtException listeners, origin unhandledRejection) and exits 1.
• Unhandled rejection tracker: JSC notifies the host when a promise with no handlers rejects; bun queues those promises and reports them (unhandledRejection listeners, otherwise print) at the end of each event loop tick. A promise whose handled flag is set is skipped by both the notification and the queue. JSC's own module loader sets that flag on the promises it hands to embedders because the embedder, not the tracker, reports them.

Probes on the release binary (before) and the debug build (after)

# before (1.4.0-canary da3851e57)
override: async () => { await 0; throw }        -> error block printed twice, exit 1
override: () => ({ then(_, reject) { reject() }}) -> error block printed twice, exit 1
override: async () => { throw }                  -> printed once, exit 1
plain main.mjs with `await 0; throw`             -> printed once, exit 1
late override + unhandledRejection/uncaughtExceptionMonitor listeners
  -> "unhandledRejection handler" AND "uncaughtExceptionMonitor ... unhandledRejection", then printed, exit 1
--hot with the late override                     -> printed twice
worker with { preload } overriding runMain (late), listeners in the worker
  -> both "unhandledRejection" and "uncaughtException ... unhandledRejection" logged by the worker
worker, same override, no listeners              -> one 'error' event in the parent, worker exit 1
  (the worker's first report swaps in a quiet handler, so the duplicate is only visible via listeners)

# after (debug build of this branch)
late override / thenable / already rejected      -> printed once, exit 1
late override + listeners                        -> only "uncaughtExceptionMonitor ... unhandledRejection", printed once
passthrough override (calls original runMain) + main.mjs that throws after await -> printed once, exit 1
--hot with the late override                     -> printed once
worker with { preload } override, listeners      -> only "uncaughtException ... unhandledRejection"
worker, no listeners                             -> still one 'error' event, worker exit 1

[coderabbitai]

Warning

Review limit reached

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

Next review available in: 26 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: 10137c0d-76c3-4de2-ab1b-faa4a574d7a1

📥 Commits

Reviewing files that changed from the base of the PR and between b7a0431 and 856f596.

📒 Files selected for processing (2)

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

---

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

[robobun]

^{Updated 2:41 PM PT - Aug 13th, 2026}

✅ @robobun, your commit 856f596e556dc39f9dd1f23ccf5ec56fbbcc0dee passed in Build #94797! 🎉

---

🧪   To try this PR locally:

bunx bun-pr 38160

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

bun-38160 --bun

[robobun]

Status: reproduced and fixed; self-review in progress.

• Reproduced on the release binary (1.4.0-canary da3851e) with a --require preload setting Module.runMain = async () => { await 0; throw new Error("boom"); }: the error block is printed twice, exit 1. Same with a rejecting thenable, and inside a worker whose preload option installs the override (both unhandledRejection and uncaughtException listeners fire there). An already rejected promise and a plain entry that throws after a top-level await report once.
• Fix: mark the adopted override result as handled when it is stored as the entry point promise (src/jsc/VirtualMachine.rs), so only the run command (or the worker's entry check) reports it.
• Tests: seven new cases in test/js/node/module/node-module-module.test.js; five fail on the unfixed binary and all pass with the fix.

[claude]

LGTM — one-line fix marking the adopted runMain override promise as handled so the entry-point rejection is reported once.

Reviewed: the reload_entry_point path where the override's return value is wrapped via resolvedPromise and stored as pending_internal_promise; confirmed JSInternalPromise aliases JSPromise so JSPromise::opaque_mut(...).set_handled() is type-correct and routes to markAsHandled(). Checked the null-return case from resolvedPromise — pre-existing (stored and deref'd null before this change), tracked separately by #38113, and opaque_mut's null panic is not a regression over that. Tests cover already-rejected / late-rejecting / thenable × stderr-count / listener-delivery, use the harness helpers, and drain pipes concurrently.

Extended reasoning...

Overview

This PR adds a single line (plus comment) to reload_entry_point in src/jsc/VirtualMachine.rs: after adopting a Module.runMain override's return value via JSC__JSInternalPromise__resolvedPromise, it marks the resulting promise as handled before storing it as pending_internal_promise. Six new subprocess test cases are added to the existing test/js/node/module/node-module-module.test.js.

Security risks

None. This touches only how a rejected entry-point promise is bookkept for the unhandled-rejection tracker; no untrusted input parsing, no auth/crypto, no filesystem or network semantics change.

Level of scrutiny

Low-to-medium. The change is one flag set on a promise the runtime already owns and reports via Run::start. I verified the mechanics: JSInternalPromise is pub use JSPromise as JSInternalPromise (src/jsc/lib.rs:206), so crate::JSPromise::opaque_mut(resolved) accepts the *mut JSInternalPromise and set_handled() calls JSC__JSPromise__setHandled → promise->markAsHandled(). The rationale — module-loader promises stored in the same field are already created marked-handled by JSC, and Run::start is the single reporter — is sound and fixes the bug at the layer that owns the invariant (the site that adopts the promise), not at the symptom.

Other factors

• opaque_mut panics on null. JSPromise::resolvedPromise can in principle return null if a thenable's then getter throws, but the pre-fix code already stored and later deref'd that null; the PR description explicitly notes #38113 covers the exception check there and the changes compose. Not a regression introduced here.
• Tests follow harness conventions exactly: test.each matrix, tempDir/bunEnv/bunExe, Promise.all over stdout/stderr/exited, combined-object assertions, no sleeps, hermetic. Placed in the existing module test file inside describe.concurrent, so they run in parallel with independent temp dirs.
• No CODEOWNERS constraints on the touched paths. No prior human review comments to address.

[claude]

LGTM — one-line fix aligning the adopted runMain override promise with the invariant every other pending_internal_promise store already has.

What was reviewed:

• Confirmed JSInternalPromise aliases JSPromise (src/jsc/lib.rs:206), so JSPromise::opaque_mut(resolved).set_handled() type-checks and calls markAsHandled() on the C++ side.
• Both consumers of this field — Run::start (run_command.rs:1434) and the worker's observe_entry (web_worker.rs:869) — already report the rejection via uncaught_exception and explicitly rely on the promise being handled; this was the one store that was not.
• opaque_mut panics on null, but the pre-existing code already stored/returned resolved unchecked, so no new failure mode (and #38113 covers the exception-check side).
• Tests cover already-rejected / late-rejecting / thenable × stderr-count and listener delivery, plus the worker preload path; they follow harness conventions (tempDir, bunEnv, drained pipes, combined-object assertions).

Extended reasoning...

Overview

Two-line change in src/jsc/VirtualMachine.rs (reload_entry_point): after adopting a patched Module.runMain override's return value via JSC__JSInternalPromise__resolvedPromise, mark the resulting promise as handled before storing it in pending_internal_promise. Seven new subprocess tests in test/js/node/module/node-module-module.test.js.

Security risks

None. This adjusts which of two internal reporting paths fires for an entry-point rejection; no untrusted-input parsing, no auth/crypto/permissions surface.

Level of scrutiny

Medium. It touches VM entry-point promise handling, but the change is minimal and mechanical: it sets the same isHandled flag on the adopted promise that the module loader already sets on every other promise stored in this field. I verified that both consumers of pending_internal_promise (Run::start and the worker's observe_entry) do report the rejection themselves and are documented as relying on the handled flag (web_worker.rs:867 comment: "The loader marks this promise handled, so nothing else would report it"). The fix is at the layer that owns the invariant — the one store site that broke it.

Other factors

• JSInternalPromise is a type alias for JSPromise (src/jsc/lib.rs:206), so crate::JSPromise::opaque_mut(*mut JSInternalPromise) compiles and dispatches to JSC__JSPromise__setHandled → promise->markAsHandled().
• opaque_mut panics on null; the pre-fix code already stored and returned resolved without a null check, so this introduces no new failure mode. The exception-check gap after resolvedPromise is explicitly deferred to #38113.
• The fulfilled case is unaffected (handled flag only matters for rejection tracking). The passthrough-override case (line 2711) returns the loader's own promise, which is already handled.
• Tests follow repo conventions: tempDir, bunEnv, Promise.all on stdout/stderr/exited, combined-object toEqual assertions. The variant matrix (already-rejected, late-rejecting, thenable; with and without listeners; main thread and worker preload) is covered, and the PR description confirms five of the seven fail on the unfixed binary.
• The comment-cop bot feedback (comment length) was addressed in 856f596 — the comment is now one line naming the invariant.