process: skip 'beforeExit' after a fatal uncaught exception

[robobun]

What

process.on('beforeExit', ...) fired after a default-fatal uncaught exception. Node never emits 'beforeExit' in that case: its fatal-exception path is effectively process.exit(1), and the docs say 'beforeExit' is "not emitted for conditions causing explicit termination, such as calling process.exit() or uncaught exceptions".

Repro

process.on('beforeExit', () => console.log('BEFOREEXIT'));
process.on('exit', c => console.log('EXIT', c));
setTimeout(() => { throw new Error('boom'); }, 1);

	stdout	exit
Node v26.3.0	EXIT 1	1
Bun (before)	BEFOREEXIT then EXIT 1	1
Bun (after)	EXIT 1	1

Cause

VirtualMachine::uncaught_exception() has two paths when no 'uncaughtException' listener handled the error:

• If exit_on_uncaught_exception is set, it hard-exits via process_exit(global, 1). But that flag is only armed inside Process__dispatchOnBeforeExit, i.e. after beforeExit has already fired.
• Otherwise it records the fatal state (unhandled_error_counter += 1, exit_handler.exit_code = 1), prints the error and returns. The main drain loop in Run::start then falls out (because is_event_loop_alive() sees the nonzero counter) and unconditionally calls vm.on_before_exit().

Fix

Gate both dispatch sites in on_before_exit() on unhandled_error_counter == 0 so 'beforeExit' only fires on a natural drain:

• Entry: skip entirely when we arrived via a fatal throw. Arm exit_on_uncaught_exception ourselves here since the skipped Process__dispatchOnBeforeExit was its sole setter; without it a throw from an 'exit' listener after a fatal throw would fall through and let subsequent 'exit' listeners run.
• Re-dispatch inside the drain loop: skip when work scheduled by a beforeExit listener itself threw. On the main thread exit_on_uncaught_exception already hard-exits this case; the guard covers workers where that flag's hard-exit is main-thread-only.

Controls verified:

• With process.on('uncaughtException', ...) installed: beforeExit still fires, exit 0. (New regression-guard test.)
• After a fatal throw, a throw from the first 'exit' listener still stops subsequent 'exit' listeners. (New regression-guard test; would regress without the flag-arming.)
• test/cli/hot/hot.test.ts including "should recover from errors" passes; the watcher loop is unaffected since it proceeds to tick_possibly_forever() either way, and exit_on_uncaught_exception was already armed by the previous unconditional dispatch.
• All test-process-beforeexit* / test-process-exit* / test-worker-*beforeexit* / test-promises-unhandled-* Node parallel tests pass.

Side effect: unhandled rejections

unhandled_error_counter is also bumped on the unhandled-rejection path (Mode::Bun fallthrough, Mode::Throw), so this guard now also skips 'beforeExit' when an unhandled rejection is processed before the drain loop exits (e.g. Promise.reject(...); setImmediate(() => {});). That is a Node-parity improvement on that path, but it is not the whole story: Bun's default mode still processes some rejections only at handle_rejected_promises() after on_before_exit(), so setTimeout(() => Promise.reject(...), 1) is unchanged. The full fix (routing the default unhandled-rejection mode through uncaughtException) is #32814's backed-out scope and needs the watcher/worker hardening first; no test pins the partial behavior here.

Verification

$ USE_SYSTEM_BUN=1 bun test test/js/node/process/process.test.js -t "is skipped after a fatal"
(fail) process.onBeforeExit > is skipped after a fatal uncaught exception
  + "beforeExit
  + exit 1
  "

$ bun bd test test/js/node/process/process.test.js -t "is skipped after a fatal"
(pass) process.onBeforeExit > is skipped after a fatal uncaught exception

Found while investigating #34627; reproduces with or without top-level await.

---

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

[coderabbitai]

Walkthrough

Changes

beforeExit error handling

Layer / File(s)	Summary
Guard beforeExit dispatch during fatal shutdown src/jsc/VirtualMachine.rs	on_before_exit skips initial and repeated beforeExit dispatch when unhandled errors are present.
Validate fatal and handled exception paths test/js/node/process/process.test.js	Subprocess tests cover fatal suppression, handled-exception dispatch, and interruption of later exit listeners after a fatal throw.

Suggested reviewers: jarred-sumner, cirospaciari

🚥 Pre-merge checks | ✅ 4

✅ Passed checks (4 passed)

Check name	Status	Explanation
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Title check	✅ Passed	The title clearly states the main behavior change: skipping beforeExit after a fatal uncaught exception.
Description check	✅ Passed	The description covers the behavior change, root cause, fix, and verification steps, which satisfies the template intent.

---

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

[robobun]

^{Updated 9:41 AM PT - Jul 18th, 2026}

✅ @robobun, your commit 7d5a2ec2e7409bf4f63ebfacaf7f30ba443f0f6f passed in Build #75451! 🎉

---

🧪   To try this PR locally:

