domain: route fs callback throws to uncaughtException, unblocking 4 tests (domain 88%→96%)

CI
21d ago
+2141-224

Fixes #22546

Stacked on #31828. An uncaught error that nothing takes ends the main run at the report. It also adds four upstream node:domain tests.

Problem

  • Bun printed an uncaught error and ran queued callbacks before it exited. Under a pending top-level await it never exits.
  • The exit came from unhandled_error_counter in the liveness check (src/jsc/VirtualMachine.rs). That check also stopped servers after reportError().

Fix

  • VirtualMachine::uncaught_exception ends the main run with status 1 when no listener takes the error. Every native dispatcher reports through it.
  • A caller that goes on says so by name (uncaught_exception_keep_alive). A source lint counts these call sites.
  • Verified: 26 new cases in 8 test files fail on bun 1.4.2 and on the base. Also 320 vendored node tests.
  • Self-reviewed: 7 concerns raised, 4 addressed, 1 rejected, 2 are questions (Notes).

Background

  • Named callers: reportError(), Bun.serve websockets, Bun.listen, Bun.connect, Bun.udpSocket, Bun.spawn ipc, Bun.WebView.
  • Considered the counter in the liveness check plus a fatal call at each node: site. It never exits under a top-level await.

Downsides

  • Behaviour change: an uncaught error or unhandled rejection ends the process at the report. Bun 1.4.2 runs queued work first. Unnamed Bun-native callbacks (Bun.sql, Bun.redis, Bun.spawn onExit) are included.
  • Behaviour change: after a throw from a named caller the process goes on. Bun 1.4.2 exits 1 at the end of the loop turn.
  • Cost: +674 B on linux-x64, 1 host function. The liveness check is 2 instructions longer. Each fs.watch registration allocates 2 more GC cells, each bound dgram socket 4.
Notes

Questions for a maintainer. REVIEW.md says: never change a Bun-native default to fix Node compatibility. landing-prs.md says: a change of an existing default is a breaking change behind a flag. This PR changes two defaults (Downsides, and the table below).

  1. Do you accept the exit at the report for the callbacks that nobody names?
  2. Is the list of named callers the one to ship? The list is data: to move a caller is a one-line change and a count in the inventory.
  3. The runtime change does not need node:domain. Do you want it as a PR of its own against main?

Self-review. Seven concerns. Fixed: a node:cluster worker went on after a socket listener threw (the socket handlers now have an owner). The default rejection mode asked the uncaughtException listeners, against #19874 (removed). The dns.lookup change for falsy hostnames was a copy of #32883 (removed). The body gave the four domain tests as the reason for the source change (corrected: they pass on the base without it). Rejected: to remove the ws once() change as a copy of #37972. The runtime change needs it (see ws below). Open: the two changes of a default need a ruling (questions 1 and 2), and this PR can be the wrong place for the runtime change (question 3).

Why not the counter in the liveness check. The loop reads the counter between turns, so queued ticks, immediates and 0 ms timers run after the throw. Under a pending top-level await the loop does not read it. Each node: site needs a fatal call, and a merge can drop such an edit with no test failing. The crypto guards of this branch were lost that way in merge 9673052985.

Bun-native callers against bun 1.4.2. One script for each family. A release build of the base gives the same column as bun 1.4.2. The callback queues a tick, an immediate and a 0 ms timer, then throws. Nothing listens. A second timer holds the loop.

callbackbun 1.4.2this branch
Bun.sql onconnect, onclosequeued work runs, exit 1 at the end of the loop turnexit 1 at the throw
Bun.redis onconnect, onclosethe sameexit 1 at the throw
Bun.Terminal datathe sameexit 1 at the throw
Bun.spawn onExitthe sameexit 1 at the throw
reportError()the samegoes on, 'beforeExit' fires, status 1
Bun.serve websocket messagethe samegoes on, 'beforeExit' fires, status 1
Bun.listen data, Bun.connect datathe samegoes on, 'beforeExit' fires, status 1
Bun.udpSocket datathe samegoes on, 'beforeExit' fires, status 1
Bun.spawn ipcthe samegoes on, 'beforeExit' fires, status 1

Rows that differ from 1.4.2: 12 of 12 families (6 exit earlier in the same turn, 6 go on by name). Bun.serve fetch and error handlers and stream pull callbacks do not reach the report: both builds answer 500 or reset the connection, and serve the next request.

Unhandled rejections. The default mode keeps its rule from #19874: it does not ask the uncaughtException listeners or the capture callback. It prints the reason and ends the main run at the report. --unhandled-rejections=throw and strict ask the listeners, as before.

