domain: route fs callback throws to uncaughtException, unblocking 4 tests (domain 88%→96%)
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_counterin the liveness check (src/jsc/VirtualMachine.rs). That check also stopped servers afterreportError().
Fix
VirtualMachine::uncaught_exceptionends 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.servewebsockets,Bun.listen,Bun.connect,Bun.udpSocket,Bun.spawnipc,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.spawnonExit) 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.watchregistration allocates 2 more GC cells, each bounddgramsocket 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).
- Do you accept the exit at the report for the callbacks that nobody names?
- 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.
- 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.
| callback | bun 1.4.2 | this branch |
|---|---|---|
Bun.sql onconnect, onclose | queued work runs, exit 1 at the end of the loop turn | exit 1 at the throw |
Bun.redis onconnect, onclose | the same | exit 1 at the throw |
Bun.Terminal data | the same | exit 1 at the throw |
Bun.spawn onExit | the same | exit 1 at the throw |
reportError() | the same | goes on, 'beforeExit' fires, status 1 |
Bun.serve websocket message | the same | goes on, 'beforeExit' fires, status 1 |
Bun.listen data, Bun.connect data | the same | goes on, 'beforeExit' fires, status 1 |
Bun.udpSocket data | the same | goes on, 'beforeExit' fires, status 1 |
Bun.spawn ipc | the same | goes 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.textin steps of 512 B. The stripped file is 4096 B smaller, because the padding between segments changed. run_callbackandrun_callback_with_result: the listing is the same in both builds, 49 and 53 instructions (objdump). The success path ofrun_callbackis 43 instructions.is_event_loop_alive: no compare againstunhandled_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 undersrc/jsnamesreportError. These are the counts of the inventory. - Host functions added: 1 (
doListen).HOST_EXPORTentries added: 0..classes.tsrows added: 0. - GC cells (Function and JSLexicalEnvironment) for each
fs.watchregistration: 3 (base: 1). For each bounddgramsocket: 10 (base: 6). - Not measured: instructions at run time.
perfandvalgrindare 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:domainrouting of crypto and zlib callback throws. The fold reports after the async context is restored, as bun 1.4.2 does.test-crypto-domain.jsstays skipped.- The status after a report that the run went on after stays 1.
- Windows and macOS were not probed.
bun run rust:check-allcovers 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_abortinnode_fs_watcher.rs: main independently made the abort-time 'error' event lenient (afromAbortargument and alistenerCountcheck inwatch.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 matchingfs.promises.tsfilter entry are dropped, leaving only theguardCallbackwraps 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 throughreport_addon_exception, which reports fatally and only lets a termination through to the fold. cron.rs: main moved the tick ontorun_callback_with_result; the tick keeps main's shape (entry gate, report before re-arm) with a direct fatal report.child_process.ts: main'sinternalMessagerouting kept inside the try/catch;VirtualMachine.rsliveness picks up main's yield and concurrent task terms;hot_reloadis 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_exceptionhanding 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_erroras the infallible front door (a hostile reason or OOM while describing it reports the reason as is, a termination wins). Thebun,strictandthrowarms call that helper and keepuncaught_exception_fatal. The TSF now delivers from onedeliverfunction with two landing frames (the main-thread drain and a worker's env teardown), soreport_addon_exceptionmoved intodeliverand both frames fold only a termination.cron.rslost itsthis_refalias,wsgainedcontrolPayloadfor ping/pong (kept inside the once-armed bridge), andchild_process.tsloadsnode:oslazily. - Fourth merge (13060c01e6): two test files only.
repl.test.tstakes main's merged error test and keeps the nextTick test of this branch.node-dns.test.jskeeps 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:domainis a 96-line stub with noDomainconstructor, so the tests die before reaching any fs call. The callback fix alone converts nothing. - On #31828:
node:domainworks, but a throw inside an fs callback is dispatched from a promise reaction, so it surfaces as anunhandledRejectionand never reachesuncaughtException, 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)