bunx bun-pr 34639

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

bun-34639 --bun

[robobun]

Reproduced with setTimeout(() => { throw ... }) + a beforeExit listener: Bun fires beforeExit then exit 1; Node fires only exit 1.

Fix: gate both dispatch sites in on_before_exit() on unhandled_error_counter == 0 (entry + the re-dispatch inside the drain loop), and arm exit_on_uncaught_exception in the entry guard since the skipped Process__dispatchOnBeforeExit was its sole setter. Verified against Node v26.3.0 for the unhandled case (beforeExit skipped), the handled case (uncaughtException listener installed; beforeExit still fires), and the exit-listener-throws-after-fatal case (subsequent exit listeners still stopped). hot.test.ts and the Node parallel test-process-beforeexit*/test-process-exit*/test-worker-*exit*/test-promises-unhandled-* suite pass.

CI: test/js/node/process/process.test.js is green on every lane in #75439 and #75451. Remaining red is unrelated retry-pass flake (install registry/security-scanner on Windows, http3 adversarial, es-module-lexer/spawn/repl-close Windows timeouts, node-net mimalloc threshold, webview-chrome animation, cron). The one non-flaky hit on #75439 was worker-transfer-terminate-stress.test.ts, a documented intermittent asan-lane SIGABRT in the MessagePort-transfer-during-terminate path; my change doesn't touch that path (workers with has_requested_terminate() never reach on_before_exit()) and it passes locally 3/3. Ready for review.

[claude]

LGTM — the follow-up commit gates the re-dispatch on the same counter, addressing the worker-thread sibling site I flagged.

What was reviewed:

• Traced all four on_before_exit() callers (main run loop, watcher, worker, repl); each correctly proceeds past the new early-return, and the watcher still reaches tick_possibly_forever().
• Confirmed unhandled_error_counter is only bumped on genuinely fatal paths (unhandled exception/rejection, recursive-handler, hot-reload rejection), so the guard cannot suppress beforeExit after a handled throw — the second new test pins that.
• Tests drain stdout/stderr/exited concurrently, assert exact stdout before exit code, and use targeted stderr checks rather than exact-empty.

Extended reasoning...

Overview

Two-file change: src/jsc/VirtualMachine.rs gains an early-return in on_before_exit() when unhandled_error_counter > 0, plus the same condition on the inner re-dispatch (added in 6056186 after my earlier review). test/js/node/process/process.test.js gains two subprocess tests inside the existing process.onBeforeExit describe block — one asserting beforeExit is skipped after a fatal async throw, and a regression guard that it still fires when an uncaughtException listener swallows the throw.

Security risks

None. This is purely event-emission ordering for process lifecycle; no untrusted input parsing, no new syscalls or FFI surface.

Level of scrutiny

Medium. VirtualMachine.rs is core, but the diff is a narrow, additive guard on a counter that is_event_loop_alive() already reads for the same purpose. I traced every unhandled_error_counter += 1 site (:1071/:1079/:1392/:1438/:3361/:3446) — all represent fatal states in which Node would not emit beforeExit, so the gate can't over-suppress. The handled-exception path at :1416–1420 does not bump the counter, so beforeExit still fires when a listener swallows the error (verified by the second test, and consistent with the existing zeroExitWithUncaughtHandler test).

Other factors