Gates on the exit. Only the VM of the main run ends the process. These print, set status 1 and go on: a worker (its parent gets the error), a macro VM, the debugger VM, --hot and --watch, bun test (the isBunTest early return), the interactive REPL and bun build --app (suppress_fatal_uncaught), expect(fn).toThrow() while it reads a rejection. The run command ends the process itself after the rejection of the entry point.

Named callers. reportError() and Bun__reportError (BunObject.rs, ZigGlobalObject.cpp, ExposeNodeModuleGlobals.cpp, the two Bun.WebView backends), Handlers::call_error_handler for the sockets of Bun.listen and Bun.connect, udp_socket.rs, the ipc handler in subprocess.rs, WebSocketServerContext.rs when the server is not a node:http server, Bun__EventLoop__runCallback2, the entry point report in run_command.rs, and the macro runner (Macro.rs).

Owners. A handler table that node code registered takes the default. HandlersOwner::NodeNet marks the socket handlers of node:net (doConnect, doListen, the TLS upgrades). HandlerFlags::NODE_HTTP_SERVER marks the websocket handlers of the ws package on a node:http server.

JS guards that stay. guardCallback and reportUncaughtException (src/js/internal/shared.ts) send a throw to Bun__reportUnhandledError from inside the callback. They stay on net (handshake, onread), dgram, child_process 'message', fs.watch, fs.watchFile and fs.promises.watch. diagnostics_channel and the webstreams adapters rethrow on the next tick. EventTarget listeners report on the next tick, so that the dispatch completes (node does the same).

ws. once() armed two bridges to the native socket: its own, and one through on(), which EventEmitter's once() calls. After the first event the second bridge emitted 'error' with no listener. Bun printed that throw before. It is an uncaught exception now, so two cases of ws-upgrade-events.test.ts fail without this change. Each event has one bridge, and EventEmitter supplies the once semantics. #37972 is the same change against main, and a maintainer asked there for a form without closures.

Macros. In a script the macro runner uses the VM of the program. It marks the promise of the macro as handled before it waits, reports a macro that fails through unhandled_rejection_keep_alive, and hands the error to its caller as a build error, as before. The base printed that error two times. It is printed one time now.

Measurements (linux-x64).

  • Release binary, base 32c7a46021 against 031f2106d8, one toolchain: all sections +674 B (.text +0 B, .bun_builtins +674 B, .rodata +0 B, size -A). The linker pads .text in steps of 512 B. The stripped file is 4096 B smaller, because the padding between segments changed.
  • run_callback and run_callback_with_result: the listing is the same in both builds, 49 and 53 instructions (objdump). The success path of run_callback is 43 instructions.
  • is_event_loop_alive: no compare against unhandled_error_counter. The base has two. With one active handle and nothing else, the path is 40 instructions and 6 conditional branches (base: 38 and 7). The source has one term less, and the compiler lays the function out differently.
  • Polls between the report and exit_group: 10 of 10 scripts at 0. Syscalls between them: 1 (strace, release build).
  • New cases: 26 of 26 pass on this branch. 26 of 26 fail on bun 1.4.2 and on a release build of the base. 25 of them were also run on bun 1.4.3-canary.1+367d939d9, and 25 fail.
  • Bun-native families that differ from bun 1.4.2 and from the base: 12 of 12 (6 exit earlier in the same turn, 6 go on by name).
  • Named callers: 16 lines in 9 Rust files name a keep-alive entry, 6 lines in 4 C++ files name Bun__reportError, and 1 line under src/js names reportError. These are the counts of the inventory.
  • Host functions added: 1 (doListen). HOST_EXPORT entries added: 0. .classes.ts rows added: 0.
  • GC cells (Function and JSLexicalEnvironment) for each fs.watch registration: 3 (base: 1). For each bound dgram socket: 10 (base: 6).
  • Not measured: instructions at run time. perf and valgrind are not available in the build container.

Tests. The new cases are in node-crypto.test.js (5), node-http.test.ts (3), node-http2.test.js (2), zlib.test.js (2), cluster.test.ts (2), ws.test.ts (1), spawn.test.ts (1) and process.test.js (10). production.test.ts has one more case for bun build --app: it passes on the base too, and fails when production.rs does not set the flag. macro-test.test.ts has six cases for a macro that fails in a script. They pass on the base too. All six fail on c04d2f725a, and the two with a rejection after an await fail on d4ad11de70. Earlier commits of this PR added the cases for fs, the fs watchers, dgram, child_process ipc, net, tls, N-API, the REPL and Bun.cron.

Not in this PR.

  • A throw from a node:net 'data' listener goes to the 'error' listeners of the socket first. Node calls no 'error' listener and exits 1. The base does the same as this branch.
  • node:domain routing of crypto and zlib callback throws. The fold reports after the async context is restored, as bun 1.4.2 does. test-crypto-domain.js stays skipped.
  • The status after a report that the run went on after stays 1.
  • Windows and macOS were not probed. bun run rust:check-all covers compilation.

