fix(node): match domain uncaught exception handling

[nathanwhit]

Summary

• route uncaught errors from child domains without an error listener, and errors thrown by nested domain handlers, to the parent domain
• let top-level domains without an error listener reach process.on("uncaughtException"), while preserving fatal top-level handler throws
• clear the complete domain stack after handled uncaught errors and keep exception capture aligned with domain listener changes

Context

The compatibility layer installed its exception capture callback for every entered domain and always emitted error from the active domain. This diverged from Node in several related cases: a child without a listener could crash instead of reaching its parent, a top-level domain without a listener could bypass uncaughtException, and handled errors could leave parent domains active on a later turn.

This ports the Node top-level/nested handler split, listener-aware capture, process-level cleanup hook, parent recursion, and end-of-turn stack cleanup without changing balanced run(), enter(), and exit() behavior.

Validation

• CARGO_INCREMENTAL=0 CARGO_PROFILE_DEV_DEBUG=0 CARGO_PROFILE_DEV_SPLIT_DEBUGINFO=off cargo build -p deno -p test_server -j 2
• target/debug/deno test --import-map ../deno/import_map.json --no-lock --allow-all tests/unit_node/domain_test.ts (14 passed)
• target/debug/deno lint ext/node/polyfills/domain.ts tests/unit_node/domain_test.ts
• dprint check ext/node/polyfills/domain.ts tests/unit_node/domain_test.ts
• Node v26.7.0 parity repros for a no-listener child, non-adjacent re-entry, three nested throwing handlers, and top-level process/fatal paths

[bartlomieju]

I diffed this against Node v22's lib/domain.js and ran the scenarios against real node v24 and deno 2.9.5. It's a faithful port of Domain.prototype._errorHandler, and it fixes more than the description claims — see the comment on the catch block, which I think is the headline here and currently has no test.

Things it gets right that were easy to get wrong:

• clearDomainStack() does stack.length = 0 rather than stack = []. That matters, because _stack is an exported alias (lines 435/441) that would dangle on reassignment. Matches Node's domainUncaughtExceptionClear exactly.
• The exit loop terminates even in inconsistent states. Node's exit() early-returns on index === -1, so Node's own while (exports.active === this) would spin forever if active pointed at a domain absent from the stack — the polyfill's exit() recomputes active unconditionally, so it can't.
• The recursion terminates: each level's exit loop strictly shrinks the stack, bottoming out at throw handlerError.
• dprint check passes on both files, as claimed.
• checkNoDomainOnLaterTurn creating the promise reaction before any domain is entered, with a comment saying why, is a real subtlety handled well.

One process note: the PR is explicitly about matching Node semantics, but Validation lists only Deno runs. Running these three scenarios against real node would be strong evidence — and would have surfaced the first three comments below.

[bartlomieju]

This fixes a real Node-compat bug that the description doesn't claim and nothing tests. I'd lead with it — it's the best evidence the change is right.

Nested domains where the child has no error listener:

const parent = domain.create(), child = domain.create();
parent.on("error", (e) => console.log("parent handled:", e.message));
parent.run(() => setTimeout(() => {
  child.enter();
  throw new Error("child cb failed");
}, 0));

• node v24 → parent handled: child cb failed
• deno 2.9.5 (main) → hard crash, Uncaught Error: child cb failed

