From 1d249478f1e391846b27b7504cccd0d8498c9e9c Mon Sep 17 00:00:00 2001 From: Matteo Collina Date: Mon, 15 Dec 2025 20:04:12 +0100 Subject: [PATCH 1/6] domain: port to AsyncLocalStorage Port the domain module from createHook (async_hooks) to AsyncLocalStorage using the AsyncContextFrame-based implementation. Key changes: - Use AsyncLocalStorage for domain context propagation instead of async_hooks.createHook() - Lazy initialization that triggers AsyncContextFrame prototype swap on first domain use - Use enterWith instead of ALS.run() so domain context is NOT automatically restored on exception - this matches the original domain.run() behavior where exit() only runs on success - Add ERR_ASYNC_RESOURCE_DOMAIN_REMOVED error for AsyncResource.domain - Update DEP0097 to End-of-Life status - Remove tests that relied on the removed MakeCallback domain property The domain module now uses the AsyncContextFrame version of AsyncLocalStorage directly for proper context propagation across async boundaries. --- doc/api/deprecations.md | 12 +- doc/api/errors.md | 8 + lib/async_hooks.js | 5 + lib/domain.js | 432 ++++++++++++------ lib/internal/errors.js | 3 + .../make-callback-domain-warning/binding.cc | 32 -- .../make-callback-domain-warning/binding.gyp | 9 - .../make-callback-domain-warning/test.js | 43 -- test/addons/make-callback-recurse/test.js | 66 +-- .../test_make_callback_recurse/test.js | 67 +-- .../parallel/test-domain-async-id-map-leak.js | 43 +- test/parallel/test-domain-dep0097.js | 17 - .../test-domain-emit-error-handler-stack.js | 10 +- ...tack-empty-in-process-uncaughtexception.js | 4 +- test/parallel/test-repl-top-level-await.js | 2 +- ...st-timers-reset-process-domain-on-throw.js | 4 +- 16 files changed, 349 insertions(+), 408 deletions(-) delete mode 100644 test/addons/make-callback-domain-warning/binding.cc delete mode 100644 test/addons/make-callback-domain-warning/binding.gyp delete mode 100644 test/addons/make-callback-domain-warning/test.js delete mode 100644 test/parallel/test-domain-dep0097.js diff --git a/doc/api/deprecations.md b/doc/api/deprecations.md index 78ff6d5222b006..0fc521058a8af4 100644 --- a/doc/api/deprecations.md +++ b/doc/api/deprecations.md @@ -2272,16 +2272,20 @@ Type: End-of-Life -Type: Runtime +Type: End-of-Life -Users of `MakeCallback` that add the `domain` property to carry context, -should start using the `async_context` variant of `MakeCallback` or -`CallbackScope`, or the high-level `AsyncResource` class. +The `domain` property on async resources and `MakeCallback` has been removed. +The domain module now uses `AsyncLocalStorage` for context propagation instead +of `async_hooks`. Accessing the `domain` property on `AsyncResource` will throw +an error. Use `AsyncLocalStorage` instead for context propagation. ### DEP0098: AsyncHooks embedder `AsyncResource.emitBefore` and `AsyncResource.emitAfter` APIs diff --git a/doc/api/errors.md b/doc/api/errors.md index d92521755e4e6a..acfa3e6ae4edcb 100644 --- a/doc/api/errors.md +++ b/doc/api/errors.md @@ -710,6 +710,14 @@ An attempt was made to register something that is not a function as an An operation related to module loading is customized by an asynchronous loader hook that never settled the promise before the loader thread exits. + + +### `ERR_ASYNC_RESOURCE_DOMAIN_REMOVED` + +The `domain` property on `AsyncResource` has been removed. The domain module +now uses `AsyncLocalStorage` for context propagation instead of `async_hooks`. +Use `AsyncLocalStorage` instead for context propagation. + ### `ERR_ASYNC_TYPE` diff --git a/lib/async_hooks.js b/lib/async_hooks.js index dcb046e13e3388..5e1f0ee3f07ae9 100644 --- a/lib/async_hooks.js +++ b/lib/async_hooks.js @@ -16,6 +16,7 @@ const { const { ERR_ASYNC_CALLBACK, + ERR_ASYNC_RESOURCE_DOMAIN_REMOVED, ERR_ASYNC_TYPE, ERR_INVALID_ASYNC_ID, } = require('internal/errors').codes; @@ -262,6 +263,10 @@ class AsyncResource { type ||= fn.name; return (new AsyncResource(type || 'bound-anonymous-fn')).bind(fn, thisArg); } + + get domain() { + throw new ERR_ASYNC_RESOURCE_DOMAIN_REMOVED(); + } } // Placing all exports down here because the exported classes won't export diff --git a/lib/domain.js b/lib/domain.js index 7dd16ee1bf59ef..394abbefb21aee 100644 --- a/lib/domain.js +++ b/lib/domain.js @@ -27,7 +27,6 @@ // unless they address existing, critical bugs. const { - ArrayPrototypeEvery, ArrayPrototypeIndexOf, ArrayPrototypeLastIndexOf, ArrayPrototypePush, @@ -36,12 +35,9 @@ const { Error, FunctionPrototypeCall, ObjectDefineProperty, - Promise, ReflectApply, - SafeMap, - SafeWeakMap, + SafeSet, StringPrototypeRepeat, - Symbol, } = primordials; const EventEmitter = require('events'); @@ -50,72 +46,56 @@ const { ERR_DOMAIN_CANNOT_SET_UNCAUGHT_EXCEPTION_CAPTURE, ERR_UNHANDLED_ERROR, } = require('internal/errors').codes; -const { createHook } = require('async_hooks'); -const { useDomainTrampoline } = require('internal/async_hooks'); -const kWeak = Symbol('kWeak'); -const { WeakReference } = require('internal/util'); +// Use AsyncLocalStorage for domain context propagation +// Lazy initialization - ALS is created on first domain use +let domainStorage; -// Overwrite process.domain with a getter/setter that will allow for more -// effective optimizations -const _domain = [null]; +function initDomainStorage() { + if (domainStorage === undefined) { + // Import AsyncContextFrame and trigger enabled check to ensure prototype swap + const AsyncContextFrame = require('internal/async_context_frame'); + // eslint-disable-next-line no-unused-expressions + AsyncContextFrame.enabled; // Triggers prototype swap from inactive to active + + // Import the async_context_frame-based ALS directly to avoid circular deps + const AsyncLocalStorage = require('internal/async_local_storage/async_context_frame'); + + // Single AsyncLocalStorage instance for all domain context + // Store structure: { domain: Domain | null, stack: Domain[] } + domainStorage = new AsyncLocalStorage(); + } + return domainStorage; +} + +// Helper functions to get/set domain store +function getDomainStore() { + return initDomainStorage().getStore(); +} + +function setDomainStore(store) { + initDomainStorage().enterWith(store); +} + +// Overwrite process.domain with a getter/setter backed by native context ObjectDefineProperty(process, 'domain', { __proto__: null, enumerable: true, get: function() { - return _domain[0]; + const store = getDomainStore(); + // Return undefined when no domain is active (store doesn't exist OR domain is null) + // This makes the API consistent: undefined = no domain, Domain object = domain active + const domain = store?.domain; + return domain === null ? undefined : domain; }, set: function(arg) { - return _domain[0] = arg; - }, -}); - -const vmPromises = new SafeWeakMap(); -const pairing = new SafeMap(); -const asyncHook = createHook({ - init(asyncId, type, triggerAsyncId, resource) { - if (process.domain !== null && process.domain !== undefined) { - // If this operation is created while in a domain, let's mark it - pairing.set(asyncId, process.domain[kWeak]); - // Promises from other contexts, such as with the VM module, should not - // have a domain property as it can be used to escape the sandbox. - if (type !== 'PROMISE' || resource instanceof Promise) { - ObjectDefineProperty(resource, 'domain', { - __proto__: null, - configurable: true, - enumerable: false, - value: process.domain, - writable: true, - }); - // Because promises from other contexts don't get a domain field, - // the domain needs to be held alive another way. Stuffing it in a - // weakmap connected to the promise lifetime can fix that. - } else { - vmPromises.set(resource, process.domain); - } + const store = getDomainStore(); + if (store) { + // Create new store to avoid affecting other async contexts + setDomainStore({ domain: arg, stack: store.stack }); } - }, - before(asyncId) { - const current = pairing.get(asyncId); - if (current !== undefined) { // Enter domain for this cb - // We will get the domain through current.get(), because the resource - // object's .domain property makes sure it is not garbage collected. - // However, we do need to make the reference to the domain non-weak, - // so that it cannot be garbage collected before the after() hook. - current.incRef(); - current.get().enter(); - } - }, - after(asyncId) { - const current = pairing.get(asyncId); - if (current !== undefined) { // Exit domain for this cb - const domain = current.get(); - current.decRef(); - domain.exit(); - } - }, - destroy(asyncId) { - pairing.delete(asyncId); // cleaning up + // If no store exists, setting is a no-op (will be set when domain is entered) + return arg; }, }); @@ -137,49 +117,143 @@ process.setUncaughtExceptionCaptureCallback = function(fn) { }; -let sendMakeCallbackDeprecation = false; -function emitMakeCallbackDeprecation({ target, method }) { - if (!sendMakeCallbackDeprecation) { - process.emitWarning( - 'Using a domain property in MakeCallback is deprecated. Use the ' + - 'async_context variant of MakeCallback or the AsyncResource class ' + - 'instead. ' + - `(Triggered by calling ${method?.name || ''} ` + - `on ${target?.constructor?.name}.)`, - 'DeprecationWarning', 'DEP0097'); - sendMakeCallbackDeprecation = true; +// Helper to get the current stack from native context +function getStack() { + const store = getDomainStore(); + return store?.stack ?? []; +} + +// Helper to set the stack in native context +function setStack(newStack) { + const store = getDomainStore(); + if (store) { + // Create new store to avoid affecting other async contexts + setDomainStore({ domain: store.domain, stack: newStack }); } } -function topLevelDomainCallback(cb, ...args) { - const domain = this.domain; - if (exports.active && domain) - emitMakeCallbackDeprecation({ target: this, method: cb }); +// It's possible to enter one domain while already inside +// another one. The stack is each entered domain. +// For backward compatibility, exports._stack is a getter/setter +ObjectDefineProperty(exports, '_stack', { + __proto__: null, + enumerable: true, + get: getStack, + set: setStack, +}); - if (domain) - domain.enter(); - const ret = ReflectApply(cb, this, args); - if (domain) - domain.exit(); +// Module-level tracking of the current domain and stack for exception handling. +// This is needed because when exceptions propagate out of domainStorage.run(), +// the ALS context is restored before the exception capture callback runs. +// For async callbacks, ALS context is still available, but for synchronous +// exceptions we need this fallback. +let currentDomain = null; +let currentStack = null; // The stack at the time of the current sync operation + +// Track all domains that have error listeners. This is needed because async +// callbacks may fire with a domain context even when the domain is not on the +// current stack. We install the exception handler when ANY domain has listeners. +const domainsWithListeners = new SafeSet(); + +// The domain exception handler - only installed when a domain with error listeners +// exists. This allows uncaughtException event to fire when no handler. +function domainExceptionHandler(er) { + // For sync exceptions (from run/bound/intercepted), currentDomain is set to the + // domain that was active when the exception was thrown. Check it first because + // by the time we get here, the ALS context may have already been restored to the + // outer domain. + // For async callbacks, currentDomain is null and process.domain reads from ALS. + const domain = currentDomain || process.domain; + + // For sync exceptions, we need to restore the ALS context before calling _errorHandler + // because ALS unwinds during exception propagation. This ensures exit() works correctly. + if (currentDomain !== null && currentStack !== null) { + // Restore the ALS context and exports.active to match the state at the time of the throw + setDomainStore({ domain: currentDomain, stack: currentStack }); + exports.active = currentDomain; + } - return ret; + // First check the current domain + if (domain && domain.listenerCount('error') > 0) { + return domain._errorHandler(er); + } + + // For SYNC exceptions only (currentDomain is set), check parent domains on the stack. + // This handles the case where d2 (no handler) throws synchronously inside d (has handler). + // For ASYNC exceptions (currentDomain is null), only the domain from ALS matters - + // the exception should NOT bubble up to parent domains that were on the stack when + // the async callback was scheduled. + if (currentDomain !== null && currentStack !== null) { + for (let i = currentStack.length - 1; i >= 0; i--) { + const parentDomain = currentStack[i]; + if (parentDomain !== domain && parentDomain.listenerCount('error') > 0) { + return parentDomain._errorHandler(er); + } + } + } + + // No domain with error handler found - let the exception propagate. + // Since setUncaughtExceptionCaptureCallback ignores our return value, + // we must emit 'uncaughtException' manually to give other handlers a chance. + // We temporarily uninstall ourselves to avoid infinite recursion if someone + // throws from their uncaughtException handler. + uninstallExceptionHandler(); + + // Emit uncaughtException to give process.on('uncaughtException') handlers a chance + const handled = process.emit('uncaughtException', er, 'uncaughtException'); + + if (handled) { + // Reinstall ourselves for future exceptions + installExceptionHandler(); + } else { + // No handler caught the exception - we need to exit. + // Throw it to let the C++ side handle the exit properly (respects --abort-on-uncaught). + // Don't reinstall the callback - we want the throw to propagate directly to C++. + throw er; + } } -// It's possible to enter one domain while already inside -// another one. The stack is each entered domain. -let stack = []; -exports._stack = stack; -useDomainTrampoline(topLevelDomainCallback); +// Track if exception handler is currently installed +let exceptionHandlerInstalled = false; -function updateExceptionCapture() { - if (ArrayPrototypeEvery(stack, - (domain) => domain.listenerCount('error') === 0)) { +function installExceptionHandler() { + if (!exceptionHandlerInstalled) { + setUncaughtExceptionCaptureCallback(domainExceptionHandler); + exceptionHandlerInstalled = true; + } +} + +function uninstallExceptionHandler() { + if (exceptionHandlerInstalled) { setUncaughtExceptionCaptureCallback(null); + exceptionHandlerInstalled = false; + } +} + +function updateExceptionCapture() { + // Install exception handler if ANY domain has error listeners + if (domainsWithListeners.size > 0) { + installExceptionHandler(); } else { - setUncaughtExceptionCaptureCallback(null); - setUncaughtExceptionCaptureCallback((er) => { - return process.domain._errorHandler(er); - }); + uninstallExceptionHandler(); + } +} + +// Called when an 'error' listener is being added +// 'newListener' is called BEFORE listener is added, so count is still 0 +function domainNewListener(type) { + if (type === 'error') { + domainsWithListeners.add(this); + updateExceptionCapture(); + } +} + +// Called when an 'error' listener is being removed +// 'removeListener' is called AFTER listener is removed, so count reflects new state +function domainRemoveListener(type) { + if (type === 'error' && this.listenerCount('error') === 0) { + domainsWithListeners.delete(this); + updateExceptionCapture(); } } @@ -205,8 +279,13 @@ process.on('removeListener', (name, listener) => { }); function domainUncaughtExceptionClear() { - stack.length = 0; - exports.active = process.domain = null; + // Clear the stack and active domain in the CURRENT context only + // Don't mutate the store in-place as it would affect other async contexts + // that share the same store reference + setDomainStore({ domain: null, stack: [] }); + exports.active = null; + currentDomain = null; + currentStack = null; updateExceptionCapture(); } @@ -216,11 +295,9 @@ class Domain extends EventEmitter { super(); this.members = []; - this[kWeak] = new WeakReference(this); - asyncHook.enable(); - this.on('removeListener', updateExceptionCapture); - this.on('newListener', updateExceptionCapture); + this.on('newListener', domainNewListener); + this.on('removeListener', domainRemoveListener); } } @@ -256,6 +333,8 @@ Domain.prototype._errorHandler = function(er) { this.exit(); } + const stack = getStack(); + // The top-level domain-handler is handled separately. // // The reason is that if V8 was passed a command line option @@ -274,11 +353,11 @@ Domain.prototype._errorHandler = function(er) { // Clear the uncaughtExceptionCaptureCallback so that we know that, since // the top-level domain is not active anymore, it would be ok to abort on // an uncaught exception at this point - setUncaughtExceptionCaptureCallback(null); + uninstallExceptionHandler(); try { caught = this.emit('error', er); } finally { - updateExceptionCapture(); + installExceptionHandler(); } } } else { @@ -298,9 +377,16 @@ Domain.prototype._errorHandler = function(er) { // See if another domain can catch THIS error, // or else crash on the original one. updateExceptionCapture(); - if (stack.length) { - exports.active = process.domain = stack[stack.length - 1]; - caught = process.domain._errorHandler(er2); + const currentStack = getStack(); + if (currentStack.length) { + const parentDomain = currentStack[currentStack.length - 1]; + const store = getDomainStore(); + if (store) { + // Create new store to avoid affecting other async contexts + setDomainStore({ domain: parentDomain, stack: store.stack }); + } + exports.active = parentDomain; + caught = parentDomain._errorHandler(er2); } else { // Pass on to the next exception handler. throw er2; @@ -320,22 +406,33 @@ Domain.prototype._errorHandler = function(er) { Domain.prototype.enter = function() { // Note that this might be a no-op, but we still need // to push it onto the stack so that we can pop it later. - exports.active = process.domain = this; - ArrayPrototypePush(stack, this); + const currentStore = getDomainStore() || { domain: null, stack: [] }; + const newStack = ArrayPrototypeSlice(currentStore.stack); + ArrayPrototypePush(newStack, this); + setDomainStore({ domain: this, stack: newStack }); + exports.active = this; + currentDomain = this; + currentStack = newStack; updateExceptionCapture(); }; Domain.prototype.exit = function() { // Don't do anything if this domain is not on the stack. + const store = getDomainStore(); + if (!store) return; + + const stack = store.stack; const index = ArrayPrototypeLastIndexOf(stack, this); if (index === -1) return; // Exit all domains until this one. - ArrayPrototypeSplice(stack, index); - - exports.active = stack.length === 0 ? undefined : stack[stack.length - 1]; - process.domain = exports.active; + const newStack = ArrayPrototypeSlice(stack, 0, index); + const parentDomain = newStack.length === 0 ? null : newStack[newStack.length - 1]; + setDomainStore({ domain: parentDomain, stack: newStack }); + exports.active = parentDomain; + currentDomain = parentDomain; + currentStack = newStack.length === 0 ? null : newStack; updateExceptionCapture(); }; @@ -385,11 +482,34 @@ Domain.prototype.remove = function(ee) { Domain.prototype.run = function(fn) { - this.enter(); - const ret = ReflectApply(fn, this, ArrayPrototypeSlice(arguments, 1)); - this.exit(); + const currentStore = getDomainStore() || { domain: null, stack: [] }; + const newStack = ArrayPrototypeSlice(currentStore.stack); + ArrayPrototypePush(newStack, this); - return ret; + const args = ArrayPrototypeSlice(arguments, 1); + const previousDomain = currentDomain; + const previousStack = currentStack; + + // Set currentDomain and currentStack before running so exception handler can see them + currentDomain = this; + currentStack = newStack; + updateExceptionCapture(); + + // Use enterWith instead of run() so the context is NOT automatically restored on exception. + // This matches the original domain.run() behavior where exit() only runs on success. + setDomainStore({ domain: this, stack: newStack }); + exports.active = this; + + const result = ReflectApply(fn, this, args); + + // On success, restore context (if exception thrown, context stays for catch blocks) + setDomainStore(currentStore); + exports.active = currentStore.domain; + currentDomain = previousDomain; + currentStack = previousStack; + updateExceptionCapture(); + + return result; }; @@ -409,11 +529,31 @@ function intercepted(_this, self, cb, fnargs) { return; } - self.enter(); - const ret = ReflectApply(cb, _this, ArrayPrototypeSlice(fnargs, 1)); - self.exit(); + const currentStore = getDomainStore() || { domain: null, stack: [] }; + const newStack = ArrayPrototypeSlice(currentStore.stack); + ArrayPrototypePush(newStack, self); + const args = ArrayPrototypeSlice(fnargs, 1); + const previousDomain = currentDomain; + const previousStack = currentStack; - return ret; + currentDomain = self; + currentStack = newStack; + updateExceptionCapture(); + + // Use enterWith instead of run() so the context is NOT automatically restored on exception. + setDomainStore({ domain: self, stack: newStack }); + exports.active = self; + + const result = ReflectApply(cb, _this, args); + + // On success, restore context + setDomainStore(currentStore); + exports.active = currentStore.domain; + currentDomain = previousDomain; + currentStack = previousStack; + updateExceptionCapture(); + + return result; } @@ -429,11 +569,30 @@ Domain.prototype.intercept = function(cb) { function bound(_this, self, cb, fnargs) { - self.enter(); - const ret = ReflectApply(cb, _this, fnargs); - self.exit(); + const currentStore = getDomainStore() || { domain: null, stack: [] }; + const newStack = ArrayPrototypeSlice(currentStore.stack); + ArrayPrototypePush(newStack, self); + const previousDomain = currentDomain; + const previousStack = currentStack; + + currentDomain = self; + currentStack = newStack; + updateExceptionCapture(); - return ret; + // Use enterWith instead of run() so the context is NOT automatically restored on exception. + setDomainStore({ domain: self, stack: newStack }); + exports.active = self; + + const result = ReflectApply(cb, _this, fnargs); + + // On success, restore context + setDomainStore(currentStore); + exports.active = currentStore.domain; + currentDomain = previousDomain; + currentStack = previousStack; + updateExceptionCapture(); + + return result; } @@ -510,30 +669,33 @@ EventEmitter.prototype.emit = function emit(...args) { // handler doesn't run in its own context. This prevents any event emitter // created or any exception thrown in that error handler from recursively // executing that error handler. + const store = getDomainStore(); + const stack = store?.stack ?? []; const origDomainsStack = ArrayPrototypeSlice(stack); - const origActiveDomain = process.domain; + const origActiveDomain = store?.domain ?? null; // Travel the domains stack from top to bottom to find the first domain // instance that is not a duplicate of the current active domain. let idx = stack.length - 1; - while (idx > -1 && process.domain === stack[idx]) { + while (idx > -1 && origActiveDomain === stack[idx]) { --idx; } // Change the stack to not contain the current active domain, and only the // domains above it on the stack. + let newStack; if (idx < 0) { - stack.length = 0; + newStack = []; } else { - ArrayPrototypeSplice(stack, idx + 1); + newStack = ArrayPrototypeSlice(stack, 0, idx + 1); } // Change the current active domain - if (stack.length > 0) { - exports.active = process.domain = stack[stack.length - 1]; - } else { - exports.active = process.domain = null; + const newActiveDomain = newStack.length > 0 ? newStack[newStack.length - 1] : null; + if (store) { + setDomainStore({ domain: newActiveDomain, stack: newStack }); } + exports.active = newActiveDomain; updateExceptionCapture(); @@ -541,8 +703,10 @@ EventEmitter.prototype.emit = function emit(...args) { // Now that the domain's error handler has completed, restore the domains // stack and the active domain to their original values. - exports._stack = stack = origDomainsStack; - exports.active = process.domain = origActiveDomain; + if (store) { + setDomainStore({ domain: origActiveDomain, stack: origDomainsStack }); + } + exports.active = origActiveDomain; updateExceptionCapture(); return false; diff --git a/lib/internal/errors.js b/lib/internal/errors.js index 5fa4437b09e556..acb42326b730f5 100644 --- a/lib/internal/errors.js +++ b/lib/internal/errors.js @@ -1139,6 +1139,9 @@ E('ERR_ASSERTION', '%s', Error); E('ERR_ASYNC_CALLBACK', '%s must be a function', TypeError); E('ERR_ASYNC_LOADER_REQUEST_NEVER_SETTLED', 'Async loader request never settled', Error); +E('ERR_ASYNC_RESOURCE_DOMAIN_REMOVED', + 'The domain property on AsyncResource has been removed. ' + + 'Use AsyncLocalStorage instead.', Error); E('ERR_ASYNC_TYPE', 'Invalid name for async "type": %s', TypeError); E('ERR_BROTLI_INVALID_PARAM', '%s is not a valid Brotli parameter', RangeError); E('ERR_BUFFER_OUT_OF_BOUNDS', diff --git a/test/addons/make-callback-domain-warning/binding.cc b/test/addons/make-callback-domain-warning/binding.cc deleted file mode 100644 index d02c8f517661eb..00000000000000 --- a/test/addons/make-callback-domain-warning/binding.cc +++ /dev/null @@ -1,32 +0,0 @@ -#include "node.h" -#include "v8.h" - -#include - -using v8::Function; -using v8::FunctionCallbackInfo; -using v8::Isolate; -using v8::Local; -using v8::Object; -using v8::Value; - -namespace { - -void MakeCallback(const FunctionCallbackInfo& args) { - assert(args[0]->IsObject()); - assert(args[1]->IsFunction()); - Isolate* isolate = args.GetIsolate(); - Local recv = args[0].As(); - Local method = args[1].As(); - - node::MakeCallback(isolate, recv, method, 0, nullptr, - node::async_context{0, 0}); -} - -void Initialize(Local exports) { - NODE_SET_METHOD(exports, "makeCallback", MakeCallback); -} - -} // namespace - -NODE_MODULE(NODE_GYP_MODULE_NAME, Initialize) diff --git a/test/addons/make-callback-domain-warning/binding.gyp b/test/addons/make-callback-domain-warning/binding.gyp deleted file mode 100644 index 55fbe7050f18e4..00000000000000 --- a/test/addons/make-callback-domain-warning/binding.gyp +++ /dev/null @@ -1,9 +0,0 @@ -{ - 'targets': [ - { - 'target_name': 'binding', - 'sources': [ 'binding.cc' ], - 'includes': ['../common.gypi'], - } - ] -} diff --git a/test/addons/make-callback-domain-warning/test.js b/test/addons/make-callback-domain-warning/test.js deleted file mode 100644 index e6eaa9d337c179..00000000000000 --- a/test/addons/make-callback-domain-warning/test.js +++ /dev/null @@ -1,43 +0,0 @@ -'use strict'; - -const common = require('../../common'); -const assert = require('assert'); -const domain = require('domain'); -const binding = require(`./build/${common.buildType}/binding`); - -function makeCallback(object, cb) { - binding.makeCallback(object, function someMethod() { setImmediate(cb); }); -} - -let latestWarning = null; -process.on('warning', (warning) => { - latestWarning = warning; -}); - -const d = domain.create(); - -class Resource { - constructor(domain) { - this.domain = domain; - } -} - -// When domain is disabled, no warning will be emitted -makeCallback(new Resource(d), common.mustCall(() => { - assert.strictEqual(latestWarning, null); - - d.run(common.mustCall(() => { - // No warning will be emitted when no domain property is applied - makeCallback({}, common.mustCall(() => { - assert.strictEqual(latestWarning, null); - - // Warning is emitted when domain property is used and domain is enabled - makeCallback(new Resource(d), common.mustCall(() => { - assert.match(latestWarning.message, - /Triggered by calling someMethod on Resource\./); - assert.strictEqual(latestWarning.name, 'DeprecationWarning'); - assert.strictEqual(latestWarning.code, 'DEP0097'); - })); - })); - })); -})); diff --git a/test/addons/make-callback-recurse/test.js b/test/addons/make-callback-recurse/test.js index 6b927ffbfdba48..eb797d7153b604 100644 --- a/test/addons/make-callback-recurse/test.js +++ b/test/addons/make-callback-recurse/test.js @@ -2,13 +2,9 @@ const common = require('../../common'); const assert = require('assert'); -const domain = require('domain'); const binding = require(`./build/${common.buildType}/binding`); const makeCallback = binding.makeCallback; -// Make sure this is run in the future. -const mustCallCheckDomains = common.mustCall(checkDomains); - // Make sure that using MakeCallback allows the error to propagate. assert.throws(() => { makeCallback({}, () => { @@ -82,68 +78,8 @@ assert.throws(() => { setTimeout(common.mustCall(() => { verifyExecutionOrder(3); }), 10); - } else if (arg === 3) { - mustCallCheckDomains(); } else { - throw new Error('UNREACHABLE'); + assert.strictEqual(arg, 3); } })); }(1)); - - -function checkDomains() { - // Check that domains are properly entered/exited when called in multiple - // levels from both node::MakeCallback() and AsyncWrap::MakeCallback - setImmediate(common.mustCall(() => { - const d1 = domain.create(); - const d2 = domain.create(); - const d3 = domain.create(); - - makeCallback({ domain: d1 }, common.mustCall(() => { - assert.strictEqual(d1, process.domain); - makeCallback({ domain: d2 }, common.mustCall(() => { - assert.strictEqual(d2, process.domain); - makeCallback({ domain: d3 }, common.mustCall(() => { - assert.strictEqual(d3, process.domain); - })); - assert.strictEqual(d2, process.domain); - })); - assert.strictEqual(d1, process.domain); - })); - })); - - setTimeout(common.mustCall(() => { - const d1 = domain.create(); - const d2 = domain.create(); - const d3 = domain.create(); - - makeCallback({ domain: d1 }, common.mustCall(() => { - assert.strictEqual(d1, process.domain); - makeCallback({ domain: d2 }, common.mustCall(() => { - assert.strictEqual(d2, process.domain); - makeCallback({ domain: d3 }, common.mustCall(() => { - assert.strictEqual(d3, process.domain); - })); - assert.strictEqual(d2, process.domain); - })); - assert.strictEqual(d1, process.domain); - })); - }), 1); - - function testTimer(id) { - // Make sure nextTick, setImmediate and setTimeout can all recover properly - // after a thrown makeCallback call. - const d = domain.create(); - d.on('error', common.mustCall((e) => { - assert.strictEqual(e.message, `throw from domain ${id}`); - })); - makeCallback({ domain: d }, () => { - throw new Error(`throw from domain ${id}`); - }); - throw new Error('UNREACHABLE'); - } - - process.nextTick(common.mustCall(testTimer), 3); - setImmediate(common.mustCall(testTimer), 2); - setTimeout(common.mustCall(testTimer), 1, 1); -} diff --git a/test/node-api/test_make_callback_recurse/test.js b/test/node-api/test_make_callback_recurse/test.js index 6a35f970e34eae..1837f7081137b2 100644 --- a/test/node-api/test_make_callback_recurse/test.js +++ b/test/node-api/test_make_callback_recurse/test.js @@ -2,14 +2,9 @@ const common = require('../../common'); const assert = require('assert'); -const domain = require('domain'); const binding = require(`./build/${common.buildType}/binding`); const makeCallback = binding.makeCallback; -// Make sure this is run in the future. -const mustCallCheckDomains = common.mustCall(checkDomains); - - // Make sure that using MakeCallback allows the error to propagate. assert.throws(function() { makeCallback({}, function() { @@ -83,68 +78,8 @@ assert.throws(function() { setTimeout(common.mustCall(function() { verifyExecutionOrder(3); }), 10); - } else if (arg === 3) { - mustCallCheckDomains(); } else { - throw new Error('UNREACHABLE'); + assert.strictEqual(arg, 3); } })); }(1)); - - -function checkDomains() { - // Check that domains are properly entered/exited when called in multiple - // levels from both node::MakeCallback() and AsyncWrap::MakeCallback - setImmediate(common.mustCall(function() { - const d1 = domain.create(); - const d2 = domain.create(); - const d3 = domain.create(); - - makeCallback({ domain: d1 }, common.mustCall(function() { - assert.strictEqual(d1, process.domain); - makeCallback({ domain: d2 }, common.mustCall(function() { - assert.strictEqual(d2, process.domain); - makeCallback({ domain: d3 }, common.mustCall(function() { - assert.strictEqual(d3, process.domain); - })); - assert.strictEqual(d2, process.domain); - })); - assert.strictEqual(d1, process.domain); - })); - })); - - setTimeout(common.mustCall(function() { - const d1 = domain.create(); - const d2 = domain.create(); - const d3 = domain.create(); - - makeCallback({ domain: d1 }, common.mustCall(function() { - assert.strictEqual(d1, process.domain); - makeCallback({ domain: d2 }, common.mustCall(function() { - assert.strictEqual(d2, process.domain); - makeCallback({ domain: d3 }, common.mustCall(function() { - assert.strictEqual(d3, process.domain); - })); - assert.strictEqual(d2, process.domain); - })); - assert.strictEqual(d1, process.domain); - })); - }), 1); - - function testTimer(id) { - // Make sure nextTick, setImmediate and setTimeout can all recover properly - // after a thrown makeCallback call. - const d = domain.create(); - d.on('error', common.mustCall(function(e) { - assert.strictEqual(e.message, `throw from domain ${id}`); - })); - makeCallback({ domain: d }, function() { - throw new Error(`throw from domain ${id}`); - }); - throw new Error('UNREACHABLE'); - } - - process.nextTick(common.mustCall(testTimer), 3); - setImmediate(common.mustCall(testTimer), 2); - setTimeout(common.mustCall(testTimer), 1, 1); -} diff --git a/test/parallel/test-domain-async-id-map-leak.js b/test/parallel/test-domain-async-id-map-leak.js index 40d051aa275179..96932901a167ab 100644 --- a/test/parallel/test-domain-async-id-map-leak.js +++ b/test/parallel/test-domain-async-id-map-leak.js @@ -1,21 +1,17 @@ -// Flags: --expose-gc 'use strict'; const common = require('../common'); -const { onGC } = require('../common/gc'); -const { gcUntil } = require('../common/gc'); const assert = require('assert'); const async_hooks = require('async_hooks'); const domain = require('domain'); const EventEmitter = require('events'); const isEnumerable = Function.call.bind(Object.prototype.propertyIsEnumerable); -// This test makes sure that the (async id → domain) map which is part of the -// domain module does not get in the way of garbage collection. -// See: https://github.com/nodejs/node/issues/23862 +// This test verifies that: +// 1. emitter.domain works correctly when added to a domain +// 2. resource.domain throws ERR_ASYNC_RESOURCE_DOMAIN_REMOVED + +const d = domain.create(); -let d = domain.create(); -let resourceGCed = false; let domainGCed = false; let - emitterGCed = false; d.run(common.mustCall(() => { const resource = new async_hooks.AsyncResource('TestResource'); const emitter = new EventEmitter(); @@ -23,27 +19,16 @@ d.run(common.mustCall(() => { d.remove(emitter); d.add(emitter); - emitter.linkToResource = resource; + // emitter.domain should work assert.strictEqual(emitter.domain, d); assert.strictEqual(isEnumerable(emitter, 'domain'), false); - assert.strictEqual(resource.domain, d); - assert.strictEqual(isEnumerable(resource, 'domain'), false); - - // This would otherwise be a circular chain now: - // emitter → resource → async id ⇒ domain → emitter. - // Make sure that all of these objects are released: - onGC(resource, { ongc: common.mustCall(() => { resourceGCed = true; }) }); - onGC(d, { ongc: common.mustCall(() => { domainGCed = true; }) }); - onGC(emitter, { ongc: common.mustCall(() => { emitterGCed = true; }) }); + // resource.domain is no longer supported - accessing it throws an error + assert.throws(() => { + return resource.domain; + }, { + code: 'ERR_ASYNC_RESOURCE_DOMAIN_REMOVED', + message: 'The domain property on AsyncResource has been removed. ' + + 'Use AsyncLocalStorage instead.', + }); })); - -d = null; - -async function main() { - await gcUntil( - 'All objects garbage collected', - () => resourceGCed && domainGCed && emitterGCed); -} - -main(); diff --git a/test/parallel/test-domain-dep0097.js b/test/parallel/test-domain-dep0097.js deleted file mode 100644 index 7ed823aa4165af..00000000000000 --- a/test/parallel/test-domain-dep0097.js +++ /dev/null @@ -1,17 +0,0 @@ -'use strict'; -const common = require('../common'); - -common.skipIfInspectorDisabled(); - -const assert = require('assert'); -const domain = require('domain'); -const inspector = require('inspector'); - -process.on('warning', common.mustCall((warning) => { - assert.strictEqual(warning.code, 'DEP0097'); - assert.match(warning.message, /Triggered by calling emit on process/); -})); - -domain.create().run(() => { - inspector.open(0); -}); diff --git a/test/parallel/test-domain-emit-error-handler-stack.js b/test/parallel/test-domain-emit-error-handler-stack.js index c89cf563a69bd6..13faf80a8a04ae 100644 --- a/test/parallel/test-domain-emit-error-handler-stack.js +++ b/test/parallel/test-domain-emit-error-handler-stack.js @@ -36,9 +36,11 @@ function checkExpectedDomains(err) { // Then make sure that the domains stack and active domain is setup as // expected when executing a callback scheduled via nextTick from the error // handler. + // Note: With AsyncLocalStorage-based domains, async callbacks inherit the + // full stack from when they were scheduled, so stack length matches the + // sync context. process.nextTick(() => { - const expectedStackLengthInNextTickCb = - err.expectedStackLength > 0 ? 1 : 0; + const expectedStackLengthInNextTickCb = err.expectedStackLength; if (domain._stack.length !== expectedStackLengthInNextTickCb) { console.error('expected stack length in nextTick cb to be %d, ' + 'but instead is %d', expectedStackLengthInNextTickCb, @@ -77,7 +79,7 @@ d1.run(common.mustCall(() => { const err = new Error('oops'); err.expectedStackLength = 0; - err.expectedActiveDomain = null; + err.expectedActiveDomain = undefined; ee.emit('error', err); assert.strictEqual(process.domain, d1); @@ -93,7 +95,7 @@ d1.run(common.mustCall(() => { const err = new Error('oops'); err.expectedStackLength = 0; - err.expectedActiveDomain = null; + err.expectedActiveDomain = undefined; ee.emit('error', err); assert.strictEqual(process.domain, d1); diff --git a/test/parallel/test-domain-stack-empty-in-process-uncaughtexception.js b/test/parallel/test-domain-stack-empty-in-process-uncaughtexception.js index e9e8ab12d59d2c..8b5468a762e7c2 100644 --- a/test/parallel/test-domain-stack-empty-in-process-uncaughtexception.js +++ b/test/parallel/test-domain-stack-empty-in-process-uncaughtexception.js @@ -8,14 +8,14 @@ const d = domain.create(); process.once('uncaughtException', common.mustCall(function onUncaught() { assert.strictEqual( - process.domain, null, + process.domain, undefined, 'Domains stack should be empty in uncaughtException handler ' + `but the value of process.domain is ${JSON.stringify(process.domain)}`); })); process.on('beforeExit', common.mustCall(function onBeforeExit() { assert.strictEqual( - process.domain, null, + process.domain, undefined, 'Domains stack should be empty in beforeExit handler ' + `but the value of process.domain is ${JSON.stringify(process.domain)}`); })); diff --git a/test/parallel/test-repl-top-level-await.js b/test/parallel/test-repl-top-level-await.js index 0b35443dbdce14..4917dffc0e7674 100644 --- a/test/parallel/test-repl-top-level-await.js +++ b/test/parallel/test-repl-top-level-await.js @@ -180,7 +180,7 @@ async function ordinaryTests() { ['k', '234'], ['const k = await Promise.resolve(345)', "Uncaught SyntaxError: Identifier 'k' has already been declared"], // Regression test for https://github.com/nodejs/node/issues/43777. - ['await Promise.resolve(123), Promise.resolve(456)', 'Promise {', { line: 0 }], + ['await Promise.resolve(123), Promise.resolve(456)', 'Promise { 456 }', { line: 0 }], ['await Promise.resolve(123), await Promise.resolve(456)', '456'], ['await (Promise.resolve(123), Promise.resolve(456))', '456'], ]; diff --git a/test/parallel/test-timers-reset-process-domain-on-throw.js b/test/parallel/test-timers-reset-process-domain-on-throw.js index a0de01f0922964..f4e0d549d7d323 100644 --- a/test/parallel/test-timers-reset-process-domain-on-throw.js +++ b/test/parallel/test-timers-reset-process-domain-on-throw.js @@ -36,8 +36,8 @@ function err() { function secondTimer() { // secondTimer was scheduled before any domain had been created, so its // callback should not have any active domain set when it runs. - if (process.domain !== null) { - console.log('process.domain should be null in this timer callback, but is:', + if (process.domain !== undefined) { + console.log('process.domain should be undefined in this timer callback, but is:', process.domain); // Do not use assert here, as it throws errors and if a domain with an error // handler is active, then asserting wouldn't make the test fail. From 693a83d55016961cf53233b0298056ab960700aa Mon Sep 17 00:00:00 2001 From: Matteo Collina Date: Sat, 20 Dec 2025 23:43:59 +0100 Subject: [PATCH 2/6] async_hooks: remove unused useDomainTrampoline Remove the useDomainTrampoline function and related domain_cb variable that are no longer used after domain was ported to AsyncLocalStorage. --- lib/internal/async_hooks.js | 14 +------------- 1 file changed, 1 insertion(+), 13 deletions(-) diff --git a/lib/internal/async_hooks.js b/lib/internal/async_hooks.js index d698831a1425c7..ef19d81e8bf12c 100644 --- a/lib/internal/async_hooks.js +++ b/lib/internal/async_hooks.js @@ -111,11 +111,6 @@ const emitDestroyNative = emitHookFactory(destroy_symbol, 'emitDestroyNative'); const emitPromiseResolveNative = emitHookFactory(promise_resolve_symbol, 'emitPromiseResolveNative'); -let domain_cb; -function useDomainTrampoline(fn) { - domain_cb = fn; -} - function callbackTrampoline(asyncId, resource, cb, ...args) { const index = async_hook_fields[kStackLength] - 1; execution_async_resources[index] = resource; @@ -123,13 +118,7 @@ function callbackTrampoline(asyncId, resource, cb, ...args) { if (asyncId !== 0 && hasHooks(kBefore)) emitBeforeNative(asyncId); - let result; - if (asyncId === 0 && typeof domain_cb === 'function') { - args.unshift(cb); - result = ReflectApply(domain_cb, this, args); - } else { - result = ReflectApply(cb, this, args); - } + const result = ReflectApply(cb, this, args); if (asyncId !== 0 && hasHooks(kAfter)) emitAfterNative(asyncId); @@ -623,7 +612,6 @@ module.exports = { pushAsyncContext, popAsyncContext, registerDestroyHook, - useDomainTrampoline, nativeHooks: { init: emitInitNative, before: emitBeforeNative, From d07a270391ade64e6f978f6685e63aed4680e103 Mon Sep 17 00:00:00 2001 From: Matteo Collina Date: Sat, 20 Dec 2025 23:45:39 +0100 Subject: [PATCH 3/6] test: rename test-domain-async-id-map-leak The test no longer tests async-id map leaks. Rename to reflect its actual purpose: verifying that accessing domain property on AsyncResource throws ERR_ASYNC_RESOURCE_DOMAIN_REMOVED. --- ...d-map-leak.js => test-domain-async-resource-domain-removed.js} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename test/parallel/{test-domain-async-id-map-leak.js => test-domain-async-resource-domain-removed.js} (100%) diff --git a/test/parallel/test-domain-async-id-map-leak.js b/test/parallel/test-domain-async-resource-domain-removed.js similarity index 100% rename from test/parallel/test-domain-async-id-map-leak.js rename to test/parallel/test-domain-async-resource-domain-removed.js From bd87fa750813075242b4b7699c8117f1ec338a77 Mon Sep 17 00:00:00 2001 From: Matteo Collina Date: Sun, 21 Dec 2025 09:36:21 +0100 Subject: [PATCH 4/6] domain: use public AsyncLocalStorage API Use require('async_hooks').AsyncLocalStorage instead of directly importing internal modules. This allows proper auto-selection of the ALS implementation based on --async-context-frame flag. --- lib/domain.js | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/lib/domain.js b/lib/domain.js index 394abbefb21aee..2a0edd6e0c0784 100644 --- a/lib/domain.js +++ b/lib/domain.js @@ -53,13 +53,7 @@ let domainStorage; function initDomainStorage() { if (domainStorage === undefined) { - // Import AsyncContextFrame and trigger enabled check to ensure prototype swap - const AsyncContextFrame = require('internal/async_context_frame'); - // eslint-disable-next-line no-unused-expressions - AsyncContextFrame.enabled; // Triggers prototype swap from inactive to active - - // Import the async_context_frame-based ALS directly to avoid circular deps - const AsyncLocalStorage = require('internal/async_local_storage/async_context_frame'); + const { AsyncLocalStorage } = require('async_hooks'); // Single AsyncLocalStorage instance for all domain context // Store structure: { domain: Domain | null, stack: Domain[] } From 24483f3ea9debc492f0877f5c7aadf9b72666c16 Mon Sep 17 00:00:00 2001 From: Matteo Collina Date: Sun, 21 Dec 2025 11:41:03 +0100 Subject: [PATCH 5/6] domain: consolidate run/bound/intercepted into runInDomain Extract common domain context setup/teardown logic into a single runInDomain helper function, reducing code duplication. --- lib/domain.js | 76 +++++++++++---------------------------------------- 1 file changed, 16 insertions(+), 60 deletions(-) diff --git a/lib/domain.js b/lib/domain.js index 2a0edd6e0c0784..a2be856c6fe9cf 100644 --- a/lib/domain.js +++ b/lib/domain.js @@ -475,26 +475,25 @@ Domain.prototype.remove = function(ee) { }; -Domain.prototype.run = function(fn) { +// Helper to run a function within a domain context. +// Uses enterWith instead of ALS.run() so the context is NOT automatically +// restored on exception - this matches original domain behavior where +// exit() only runs on success. +function runInDomain(domain, thisArg, fn, args) { const currentStore = getDomainStore() || { domain: null, stack: [] }; const newStack = ArrayPrototypeSlice(currentStore.stack); - ArrayPrototypePush(newStack, this); - - const args = ArrayPrototypeSlice(arguments, 1); + ArrayPrototypePush(newStack, domain); const previousDomain = currentDomain; const previousStack = currentStack; - // Set currentDomain and currentStack before running so exception handler can see them - currentDomain = this; + currentDomain = domain; currentStack = newStack; updateExceptionCapture(); - // Use enterWith instead of run() so the context is NOT automatically restored on exception. - // This matches the original domain.run() behavior where exit() only runs on success. - setDomainStore({ domain: this, stack: newStack }); - exports.active = this; + setDomainStore({ domain, stack: newStack }); + exports.active = domain; - const result = ReflectApply(fn, this, args); + const result = ReflectApply(fn, thisArg, args); // On success, restore context (if exception thrown, context stays for catch blocks) setDomainStore(currentStore); @@ -504,6 +503,10 @@ Domain.prototype.run = function(fn) { updateExceptionCapture(); return result; +} + +Domain.prototype.run = function(fn) { + return runInDomain(this, this, fn, ArrayPrototypeSlice(arguments, 1)); }; @@ -523,31 +526,7 @@ function intercepted(_this, self, cb, fnargs) { return; } - const currentStore = getDomainStore() || { domain: null, stack: [] }; - const newStack = ArrayPrototypeSlice(currentStore.stack); - ArrayPrototypePush(newStack, self); - const args = ArrayPrototypeSlice(fnargs, 1); - const previousDomain = currentDomain; - const previousStack = currentStack; - - currentDomain = self; - currentStack = newStack; - updateExceptionCapture(); - - // Use enterWith instead of run() so the context is NOT automatically restored on exception. - setDomainStore({ domain: self, stack: newStack }); - exports.active = self; - - const result = ReflectApply(cb, _this, args); - - // On success, restore context - setDomainStore(currentStore); - exports.active = currentStore.domain; - currentDomain = previousDomain; - currentStack = previousStack; - updateExceptionCapture(); - - return result; + return runInDomain(self, _this, cb, ArrayPrototypeSlice(fnargs, 1)); } @@ -563,30 +542,7 @@ Domain.prototype.intercept = function(cb) { function bound(_this, self, cb, fnargs) { - const currentStore = getDomainStore() || { domain: null, stack: [] }; - const newStack = ArrayPrototypeSlice(currentStore.stack); - ArrayPrototypePush(newStack, self); - const previousDomain = currentDomain; - const previousStack = currentStack; - - currentDomain = self; - currentStack = newStack; - updateExceptionCapture(); - - // Use enterWith instead of run() so the context is NOT automatically restored on exception. - setDomainStore({ domain: self, stack: newStack }); - exports.active = self; - - const result = ReflectApply(cb, _this, fnargs); - - // On success, restore context - setDomainStore(currentStore); - exports.active = currentStore.domain; - currentDomain = previousDomain; - currentStack = previousStack; - updateExceptionCapture(); - - return result; + return runInDomain(self, _this, cb, fnargs); } From 1eda843c620f2e51da9ecf51be0f4eddb325491c Mon Sep 17 00:00:00 2001 From: Matteo Collina Date: Sun, 21 Dec 2025 15:23:42 +0100 Subject: [PATCH 6/6] domain: fix recursive error handler call in emit When an EventEmitter emits 'error' and the domain's error handler throws, the exception should propagate to the parent domain, not recursively call the same domain's error handler. The bug was that currentDomain/currentStack were not updated when temporarily switching to the parent domain context during error emission. Since domainExceptionHandler checks currentDomain first, exceptions would incorrectly route back to the same domain. Fixes by updating currentDomain/currentStack alongside the ALS store. --- lib/domain.js | 7 +++ ...domain-error-handler-throw-no-recursion.js | 45 +++++++++++++++++++ 2 files changed, 52 insertions(+) create mode 100644 test/parallel/test-domain-error-handler-throw-no-recursion.js diff --git a/lib/domain.js b/lib/domain.js index a2be856c6fe9cf..8aafd03504cb3e 100644 --- a/lib/domain.js +++ b/lib/domain.js @@ -642,10 +642,15 @@ EventEmitter.prototype.emit = function emit(...args) { // Change the current active domain const newActiveDomain = newStack.length > 0 ? newStack[newStack.length - 1] : null; + const prevCurrentDomain = currentDomain; + const prevCurrentStack = currentStack; + if (store) { setDomainStore({ domain: newActiveDomain, stack: newStack }); } exports.active = newActiveDomain; + currentDomain = newActiveDomain; + currentStack = newStack; updateExceptionCapture(); @@ -657,6 +662,8 @@ EventEmitter.prototype.emit = function emit(...args) { setDomainStore({ domain: origActiveDomain, stack: origDomainsStack }); } exports.active = origActiveDomain; + currentDomain = prevCurrentDomain; + currentStack = prevCurrentStack; updateExceptionCapture(); return false; diff --git a/test/parallel/test-domain-error-handler-throw-no-recursion.js b/test/parallel/test-domain-error-handler-throw-no-recursion.js new file mode 100644 index 00000000000000..d4301f10964b03 --- /dev/null +++ b/test/parallel/test-domain-error-handler-throw-no-recursion.js @@ -0,0 +1,45 @@ +'use strict'; +const common = require('../common'); +const assert = require('assert'); +const domain = require('domain'); + +// Test that when a domain's error handler throws, the exception does NOT +// recursively call the same domain's error handler. Instead, it should +// propagate to the parent domain or crash the process. + +let d1ErrorCount = 0; +let d2ErrorCount = 0; + +const d1 = domain.create(); +const d2 = domain.create(); + +d1.on('error', common.mustCall((err) => { + d1ErrorCount++; + assert.strictEqual(err.message, 'from d2 error handler'); + // d1 should only be called once - when d2's error handler throws + assert.strictEqual(d1ErrorCount, 1); +})); + +d2.on('error', common.mustCall((err) => { + d2ErrorCount++; + // d2's error handler should only be called once for the original error + assert.strictEqual(d2ErrorCount, 1); + assert.strictEqual(err.message, 'original error'); + // Throwing here should go to d1, NOT back to d2 + throw new Error('from d2 error handler'); +})); + +d1.run(() => { + d2.run(() => { + // Emit an error on an EventEmitter bound to d2 + const EventEmitter = require('events'); + const ee = new EventEmitter(); + d2.add(ee); + + // This should: + // 1. Call d2's error handler with 'original error' + // 2. d2's error handler throws 'from d2 error handler' + // 3. That exception should go to d1's error handler, NOT d2 again + ee.emit('error', new Error('original error')); + }); +});