Skip to content

Implement process._fatalException with domain error routing #28665

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
robobun wants to merge 1 commit into from
Closed

Implement process._fatalException with domain error routing #28665

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
67 changes: 66 additions & 1 deletion src/bun.js/bindings/BunProcess.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3457,6 +3457,71 @@
return JSValue::encode(jsUndefined());
}

JSC_DEFINE_HOST_FUNCTION(Process_fatalException, (JSGlobalObject * lexicalGlobalObject, CallFrame* callFrame))
{
auto* globalObject = defaultGlobalObject(lexicalGlobalObject);
auto& vm = JSC::getVM(globalObject);
auto scope = DECLARE_THROW_SCOPE(vm);

JSValue exception = callFrame->argument(0);

// Check if process.domain is set and can handle this error
auto domainIdent = Identifier::fromString(vm, "domain"_s);
auto* process = globalObject->processObject();
JSValue domainValue = process->get(globalObject, domainIdent);
RETURN_IF_EXCEPTION(scope, {});

if (domainValue && !domainValue.isUndefinedOrNull() && domainValue.isObject()) {
auto* domainObj = domainValue.getObject();

// Set domainThrown and domain on the error object
if (exception.isObject()) {
auto* exObj = exception.getObject();
exObj->putDirect(vm, Identifier::fromString(vm, "domainThrown"_s), jsBoolean(true));
Comment thread
claude[bot] marked this conversation as resolved.
exObj->putDirect(vm, domainIdent, domainValue, JSC::PropertyAttribute::DontEnum | 0);
}

// Get listenerCount to check if domain has 'error' listeners
JSValue listenerCountFn = domainObj->get(globalObject, Identifier::fromString(vm, "listenerCount"_s));
RETURN_IF_EXCEPTION(scope, {});

if (listenerCountFn && listenerCountFn.isCallable()) {
auto listenerCountCallData = JSC::getCallData(listenerCountFn);
MarkedArgumentBuffer lcArgs;
lcArgs.append(jsString(vm, String("error"_s)));
JSValue countValue = JSC::call(globalObject, listenerCountFn, listenerCountCallData, domainObj, lcArgs);
RETURN_IF_EXCEPTION(scope, {});

double count = countValue.toNumber(globalObject);
RETURN_IF_EXCEPTION(scope, {});

if (count > 0) {
// Domain has error listeners - emit the error on the domain
JSValue emitFn = domainObj->get(globalObject, Identifier::fromString(vm, "emit"_s));
RETURN_IF_EXCEPTION(scope, {});

if (emitFn && emitFn.isCallable()) {
auto emitCallData = JSC::getCallData(emitFn);
MarkedArgumentBuffer emitArgs;
emitArgs.append(jsString(vm, String("error"_s)));
emitArgs.append(exception);
JSC::call(globalObject, emitFn, emitCallData, domainObj, emitArgs);
RETURN_IF_EXCEPTION(scope, {});
return JSValue::encode(jsBoolean(true));

Check failure on line 3510 in src/bun.js/bindings/BunProcess.cpp

View check run for this annotation

Claude / Claude Code Review

uncaughtExceptionMonitor skipped when domain handles error 

Process_fatalException returns early after routing to a domain error listener without ever emitting the 'uncaughtExceptionMonitor' event, violating Node.js compatibility. In Node.js, 'uncaughtExceptionMonitor' is an unconditional passive observer that must fire before any domain or uncaughtException routing — it cannot be suppressed by a domain handler. Any monitoring tool using process.on('uncaughtExceptionMonitor', ...) alongside a domain error handler will silently fail to receive notificatio
Comment on lines +3498 to +3510

@claude claude Bot Mar 30, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔴 Process_fatalException returns early after routing to a domain error listener without ever emitting the 'uncaughtExceptionMonitor' event, violating Node.js compatibility. In Node.js, 'uncaughtExceptionMonitor' is an unconditional passive observer that must fire before any domain or uncaughtException routing — it cannot be suppressed by a domain handler. Any monitoring tool using process.on('uncaughtExceptionMonitor', ...) alongside a domain error handler will silently fail to receive notifications in Bun.

Extended reasoning...

What the bug is and how it manifests

In Process_fatalException (BunProcess.cpp), when a domain has error listeners and handles the exception, the function calls domain.emit('error', exception) and immediately returns jsBoolean(true) at line 3510. The 'uncaughtExceptionMonitor' event is only emitted inside Bun__handleUncaughtException (lines 1184-1187), which is only reached on the fallthrough path (no domain handled the error, line 3517). This means for any exception handled by a domain, uncaughtExceptionMonitor is never fired.

The specific code path that triggers it

  1. process._fatalException(err) is called while a domain is active
  2. Process_fatalException finds process.domain set to an object domain
  3. domain.listenerCount('error') > 0, so the error is emitted on the domain
  4. The function returns jsBoolean(true) immediately
  5. Bun__handleUncaughtException is never called — uncaughtExceptionMonitor never fires

Why existing code doesn't prevent it

The uncaughtExceptionMonitor emission is entirely encapsulated inside Bun__handleUncaughtException. There is no unconditional pre-routing emission. The domain-handling path at lines 3498-3510 is a pure early-return that bypasses Bun__handleUncaughtException entirely.

What the impact would be

This is a Node.js compatibility regression. APM tools (Sentry, Datadog, New Relic) and monitoring frameworks rely on uncaughtExceptionMonitor to track ALL uncaught exceptions regardless of how they are ultimately handled. A program that registers both a domain error handler and an uncaughtExceptionMonitor listener will never receive monitor events in Bun when exceptions are caught by domains. The monitor is specifically designed to be unsuppressible — it fires even when uncaughtException or domain handlers are present.

Step-by-step proof

In Node.js, uncaughtExceptionMonitor fires first in the fatalException function before any domain routing takes place. Bun's implementation never reaches the emission for domain-handled errors.

How to fix it

Before the domain check in Process_fatalException, explicitly fire the uncaughtExceptionMonitor event if there are listeners, mirroring how Bun__handleUncaughtException does it but unconditionally — before any domain routing logic.

}
Comment on lines +3498 to +3511

@claude claude Bot Mar 30, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔴 The async domain error path in Process_fatalException passes null directly to d.emit("error", null) without a null guard, while the synchronous path through JS emitError() has an explicit if (\!e) e = $ERR_UNHANDLED_ERROR() guard. This creates a behavioral split: throw null inside a d.run() callback works correctly in sync code but causes handlers to receive null in async callbacks (e.g., setImmediate), leading to TypeError: Cannot read properties of null when the handler accesses err.message.

Extended reasoning...

What the bug is and how it manifests

PR #28665 adds domain error routing via a new C++ Process_fatalException implementation. However, the async and sync code paths have an asymmetry: the synchronous path (via emitError() in domain.ts) has a null guard that converts falsy errors into proper ERR_UNHANDLED_ERROR objects, but the async path (via C++ Process_fatalException) has no such guard and passes the raw value straight to d.emit('error', exception).

The specific code path that triggers it

When code throws a falsy value (e.g., null) inside an async callback that was scheduled while a domain was active, the path is:

  1. _wrapWithDomain wraps the async callback (e.g., the setImmediate handler)
  2. The callback throws null
  3. _wrapWithDomain's catch block calls process._fatalException(null)
  4. C++ Process_fatalException receives null as exception
  5. The if (exception.isObject()) guard is skipped (null is not an object in JSC)
  6. emitArgs.append(exception) appends the raw null
  7. d.emit("error", null) fires with null as the error argument
  8. The domain error handler receives null and any access like err.message throws a TypeError

Why existing code doesn't prevent it

The if (exception.isObject()) check in Process_fatalException only guards property-stamping (adding domainThrown and domain properties). It does NOT check whether the exception is falsy before emitting. The null guard that Node.js relies on — if (\!e) e = $ERR_UNHANDLED_ERROR() — exists only in the JS emitError() function, which is only called on the synchronous path.

What the impact would be

Any code that (intentionally or by accident) does throw null, throw undefined, throw 0, or throw "" inside an async callback (timer, I/O, promise, etc.) while a domain is active will cause domain error handlers to receive that falsy value instead of a proper Error. This silently breaks the Node.js contract for domain error handlers and can cause cascading TypeErrors in handlers that expect an Error-like object.

How to fix it

Add a null/falsy guard at the top of Process_fatalException, mirroring the JS guard:

// At the start of Process_fatalException, after getting exception:
if (\!exception || exception.isNull() || exception.isUndefined() || exception.isFalse() ||
    (exception.isNumber() && exception.asNumber() == 0) ||
    (exception.isString() && exception.getString(lexicalGlobalObject)->length() == 0)) {
    // Replace with ERR_UNHANDLED_ERROR equivalent
    exception = ...;
}

Or more simply, mirror the JS: convert the exception to a proper Error if it is falsy before emitting.

Step-by-step proof

Consider this program:

const domain = require('domain');
const d = domain.create();
d.on('error', (err) => {
  console.log('got error:', err);       // prints 'null' instead of an Error
  console.log(err.message);             // throws TypeError: Cannot read properties of null
});
d.run(() => {
  setImmediate(() => { throw null; });  // async throw of null
});
  • In Node.js, d.on('error') would receive an ERR_UNHANDLED_ERROR wrapping the null (sync path also does this via emitError)
  • In Bun with this PR: setImmediate is wrapped by _wrapWithDomain, its catch block calls process._fatalException(null), C++ emits null directly, and the handler crashes on err.message
  • The same throw null inside a synchronous d.run(() => { throw null; }) would correctly produce ERR_UNHANDLED_ERROR because the sync path goes through JS emitError() which has the guard

}
}
Comment thread
coderabbitai[bot] marked this conversation as resolved.
}

// No domain or domain didn't handle it - use standard uncaught exception handling
int handled = Bun__handleUncaughtException(lexicalGlobalObject, exception, 0);
if (!handled) {
Bun__reportUnhandledError(globalObject, JSValue::encode(exception));
}

return JSValue::encode(jsBoolean(handled != 0));
Comment thread
robobun marked this conversation as resolved.
}