With this PR: child.emit("error", er) throws (EventEmitter's no-listener behavior), this catch picks it up, and it routes to parent — matching Node. Node arrives at the same place by the same mechanism; its else branch comment enumerates "3. It throws, caught = false" and falls into the identical stack.length recursion.

I confirmed the routing works here: a Domain instance's own .domain stays null (EventEmitter.init skips Domain.prototype instances), so curDomain.emit takes the plain-eventEmit path in the patched emitter and genuinely throws rather than being swallowed.

Worth adding as a test case and calling out in the description.

[bartlomieju]

Node's _errorHandler splits on stack.length === 0 and guards the top-level emit; this ports the else branch only, so it always emits:

if (stack.length === 0) {
  // If there's no error handler, do not emit an 'error' event
  // as this would throw an error, make the process exit, and thus
  // prevent the process 'uncaughtException' event from being emitted
  // if a listener is set.
  if (this.listenerCount('error') > 0) { ... }
}

That comment describes an observable divergence. Top-level domain, no error listener, process.on('uncaughtException') registered:

• node v24 → uncaughtException fired: boom, exit 0
• deno 2.9.5 → hard crash, the listener never fires

This is pre-existing — the old code also bare-emitted — so not a regression from this PR, and fine to leave for a follow-up. But since you're porting _errorHandler anyway, it's the natural moment.

Heads up that it's coupled to updateExceptionCapture, which gates on stack.length > 0 where Node gates on ArrayPrototypeEvery(stack, (d) => d.listenerCount('error') === 0). Adding the listenerCount guard alone won't help, because the polyfill installs the capture callback even when no domain has a listener.

[bartlomieju]

Node calls updateExceptionCapture() as the first statement of its catch (er2), before either recursing or rethrowing. Dropped here.

Low impact — the polyfill's enter/exit already call it, so state should be in sync — but it's a deliberate re-sync in the code being ported, so worth a comment if the omission is intentional rather than incidental.

[bartlomieju]

Nit: naming this domainUncaughtExceptionClear (Node's name for the identical function) would make the correspondence greppable. Matters more than usual in a file that's a line-by-line port — same reasoning for while (process.domain === curDomain) on line 293, where Node reads exports.active === this. Both purely cosmetic; active and process.domain are kept in sync.

[bartlomieju]

This asserts null, but real Node produces undefined here — exit() sets exports.active = stack.length === 0 ? undefined : ..., while domainUncaughtExceptionClear sets null. Confirmed on v24 with this PR's own scenario: process.domain is undefined inside the parent handler, null on a later turn. The polyfill uses null uniformly.

So this assertion would fail against Node. Given the PR is specifically about Node parity, assert(x == null) in the helpers would let the suite double as a parity check instead of pinning a Deno-specific value.

[bartlomieju]

The three new tests are well-targeted, and asserting the active domain inside handlers rather than just the message is what gives them teeth. The assertNoActiveDomain probe is a nice trick — entering and exiting a fresh domain splices from that domain's index, so any stale entry underneath surfaces as a non-null active.

Gaps worth closing, roughly in priority order:

• The no-listener child case (see the comment on domain.ts:299) — the behavior this PR actually fixes.
• Non-adjacent re-entry: stack [A, B, A] with A active. The loop comment says "adjacent", so the bottom A survives it and is only removed by clearDomainStack(). Correct, but load-bearing and unpinned.
• A 3-deep chain of throwing handlers, showing the recursion terminates rather than looping.
• The throw handlerError path — top-level handler itself throws.
• dispose()d domains still taking the early throw er.

[bartlomieju]

Nit: unknown | null collapses to unknown, so the | null is a no-op here and on line 13.

[bartlomieju]

Checked all three claimed semantics against Node v24.18.0 with a nested-domain repro (parent run → setTimeout → child.run(() => { throw })), and they hold:

1. Handler runs in the parent, not the child. Node prints CHILD handler: async boom | process.domain: parent. The while (process.domain === curDomain) curDomain.exit() loop reproduces that, and the loop (rather than a single exit()) is needed because a domain entered twice leaves an earlier occurrence behind.
2. A throw from the child's handler reaches the parent. Node prints PARENT handler: from child handler, exit code 0. The recursive domainUncaughtExceptionHandler(handlerError) after setting active to the new top matches, and it terminates because each level pops at least one stack entry and the empty-stack case rethrows.
3. The stack is cleared afterwards. Node reports process.domain as unset on a later turn; clearDomainStack() matches. This is the actual bug being fixed — the old code removed only curDomain, so a parent stayed active and leaked onto unrelated work in a later tick.

Also confirmed the while loop can't spin: process.domain is only ever assigned from stack[stack.length - 1] or null, so process.domain === curDomain implies lastIndexOf finds it and the splice makes progress.

Nice to see the ArrayPrototypeLastIndexOf/ArrayPrototypeSplice hand-rolled stack surgery replaced by the domain's own exit(). LGTM.