Report a CommonJS entry's top-level throw before the microtasks it queued

[robobun]

Problem

• A CommonJS entry that throws out of its top level first runs everything it queued before throwing. With mt.cjs being

  Promise.resolve().then(() => console.log("microtask ran"));
  throw new Error("boom");

  bun mt.cjs prints microtask ran and then error: boom; node mt.cjs prints only the error. With an uncaughtException listener the inversion is visible too: Bun runs the queued nextTicks and promise reactions, then the listener; Node runs the listener first. Same for a worker_threads worker whose entry is CommonJS (eval: true source or a .cjs file): the queued reactions ran before the worker's 'error' event and 'exit' handlers.
• Cause: the CommonJS entry body runs inside the module loader's promise chain (the synthetic module callback in src/jsc/bindings/JSCommonJSModule.cpp, commonJSModuleSyntheticSourceCode). Its throw becomes the entry promise's rejection, and Bun only looks at that promise after the microtask drain that reported it (Run::start in src/runtime/cli/run_command.rs; observe_entry in WebWorker::spin, src/jsc/web_worker.rs), by which time the drain has also run whatever the entry queued.
• This only concerns entries Bun evaluates as CommonJS (.cjs, or .js / -e / stdin / worker eval source that references CommonJS globals). Code Bun evaluates as ESM keeps the promise-based ordering, which is also Node's ordering for an ESM entry.

Fix