History. The fatal exit inside the report and the keep-alive entries by name first appeared in 3c6ba76462, 87d36b5dc7 and 6deeb745ca of this branch. 9c199eac05 inverted them to a fatal entry that each site must choose. This change goes back to the first form. The counter left the liveness check in 04a67938bd.

The four domain tests: test-domain-implicit-binding, test-domain-implicit-fs, test-domain-no-error-handler-abort-on-uncaught-5 and -9. The last two are byte for byte the tests -4 and -8 with process.nextTick replaced by fs.exists.

Merge notes (base branch took main four times)
  • emit_abort in node_fs_watcher.rs: main independently made the abort-time 'error' event lenient (a fromAbort argument and a listenerCount check in watch.ts), which is the same behavior this branch had reached with a separate event tag. Main's design is kept; this branch's tag and the matching fs.promises.ts filter entry are dropped, leaving only the guardCallback wraps here.
  • N-API: main centralized the post-callback exception check into NapiEnv::surface_exception, returning it for the dispatcher's keep-alive fold. The three sites now pass that result through report_addon_exception, which reports fatally and only lets a termination through to the fold.
  • cron.rs: main moved the tick onto run_callback_with_result; the tick keeps main's shape (entry gate, report before re-arm) with a direct fatal report.
  • child_process.ts: main's internalMessage routing kept inside the try/catch; VirtualMachine.rs liveness picks up main's yield and concurrent task terms; hot_reload is an enum now.
  • Second merge (4de110ea65): main widened the early return at the top of the report to a stopped VM, script disallowed, or a termination exception; it now sits at the top of uncaught_exception_impl, so both the keep-alive and fatal entry points get it. The per-site termination checks that decide control flow (cron's stop, report_addon_exception handing a termination to the fold) stay. The only other conflict was two test blocks appended to the same file.
  • Third merge (c593f6e7ba): main made the rejection wrapper fallible and added unhandled_rejection_as_uncaught_error as the infallible front door (a hostile reason or OOM while describing it reports the reason as is, a termination wins). The bun, strict and throw arms call that helper and keep uncaught_exception_fatal. The TSF now delivers from one deliver function with two landing frames (the main-thread drain and a worker's env teardown), so report_addon_exception moved into deliver and both frames fold only a termination. cron.rs lost its this_ref alias, ws gained controlPayload for ping/pong (kept inside the once-armed bridge), and child_process.ts loads node:os lazily.
  • Fourth merge (13060c01e6): two test files only. repl.test.ts takes main's merged error test and keeps the nextTick test of this branch. node-dns.test.js keeps both sets of imports.
Original description (domain tests only, before the fatal-path work)

Four upstream domain tests convert here that neither of the two changes involved converts on its own.

  • On main: node:domain is a 96-line stub with no Domain constructor, so the tests die before reaching any fs call. The callback fix alone converts nothing.
  • On #31828: node:domain works, but a throw inside an fs callback is dispatched from a promise reaction, so it surfaces as an unhandledRejection and never reaches uncaughtException, which is exactly the machinery domain hooks. The domain PR alone converts nothing.

The shared/fs/dns/crypto half was the same commit as #34660, which carried the measurement (node's exact semantics, byte-identical happy-path timing, a 356-file regression sweep) and has since merged, so it no longer appears in this diff.

Upstream has 50 test-domain-* files; with these four, 48 are vendored and passing. The last two are not domain problems: test-domain-dep0097 needs node:inspector (oven-sh/bun#2445), and test-domain-multi is an http client bug (the domain routing is correct; the client parser does not error on the corrupted chunk header the test writes). --abort-on-uncaught-exception itself is implemented; all 8 vendored abort tests pass.


no test proof · iteration 60 · platform-specific test(s) that do not run on this machine, deferring to CI, which covers all platforms: test/napi/napi.test.ts, test/js/workerd/html-rewriter.test.js, test/js/node/watch/fs.watchFile.test.ts, test/js/node/watch/fs.watch.test.ts, test/js/node/tls/node-tls-server.test.ts, test/js/node/process/process.test.js, test/js/node/net/node-net.test.ts, test/js/node/dgram/node-dgram.test.js, test/js/node/cluster.test.ts, test/js/bun/spawn/spawn.test.ts, test/js/bun/repl/repl.test.ts, test/bundler/bun-build-api.test.ts, test/bake/dev/production.test.ts

Compat impact (upstream node v26.3.0 test files vendored, in-tree = passing)

  • node:domain: 88% → 96% (44 → 48 of 50)