• My earlier inline concern (the re-dispatch at :1476 wasn't gated, so a worker could re-emit after a fatal) was addressed in 6056186; the author noted the worker's on_unhandled_rejection already arms termination first, but agreed the symmetric guard is the right shape. That thread is resolved.
• The watcher caller (run_command.rs:1571) still falls through to tick_possibly_forever(); the PR description reports hot.test.ts "should recover from errors" passes, and any lingering unhandled_error_counter behavior across hot reloads is pre-existing and unchanged by this diff.
• Tests follow harness conventions: Bun.spawn + bunEnv, Promise.all on all three pipes, exact stdout before exitCode, toInclude/not.toInclude on stderr rather than exact-empty.
• The bug hunting system found nothing on this revision.

[claude]

My earlier concern (the un-gated re-dispatch on the worker path) was addressed in 6056186, and 8aa9fd5 arms exit_on_uncaught_exception in the early-return so the exit-listener-throws-after-fatal case is preserved. I didn't find further issues, but this is VM shutdown logic reached from four different caller contexts (main run loop, watcher loop, worker exit, repl) with an acknowledged partial side-effect on the unhandled-rejection path, so it's worth a human look.

What was reviewed:

• Traced all four on_before_exit() callers; the watcher path's flag-arming is behavior-preserving (previously armed via the unconditional Process__dispatchOnBeforeExit → Bun__VirtualMachine__exitDuringUncaughtException).
• Confirmed exit_on_uncaught_exception is set-only (never cleared), so arming it on the early-return branch cannot regress a later reset.
• Checked unhandled_error_counter write sites; the gate correctly excludes the handled-by-uncaughtException path (counter isn't bumped there), matching the second new test.

Extended reasoning...

Overview

This PR gates both 'beforeExit' dispatch sites in VirtualMachine::on_before_exit() on unhandled_error_counter == 0, so the event is skipped after a default-fatal uncaught exception (Node parity). The early-return branch also arms exit_on_uncaught_exception since the skipped Process__dispatchOnBeforeExit was that flag's sole setter. Three subprocess tests are added to test/js/node/process/process.test.js covering the fatal case, the handled-by-uncaughtException control, and the exit-listener-throws-after-fatal regression guard.

Security risks

None. This is process-lifecycle event ordering; no untrusted input, auth, or crypto is involved.

Level of scrutiny

High. on_before_exit() is called from four distinct contexts — the main run loop (run_command.rs:1631), the watcher loop (run_command.rs:1571, in an infinite loop), the worker drain (web_worker.rs:1196), and the repl (repl_command.rs:245). Each has different semantics around what happens after the call returns, and exit_on_uncaught_exception once armed is never cleared. The change is small and I traced each caller: the watcher path is behavior-preserving on the flag (it was already being armed by the previous unconditional dispatch), and the PR reports hot.test.ts "should recover from errors" passes. The worker re-dispatch guard I raised earlier was added in 6056186. The repl/bun -e path is what the new tests exercise directly.

Other factors

• My prior inline finding is resolved; the current diff gates both dispatch sites symmetrically.
• The PR description candidly notes a partial side-effect: unhandled_error_counter is also bumped on some unhandled-rejection paths, so those now also skip beforeExit. That's directionally correct for Node parity but incomplete (deferred to #32814's scope), and no test pins the partial behavior. A maintainer should confirm they're comfortable shipping that intermediate state.
• Tests follow the repo conventions (subprocess spawn with concurrent pipe drain, exact stdout assertions, exit-code asserted last) and sit alongside the existing process.onBeforeExit describe block.
• Given this is core VM shutdown ordering, I'm deferring rather than approving so a human can sanity-check the watcher/worker interactions and the unhandled-rejection side-effect.

[claude]

My earlier concern (the un-gated re-dispatch on workers) was addressed in 6056186, and 8aa9fd5 correctly arms exit_on_uncaught_exception in the early-return branch so the watcher and exit-listener-throws paths are behavior-preserving. No further issues found, but this is process-lifecycle/shutdown code with acknowledged side effects on the unhandled-rejection path and interactions with #32814's backed-out scope, so it's worth a maintainer's eye.

What was reviewed:

• Traced all four on_before_exit() callers (run_command watcher + non-watcher, repl, web_worker) — the early return + explicit flag-arming preserves prior behavior at each.
• Checked that the --watch loop's post-fatal state is unchanged: the flag was already armed by the previous unconditional dispatch, now armed explicitly.
• Confirmed the handled-exception path (uncaughtException listener present) leaves unhandled_error_counter at 0, so beforeExit still fires — covered by the new test.

Extended reasoning...

Overview

The PR gates both beforeExit dispatch sites in VirtualMachine::on_before_exit() on unhandled_error_counter == 0, so 'beforeExit' is skipped after a default-fatal uncaught exception (matching Node's documented behavior). The entry guard also arms exit_on_uncaught_exception to preserve the side effect the skipped Process__dispatchOnBeforeExit call would have had. Three subprocess tests cover the fatal-skip, handled-still-fires, and exit-listener-throws-after-fatal cases.

Security risks

None. This is process-lifecycle event ordering; no untrusted input parsing, no auth/crypto/permissions.

Level of scrutiny

High. on_before_exit() is called from four places (main run loop, watcher loop, REPL, worker) and interacts with uncaught_exception(), exit_on_uncaught_exception, the worker termination path, and the unhandled-rejection counter. The PR description itself calls out a side effect on unhandled-rejection semantics that partially improves Node parity but overlaps with #32814's backed-out scope. Shutdown-sequence bugs in a runtime tend to manifest as hangs, double-emits, or wrong exit codes only under specific listener/throw combinations.

Other factors

• My prior review flagged the missing guard on the inner-loop re-dispatch (worker case); that was addressed in 6056186 with the same unhandled_error_counter == 0 check, and the author noted the worker path already terminates via on_unhandled_rejection before control returns there — so the guard is defensive but consistent.
• 8aa9fd5 added the exit_on_uncaught_exception = true arming plus a regression test proving a throw from an 'exit' listener after a fatal still stops subsequent listeners. I verified this preserves the watcher-loop invariant (the flag was previously armed by the unconditional dispatch; now armed explicitly on the same path).
• Test coverage is solid for the main-thread cases and follows the existing subprocess-test conventions in that file (drain both pipes, assert stdout before exit code).
• The acknowledged partial effect on unhandled rejections and the cross-reference to backed-out work in #32814 make this a good candidate for a maintainer familiar with that history to sign off.