JSC_DEFINE_HOST_FUNCTION(Process_setSourceMapsEnabled, (JSC::JSGlobalObject * lexicalGlobalObject, JSC::CallFrame* callFrame))
{
Zig::GlobalObject* globalObject = defaultGlobalObject(lexicalGlobalObject);
Expand Down Expand Up @@ -3970,7 +4035,7 @@
_debugEnd Process_stubEmptyFunction Function 0
_debugProcess Process_stubEmptyFunction Function 0
_eval processGetEval CustomAccessor
_fatalException Process_stubEmptyFunction Function 1
_fatalException Process_fatalException Function 1
_getActiveHandles Process_stubFunctionReturningArray Function 0
_getActiveRequests Process_stubFunctionReturningArray Function 0
_kill Process_functionReallyKill Function 2
Expand Down
72 changes: 57 additions & 15 deletions src/js/node/domain.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,73 +3,115 @@ let EventEmitter;

const ObjectDefineProperty = Object.defineProperty;

// Domain stack for tracking nested domains
const _stack: any[] = [];

// Export Domain
var domain: any = {};
domain._stack = _stack;
domain.active = null;
domain.createDomain = domain.create = function () {
if (!EventEmitter) {
EventEmitter = require("node:events");
}
var d = new EventEmitter();
d.members = [];

function emitError(e) {
e ||= $ERR_UNHANDLED_ERROR();
if (typeof e === "object") {
e.domainEmitter = this;
function emitError(e, thrown?) {
if (!e) e = $ERR_UNHANDLED_ERROR();
if ((typeof e === "object" && e !== null) || typeof e === "function") {
if (this != null) e.domainEmitter = this;
ObjectDefineProperty(e, "domain", {
__proto__: null,
configurable: true,
enumerable: false,
value: domain,
value: d,
writable: true,
});
e.domainThrown = false;
e.domainThrown = thrown === true;
}
d.emit("error", e);
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Comment thread
claude[bot] marked this conversation as resolved.
}
Comment thread
robobun marked this conversation as resolved.

d.add = function (emitter) {
if (emitter.domain === d) {
return;
}
if (emitter.domain && emitter.domain !== d && typeof emitter.domain.remove === "function") {
emitter.domain.remove(emitter);
}
emitter.on("error", emitError);
emitter.domain = d;
d.members.push(emitter);
};
d.remove = function (emitter) {
emitter.removeListener("error", emitError);
if (emitter.domain === d) {
emitter.domain = null;
}
var index = d.members.indexOf(emitter);
if (index !== -1) {
d.members.splice(index, 1);
Comment thread
robobun marked this conversation as resolved.
}
};
Comment thread
coderabbitai[bot] marked this conversation as resolved.
d.bind = function (fn) {
return function () {
var args = Array.prototype.slice.$call(arguments);
try {
fn.$apply(null, args);
return fn.$apply(this, args);
} catch (err) {
emitError(err);
emitError.$call(d, err, true);
}
};
};
Comment on lines 57 to 66

@claude claude Bot Mar 30, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔴 d.bind() and d.intercept() do not call d.enter()/d.exit() around the wrapped function's execution, so process.domain is wrong inside bound/intercepted callbacks; additionally, the wrapper returned by d.bind() is missing a .domain property, breaking any code that inspects bound.domain. Both issues cause incorrect domain association for errors thrown inside callbacks created with these methods.

Extended reasoning...

Missing d.enter()/d.exit() in d.bind() and d.intercept()

The Node.js domain module contract is that any callback wrapped by d.bind() or d.intercept() should execute with that domain active (i.e., process.domain === d) for the duration of the call. The current implementation at lines 107–130 of src/js/node/domain.ts calls fn directly without ever invoking d.enter() before execution or d.exit() in a finally block after. This means process.domain retains whatever value it had before the bound callback was called — which could be null, a different domain, or a stale reference.

Comparison to correct patterns in the same file

Both d.run() (lines 147–157) and _wrapWithDomain() (lines 15–29) implement the correct enter/finally-exit pattern. d.run() calls d.enter() before fn(), catches errors with emitError(), then always calls d.exit() in a finally block. _wrapWithDomain() does the same for timer-patched callbacks. d.bind() and d.intercept() conspicuously omit this pattern despite being the primary public API for domain-aware callbacks.

Missing wrapper.domain = d on the d.bind() return value

Node.js's Domain.prototype.bind explicitly stamps the returned wrapper function with bound.domain = this (set via ObjectDefineProperty with enumerable: false). Code that calls d.bind(fn) and then inspects the returned function's .domain property — or that later removes the binding by checking emitter.domain — will get undefined instead of d. This is a compatibility gap versus Node.js's documented behavior. Note: d.intercept() does not set .domain on its wrapper in Node.js either, so this specific sub-issue is scoped to d.bind() only.

Concrete proof of the enter/exit bug

Step 1: create domain d1, enter it (process.domain === d1). Step 2: create domain d2, call var cb = d2.bind(fn). Step 3: exit d1 (process.domain === null). Step 4: call cb(). Inside fn, process.domain is still null — not d2 as expected. Any throw inside fn calls emitError(err, true), which emits on d2 correctly, but process.domain is wrong throughout fn's execution, so any nested async scheduling (setTimeout, etc.) inside fn would capture null as the active domain rather than d2.

How to fix

In d.bind(), wrap the fn. call with d.enter() before and d.exit() in a finally block (matching the d.run() pattern), and add ObjectDefineProperty(bound, 'domain', { configurable: true, enumerable: false, value: d, writable: true }) on the returned wrapper. In d.intercept(), add the same d.enter()/d.exit() wrapping around the fn. call in the non-error branch.

d.intercept = function (fn) {
return function (err) {
if (err) {
emitError(err);
emitError.$call(d, err);
} else {
var args = Array.prototype.slice.$call(arguments, 1);
try {
fn.$apply(null, args);
return fn.$apply(this, args);
} catch (err) {
emitError(err);
emitError.$call(d, err, true);
}
}
};
};
d.enter = function () {
_stack.push(d);
domain.active = d;
process.domain = d;
return this;
};
d.exit = function () {
var index = _stack.lastIndexOf(d);
if (index !== -1) {
_stack.splice(index);
}
var prev = _stack.length > 0 ? _stack[_stack.length - 1] : null;
Comment thread
robobun marked this conversation as resolved.
domain.active = prev;
process.domain = prev;
return this;
Comment thread
claude[bot] marked this conversation as resolved.
};
d.run = function (fn) {
d.enter();
try {
fn();
} catch (err) {
emitError(err);
emitError.$call(d, err, true);
} finally {
d.exit();
}
return this;
};
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Comment thread
robobun marked this conversation as resolved.
d.dispose = function () {
var members = Array.prototype.slice.$call(this.members);
for (var i = 0; i < members.length; i++) {
d.remove(members[i]);
}
this.removeAllListeners();
return this;
};
d.enter = d.exit = function () {
d.exit();
Comment thread
coderabbitai[bot] marked this conversation as resolved.
return this;
};
return d;
Expand Down
Loading
Loading