• JSCommonJSModule.cpp: when the module that threw out of the synthetic CommonJS evaluation is the entry (Bun__isBunMain on the module key, the predicate behind import.meta.main), call the new Bun__VM__reportEntryPointThrow (src/runtime/hw_exports.rs) before rethrowing. It runs uncaught_exception with origin uncaughtException right there. With no listener, the main thread exits through exit_with_unhandled_note, the same tail the rejection path used (so the printed output is unchanged), and in a worker on_unhandled_rejection has already posted the 'error' event, run the 'exit' handlers and requested the stop. With a listener, the hook returns and the loader rejects the entry promise as before.
• VirtualMachine::entry_point_failure_reported() / mark_entry_point_failure_reported() wrap the existing hot-reload marker (pending_internal_promise_reported_at == hot_reload_counter). The hook sets it; Run::start, the hot reloader and WebWorker::spin consult it, so the rejection that follows is not reported a second time. The worker's failed-load arm also stops overwriting the exit code when the throw was reported that way (the 'exit' handlers may have changed it, as in Node).
• Bun__VM__noteCommonJSEvaluation and EntryPointResult.evaluated_as_cjs (which existed to pick the origin string after the fact) are removed: a CommonJS entry's throw is now reported at the site, so whatever still reaches the rejection path is an ESM entry or a load failure and is reported with origin unhandledRejection, as in Node. As a side effect the origin is now right under --preload too, where the entry is not the root module.
• uncaught_exception skips the exit_on_uncaught_exception shortcut while a watcher (--hot / --watch) keeps the process alive. The flag is armed whenever a generation's loop drains (see Background), which is the end of a normal run but not of a watched one; it already made bun --hot exit on a callback throw in the generation after a drained one (reproducible on main), and it would have made it exit on the next generation's entry throw now that those go through uncaught_exception. The guard sits at the consumer rather than clearing the flag per generation because the watcher loop re-arms it on every drain, including the drain that happens while the next generation is still being transpiled and loaded (clearing it in reload() was tried first and left hot.test.ts's "stale sourcemap" case, whose hot file is CommonJS, failing intermittently); what the shortcut actually depends on is whether the run is ending, and under a watcher it is not.
• Why this is correct: it is Node's model for a CommonJS entry, whose body runs synchronously under the runner, so the throw is the fatal exception (or the listeners' call) before any microtask checkpoint. The report itself is the same uncaught_exception call the rejection path made, only earlier, so listeners, uncaughtExceptionMonitor, the capture callback, _fatalException (exit 6), a throwing listener (exit 7), exit-handler exit codes and --hot keep-alive all behave as before; only the position of the report relative to the queued work changes.
• Not changed here: in a worker, the reactions queued before an unhandled entry throw still run after the 'error' event and 'exit' handlers, because the worker's stop request does not yet discard the microtask queue. That is worker: discard what was queued before process.exit(), an uncaught error or terminate(); first process.exit() decides the exit code #38016's change; with it, the combination gives Node's exact behaviour (nothing queued runs). The worker tests here assert the order only. (Node itself still runs nextTick callbacks queued before such a throw in a worker, an artifact of its entry running inside a message listener; not emulated.)
• Also not changed: a throwing CommonJS preload is still reported through the rejection path with origin unhandledRejection, as on main (the removed note only ever matched the entry). Report a CommonJS preload's top-level throw with origin uncaughtException #38159 is about preloads; once this lands it can extend the throw-site check to the preload being loaded (it has the bookkeeping for that), which gives preloads the ordering fix along with the origin.
• The one CI casualty was the vendored test/js/node/test/parallel/test-runner-tags-experimental-warning.mjs (adapted in the second commit). Three of its cases spawn children that, as written upstream, throw out of their top level in Bun (there is no --test CLI mode, run() rejects isolation: 'none', it() throws outside the runner) and counted the ExperimentalWarning those children had emitted just before dying. process.emitWarning prints from a nextTick, so that only worked because of the ordering fixed here; Node prints nothing in that situation either (node -e 'process.emitWarning("w"); throw new Error("x")' shows no warning). The CLI case is now skipped under Bun, and the other two trigger the warning through run() with process isolation, so they exercise the programmatic trigger for real (a file's own registrations then happen in a child whose stderr becomes test:stderr events, so exactly the driver's one warning is counted). Passes under the node-test runner configuration locally (4 pass, 1 skip); I kept the change to what the fix affected rather than un-vendoring the file, but deleting it instead is fine by me if that is preferred for upstream files.
• Verified:
  • test/js/node/process/process.test.js, new describe("a CommonJS entry's top-level throw"): unhandled (plain and under --preload), handled, and an ESM control case. The three CommonJS cases fail on main; the expected stdout is what Node v26 prints for the same scripts.
  • test/js/node/worker_threads/worker_threads.test.ts, new describe("a CommonJS worker entry's top-level throw"): eval and .cjs entries, each with the unhandled, exit-handler-exit-code and handled scenarios (parent in a subprocess, results in shared memory). Both fail on main; the handled scenario's full order matches Node.
  • test/cli/hot/hot.test.ts, new "should keep watching through uncaught errors in later generations": a callback throw after a drained generation, then a CommonJS entry throw after a failed one. Fails on main (the process exits after the first).
  • Passing with the change, on a local debug build: process.test.js (the remaining failures there are USER being unset in this container and 5s timeouts of worker-spawning tests that pass with a longer timeout), worker_threads.test.ts (124 pass), hot.test.ts (including "should not remap against a stale sourcemap after a partial-file reload", whose hot file is CommonJS and which exposed the --hot exit), hot/watch.test.ts, watch/watch.test.ts, run-eval.test.ts (CommonJS-sniffed -e and stdin entries), run-cjs, commonjs-invalid, preload-test, node-module-module, and the vendored test-process-exit-code.js, test-process-uncaught-exception-monitor.js (both exercise the new path: handled / unhandled / exit-handler exit codes / exit 6 / exit 7) and 18 test-worker-*exit* / *uncaught* / *syntax-error* tests.

Background

