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..8aafd03504cb3e 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,50 @@ 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) { + const { AsyncLocalStorage } = require('async_hooks'); + + // 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); - } - } - }, - 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(); + const store = getDomainStore(); + if (store) { + // Create new store to avoid affecting other async contexts + setDomainStore({ domain: arg, stack: store.stack }); } - }, - 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 +111,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 +273,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 +289,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 +327,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 +347,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 +371,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 +400,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(); }; @@ -384,12 +475,38 @@ Domain.prototype.remove = function(ee) { }; -Domain.prototype.run = function(fn) { - this.enter(); - const ret = ReflectApply(fn, this, ArrayPrototypeSlice(arguments, 1)); - this.exit(); +// 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, domain); + const previousDomain = currentDomain; + const previousStack = currentStack; - return ret; + currentDomain = domain; + currentStack = newStack; + updateExceptionCapture(); + + setDomainStore({ domain, stack: newStack }); + exports.active = domain; + + const result = ReflectApply(fn, thisArg, 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; +} + +Domain.prototype.run = function(fn) { + return runInDomain(this, this, fn, ArrayPrototypeSlice(arguments, 1)); }; @@ -409,11 +526,7 @@ function intercepted(_this, self, cb, fnargs) { return; } - self.enter(); - const ret = ReflectApply(cb, _this, ArrayPrototypeSlice(fnargs, 1)); - self.exit(); - - return ret; + return runInDomain(self, _this, cb, ArrayPrototypeSlice(fnargs, 1)); } @@ -429,11 +542,7 @@ Domain.prototype.intercept = function(cb) { function bound(_this, self, cb, fnargs) { - self.enter(); - const ret = ReflectApply(cb, _this, fnargs); - self.exit(); - - return ret; + return runInDomain(self, _this, cb, fnargs); } @@ -510,30 +619,38 @@ 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; + const prevCurrentDomain = currentDomain; + const prevCurrentStack = currentStack; + + if (store) { + setDomainStore({ domain: newActiveDomain, stack: newStack }); } + exports.active = newActiveDomain; + currentDomain = newActiveDomain; + currentStack = newStack; updateExceptionCapture(); @@ -541,8 +658,12 @@ 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; + currentDomain = prevCurrentDomain; + currentStack = prevCurrentStack; updateExceptionCapture(); return false; 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, 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 deleted file mode 100644 index 40d051aa275179..00000000000000 --- a/test/parallel/test-domain-async-id-map-leak.js +++ /dev/null @@ -1,49 +0,0 @@ -// 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 - -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(); - - d.remove(emitter); - d.add(emitter); - - emitter.linkToResource = resource; - 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; }) }); -})); - -d = null; - -async function main() { - await gcUntil( - 'All objects garbage collected', - () => resourceGCed && domainGCed && emitterGCed); -} - -main(); diff --git a/test/parallel/test-domain-async-resource-domain-removed.js b/test/parallel/test-domain-async-resource-domain-removed.js new file mode 100644 index 00000000000000..96932901a167ab --- /dev/null +++ b/test/parallel/test-domain-async-resource-domain-removed.js @@ -0,0 +1,34 @@ +'use strict'; +const common = require('../common'); +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 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(); + +d.run(common.mustCall(() => { + const resource = new async_hooks.AsyncResource('TestResource'); + const emitter = new EventEmitter(); + + d.remove(emitter); + d.add(emitter); + + // emitter.domain should work + assert.strictEqual(emitter.domain, d); + assert.strictEqual(isEnumerable(emitter, 'domain'), false); + + // 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.', + }); +})); 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-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')); + }); +}); 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.