• How a CommonJS entry is run: bun:main (a generated ESM wrapper) imports the entry. For a CommonJS file the loader gets a synthetic module whose generator (commonJSModuleSyntheticSourceCode) runs the CommonJS body; that generator is invoked from the loader's promise chain, i.e. inside a microtask drain. Its throw rejects the loader's promise, which Run::start / spin inspect after load_entry_point returns. ESM entries are the same shape, and for those Node also surfaces the throw as a rejection.
• Which files are CommonJS: .cjs always; .js, -e, stdin and worker eval source when they reference CommonJS globals (require, module, exports, __filename...); otherwise Bun evaluates them as ESM. Node would run most of those as CommonJS, but that is a module-format question this PR does not touch; it fixes the ordering for what Bun does evaluate as CommonJS.
• uncaught_exception (src/jsc/VirtualMachine.rs) is the one reporting function: it emits uncaughtExceptionMonitor / uncaughtException (or the capture callback), and if nothing handled the error it sets exit code 1 and calls the VM's on_unhandled_rejection, which on the main thread prints the error and in a worker posts it to the parent, runs the 'exit' handlers and requests the worker's stop. Timer callbacks, event listeners and napi already report throws through it at the throw site; this PR does the same for the entry.
• pending_internal_promise_reported_at: the hot reloader bumps hot_reload_counter per generation and records in this field the generation whose entry failure it has printed, so that a rejection is printed once and a reload does not replace an entry promise whose error has not been reported yet. The new helpers expose that "already reported" state; the throw-site report sets it for the current generation.
• exit_on_uncaught_exception: armed when 'beforeExit' is dispatched (Process__dispatchOnBeforeExit) or when the loop drains after a fatal error; it makes the next uncaught error print and exit immediately, since a draining run has no loop turn left in which the error could be reported normally. Under --hot / --watch the run command calls the same drain path and then keeps the process alive for file changes, so the flag stays armed across generations.

[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: c0292972-2cae-42c8-a445-afc521a5e034

📥 Commits

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

📒 Files selected for processing (9)

• src/jsc/VirtualMachine.rs
• src/jsc/bindings/JSCommonJSModule.cpp
• src/jsc/web_worker.rs
• src/runtime/cli/run_command.rs
• src/runtime/hw_exports.rs
• test/cli/hot/hot.test.ts
• test/js/node/process/process.test.js
• test/js/node/test/parallel/test-runner-tags-experimental-warning.mjs
• test/js/node/worker_threads/worker_threads.test.ts

---

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

[robobun]

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

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

• Failed to create agent for 🪟 11 aarch64 - test-bun

---

🧪   To try this PR locally:

bunx bun-pr 38141

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

bun-38141 --bun

[robobun]

Status: reproduced on main and fixed here; waiting on CI for a479a3c.

Reproduced on main (bdb738222e) and on the 1.4.0 canary with a two-line .cjs entry (Promise.resolve().then(() => console.log("microtask ran")); throw new Error("boom")): Bun prints microtask ran before the error, Node prints only the error. The same holds with an uncaughtException listener (Bun runs the queued nextTick/promise callbacks before the listener) and for worker_threads workers with an eval or .cjs entry. The fix and the tests that fail without it are described in the PR body.

The first CI run's only real failure was the vendored test-runner-tags-experimental-warning.mjs, whose child processes were counting a warning printed only because of the ordering fixed here; adapted in 5e2dcb6 (details in the PR body). Preloads are out of scope here and coordinated with #38159.

[claude]

I reviewed this PR and the bug hunting system found no issues. Because it reroutes entry-point exception handling across the VM, the CommonJS synthetic-module callback, worker shutdown, and --hot — including a new FFI hook that may hard-exit from inside the module loader — a human look would still be worthwhile.

What was reviewed:

• The Bun__isBunMain gate and isBunTest/worker/--hot guards in report_entry_point_throw — the exit is scoped to the main-thread run command.
• The "already reported" marker: pending_internal_promise_reported_at is now private and every reader/writer goes through the two new helpers; evaluated_as_cjs / noteCommonJSEvaluation are fully removed with no remaining references.
• The new is_watcher_enabled() guard on exit_on_uncaught_exception and its coverage by the new hot-reload test.

Extended reasoning...

Overview

This PR fixes Node-compat ordering for a CommonJS entry's top-level throw: instead of surfacing it as the entry promise's rejection (after the microtask drain that also runs whatever the entry queued), the throw is reported at the throw site via a new Bun__VM__reportEntryPointThrow FFI hook called from commonJSModuleSyntheticSourceCode in JSCommonJSModule.cpp. It touches VirtualMachine.rs (new entry_point_failure_reported/mark_entry_point_failure_reported helpers wrapping the now-private pending_internal_promise_reported_at; a new is_watcher_enabled() guard on the exit_on_uncaught_exception shortcut; removal of EntryPointResult.evaluated_as_cjs), web_worker.rs (consult the marker in spin and the failed-load arm), run_command.rs (rewire the rejected-entry branch through the marker; expose exit_with_unhandled_note as pub(crate)), and hw_exports.rs (replace note_commonjs_evaluation with report_entry_point_throw). Three test files add coverage for the main thread, workers (eval and .cjs), and --hot across generations.

Security risks

None identified. This is an ordering/Node-compat change to how an already-thrown user error is routed to uncaught_exception; no new inputs are parsed, no security gates are relaxed, and the exit path (exit_with_unhandled_note) is the same tail the rejection path already used.

Level of scrutiny

High. The change is small in line count but sits on the runtime's most critical control flow: entry-point evaluation, the single uncaught_exception funnel, process exit, worker stop/exit-code selection, and hot-reload keep-alive. report_entry_point_throw may call exit_with_unhandled_note (which never returns) from inside JSC's synthetic-module generator callback — a deliberate design that the PR justifies well, but the interaction with the test runner (isBunTest gate), workers (worker_ref().is_none()), and --hot/--watch (hot_reload == 0) is cross-cutting enough that a maintainer familiar with the module-loader stack should confirm nothing else on that C++ call path (e.g. requireMap state, ThrowScope invariants) is left inconsistent when the process exits mid-callback.

Other factors

• The is_watcher_enabled() guard in uncaught_exception is a real behaviour change independent of the CJS ordering fix (the PR notes it also fixes a pre-existing --hot exit on a callback throw in a later generation); the new hot.test.ts case covers it.
• I confirmed pending_internal_promise_reported_at has no remaining external writers after being made private, and evaluated_as_cjs/noteCommonJSEvaluation have no residual references anywhere in src/.
• Test coverage is thorough (unhandled/handled/preload/ESM control on the main thread; eval and .cjs × unhandled/exit-code/handled in workers via shared memory; a multi-generation --hot scenario), and the PR description enumerates the vendored Node tests that exercise exit-code 6/7 and monitor paths.
• No prior human or bot review comments to address; CI build was still in progress at the time of this review.

[robobun]

On the one point left open above (state of the C++ call path when the hook does not return), for whoever looks at this:

• The requireMap entry is removed before the hook is called, so the 'exit' handlers that exit_with_unhandled_note runs see the same thing they see in Node, where Module._load deletes the cache entry before the fatal exception is raised. Nothing else in the synthetic callback has been touched at that point; the rethrow is the only thing skipped.
• The exit itself is the same on_exit() + global_exit() tail that process.exit() takes from inside any host function, with whatever JSC frames (and ThrowScopes) happen to be on the stack; nothing is unwound in either case. On the returning paths (a listener took the error, --hot/--watch, workers) the callback re-checks the scope right after the call and then rethrows as before.
• The main-thread fixtures (handled, unhandled, --preload) and the worker fixtures also run clean under BUN_JSC_validateExceptionChecks=1.

[robobun]

Cross-reference: #38159 fixes the origin reported for a CommonJS preload that throws (bun -r ./preload.cjs main.js currently passes "unhandledRejection"), by keeping evaluated_as_cjs and matching the CommonJS note against the preload load_preloads is importing (EntryPointResult.loading_preload).

As this PR stands, a preload's key is not Bun__isBunMain, so its throw still goes through the rejection path and, with the flag removed, is reported as "unhandledRejection"; the five CommonJS cases in the describe("origin of a top-level throw in a preload or entry") block added in #38159 describe the expected behavior (node -r output). Node require()s preloads synchronously too, so if this lands first, widening the throw-site check to "entry or the preload currently being imported" gives preloads both the origin and the ordering fix, and #38159 reduces to the loading_preload bookkeeping plus its tests.

[robobun]

Re #38159: agreed on the division, with one correction to the premise. This PR does not change what a throwing CommonJS preload reports: the note it removes only matched when the root module's filename was main, so a preload's throw was already reported as "unhandledRejection" on main, and it still is here. So nothing here regresses preloads; it just does not fix them either.

Since Node require()s preloads synchronously too, the right end state is what you describe: the throw-site report covers "the entry, or the preload load_preloads is currently importing". That needs the loading_preload bookkeeping #38159 already has, so the natural split is for this PR to land the mechanism for the entry and for #38159, rebased on it, to add the bookkeeping and replace the Bun__isBunMain check in commonJSModuleSyntheticSourceCode with a predicate that also accepts the preload being loaded (or pass the key into Bun__VM__reportEntryPointThrow and compare on the Rust side). Its preload cases then get the ordering along with the origin, and evaluated_as_cjs does not need to come back. I am keeping this PR to the entry.

[claude]

I reviewed this PR and the automated bug-hunt found no issues. Because it changes the entry-point error-reporting path across the C++ module loader, the run command, workers, and --hot, and adds an FFI hook that may exit from inside a JSC callback, a human look is still worthwhile.

What was reviewed:

• The new Bun__VM__reportEntryPointThrow path: isBunTest guard, mark_entry_point_failure_reported before uncaught_exception, and the main-thread exit_with_unhandled_note tail — matches the rejection path's tail and the worker path returns.
• entry_point_failure_reported() consumers in Run::start, the hot reloader, and WebWorker::spin — no double-report; promise.set_handled() still runs on the already-reported branch.
• The !is_watcher_enabled() guard on the exit_on_uncaught_exception shortcut, and the new hot.test.ts case exercising both a callback throw after a drain and a CJS entry throw after a failure.
• Removal of evaluated_as_cjs — every remaining rejection-path caller now passes is_rejection = true, which is correct given CJS throws are reported at the throw site.

Extended reasoning...

Overview

This PR changes when a CommonJS entry point's top-level throw is reported: instead of surfacing as the entry promise's rejection (observed only after the microtask drain that also runs whatever the entry queued), it is reported synchronously from the throw site in commonJSModuleSyntheticSourceCode via a new FFI export Bun__VM__reportEntryPointThrow. Touches VirtualMachine.rs (new entry_point_failure_reported accessors, a !is_watcher_enabled() guard on the exit-on-uncaught shortcut, removal of EntryPointResult.evaluated_as_cjs), JSCommonJSModule.cpp (the throw-site call, gated on Bun__isBunMain), web_worker.rs (seed entry_rejection_seen from the marker; don't clobber the exit code when the throw was reported), run_command.rs (skip re-reporting; make exit_with_unhandled_note pub(crate)), and hw_exports.rs (the new export replacing note_commonjs_evaluation). Tests added in process.test.js, worker_threads.test.ts, hot.test.ts; a vendored Node test adapted.

Security risks

None identified. No untrusted-input parsing, no auth/crypto/permissions surface. The change is to error-report ordering and process-exit control flow.

Level of scrutiny

High. This is core runtime control flow: the module loader's C++ synthetic-source callback now calls into Rust and, on the unhandled main-thread path, exits the process with live JSC frames on the stack (the author notes this is the same shape as process.exit() from any host function, and verified under BUN_JSC_validateExceptionChecks=1). It also changes the --hot/--watch interaction with exit_on_uncaught_exception, and the worker exit-code path. The reasoning in the PR body is thorough and the test coverage is good (unhandled/handled × main/worker × plain/--preload, plus an ESM control and a --hot regression), but the number of interacting states (hot-reload counter, watcher, worker stop, test runner, preloads, #38159/#38016 coordination) makes this a change a maintainer should sign off on.

Other factors

• The comment-cop bot flagged long comments; the author shortened some in a479a3c and kept others with per-line justifications; all threads are resolved.
• The vendored test-runner-tags-experimental-warning.mjs was adapted rather than deleted; the author explicitly offers deletion as an alternative, which is a maintainer call.
• CI is still building for a479a3c per the robobun status comment.

[claude]

I reviewed this PR and didn't find any bugs. Because it changes error-reporting control flow across the module loader, run command, worker lifecycle, and --hot — including a new C++→Rust call that may exit the process from inside the synthetic module callback's ThrowScope — a human look would still be worthwhile.

What was reviewed:

• The new Bun__VM__reportEntryPointThrow path: isBunTest guard, mark_entry_point_failure_reported before uncaught_exception, and the main-thread-only / hot_reload == 0 gate on exit_with_unhandled_note.
• entry_point_failure_reported() consumers in Run::start, the hot reloader, and WebWorker::spin — no double-report, and the worker's failed-load arm no longer clobbers an exit-handler-chosen code.
• The !is_watcher_enabled() guard on exit_on_uncaught_exception and the new hot.test.ts case exercising both a callback throw and a CJS entry throw across generations.
• Removal of evaluated_as_cjs: remaining rejection-path callers now pass is_rejection = true, which is correct for what still reaches them.

Extended reasoning...

Overview

This PR reorders when a CommonJS entry point's top-level throw is reported: instead of surfacing as the entry promise's rejection (observed only after the microtask drain that also ran whatever the entry queued), it is now reported at the throw site via a new FFI call Bun__VM__reportEntryPointThrow from commonJSModuleSyntheticSourceCode. Touches VirtualMachine.rs (new entry_point_failure_reported / mark_entry_point_failure_reported helpers wrapping the existing hot-reload marker; !is_watcher_enabled() guard on the exit_on_uncaught_exception shortcut), JSCommonJSModule.cpp (throw-site hook gated by Bun__isBunMain), web_worker.rs (skip re-reporting; don't clobber exit code), run_command.rs (skip re-reporting; exit_with_unhandled_note made pub(crate)), hw_exports.rs (the new export replacing note_commonjs_evaluation), plus tests in process.test.js, worker_threads.test.ts, hot.test.ts, and an adaptation of a vendored Node parallel test.

Security risks

None identified. This is error-reporting ordering; no new user-controlled input reaches a security-sensitive path.

Level of scrutiny

High. This is a control-flow change in the runtime's fatal-exception path with cross-cutting effects on --hot/--watch, workers, the test runner, and preloads (coordinated with #38159). The C++ side now calls into Rust from inside an active ThrowScope where the call may not return on the main thread (author notes this is the same shape as process.exit() from any host function, and the returning paths — listener took it, watcher, worker — re-check the scope and rethrow). The exit_on_uncaught_exception change also fixes a pre-existing --hot exit bug the author found while working on this. Each of these is subtle enough on its own that a maintainer should confirm the model.

Other factors

The PR description is unusually thorough, the tests cover main-thread (handled/unhandled/--preload), worker (eval and .cjs, three scenarios each), and --hot across generations, and the author verified under BUN_JSC_validateExceptionChecks=1. The comment-cop bot flagged long comments; the author shortened some and justified keeping the rest (all threads resolved). No prior human review comments to address. The vendored Node test modification is a real behavioral adaptation (not a weakening) that a maintainer may prefer to handle differently.