domain: allow concurrent user-land impl

Currently, only one domain-lke implementation (the core domain one) can
be used to handle uncaught exceptions or unhandled error events.

This PR aims at making it possible for different domain-like
user-land implementations to be used concurrently (including with the
core domain impl) so that the state of the core domain module (doc
deprecated) does not prevent users of domains from having a
well-maintained domain-like facility.

Ref: #23348
Co-authored-by: Rich Trott <[email protected]>

Author: Julien Gilli

doc/api/errors.md

@@ -1014,23 +1014,6 @@ ongoing asynchronous operations.
 
 `c-ares` failed to set the DNS server.
 
-<a id="ERR_DOMAIN_CALLBACK_NOT_AVAILABLE"></a>
-### `ERR_DOMAIN_CALLBACK_NOT_AVAILABLE`
-
-The `domain` module was not usable since it could not establish the required
-error handling hooks, because
-[`process.setUncaughtExceptionCaptureCallback()`][] had been called at an
-earlier point in time.
-
-<a id="ERR_DOMAIN_CANNOT_SET_UNCAUGHT_EXCEPTION_CAPTURE"></a>
-### `ERR_DOMAIN_CANNOT_SET_UNCAUGHT_EXCEPTION_CAPTURE`
-
-[`process.setUncaughtExceptionCaptureCallback()`][] could not be called
-because the `domain` module has been loaded at an earlier point in time.
-
-The stack trace is extended to include the point in time at which the
-`domain` module had been loaded.
-
 <a id="ERR_ENCODING_INVALID_ENCODED_DATA"></a>
 ### `ERR_ENCODING_INVALID_ENCODED_DATA`
 
@@ -2473,6 +2456,31 @@ The UTF-16 encoding was used with [`hash.digest()`][]. While the
 causing the method to return a string rather than a `Buffer`, the UTF-16
 encoding (e.g. `ucs` or `utf16le`) is not supported.
 
+<a id="ERR_DOMAIN_CALLBACK_NOT_AVAILABLE"></a>
+### `ERR_DOMAIN_CALLBACK_NOT_AVAILABLE`
+<!-- YAML
+added: v10.0.0
+removed: REPLACEME
+-->
+
+The `domain` module was not usable since it could not establish the required
+error handling hooks, because
+[`process.setUncaughtExceptionCaptureCallback()`][] had been called at an
+earlier point in time.
+
+<a id="ERR_DOMAIN_CANNOT_SET_UNCAUGHT_EXCEPTION_CAPTURE"></a>
+### `ERR_DOMAIN_CANNOT_SET_UNCAUGHT_EXCEPTION_CAPTURE`
+<!-- YAML
+added: v10.0.0
+removed: REPLACEME
+-->
+
+[`process.setUncaughtExceptionCaptureCallback()`][] could not be called
+because the `domain` module has been loaded at an earlier point in time.
+
+The stack trace is extended to include the point in time at which the
+`domain` module had been loaded.
+
 <a id="ERR_HTTP2_FRAME_ERROR"></a>
 ### `ERR_HTTP2_FRAME_ERROR`
 <!-- YAML

doc/api/repl.md

@@ -159,25 +159,19 @@ changes:
 The REPL uses the [`domain`][] module to catch all uncaught exceptions for that
 REPL session.
 
-This use of the [`domain`][] module in the REPL has these side effects:
+This use of the [`domain`][] module in the REPL has the side effect of making
+uncaught exceptions not emit the [`'uncaughtException'`][] event.
 
-* Uncaught exceptions only emit the [`'uncaughtException'`][] event in the
-  standalone REPL. Adding a listener for this event in a REPL within
-  another Node.js program results in [`ERR_INVALID_REPL_INPUT`][].
-
-  ```js
-  const r = repl.start();
-
-  r.write('process.on("uncaughtException", () => console.log("Foobar"));\n');
-  // Output stream includes:
-  //   TypeError [ERR_INVALID_REPL_INPUT]: Listeners for `uncaughtException`
-  //   cannot be used in the REPL
+```js
+const r = repl.start();
 
-  r.close();
-  ```
+r.write('process.on("uncaughtException", () => console.log("Foobar"));\n');
+// Output stream includes:
+//   TypeError [ERR_INVALID_REPL_INPUT]: Listeners for `uncaughtException`
+//   cannot be used in the REPL
 
-* Trying to use [`process.setUncaughtExceptionCaptureCallback()`][] throws
-  an [`ERR_DOMAIN_CANNOT_SET_UNCAUGHT_EXCEPTION_CAPTURE`][] error.
+r.close();
+```
 
 #### Assignment of the `_` (underscore) variable
 <!-- YAML
@@ -746,16 +740,13 @@ For an example of running a REPL instance over [`curl(1)`][], see:
 
 [TTY keybindings]: readline.md#readline_tty_keybindings
 [ZSH]: https://en.wikipedia.org/wiki/Z_shell
-[`'uncaughtException'`]: process.md#process_event_uncaughtexception
 [`--experimental-repl-await`]: cli.md#cli_experimental_repl_await
-[`ERR_DOMAIN_CANNOT_SET_UNCAUGHT_EXCEPTION_CAPTURE`]: errors.md#errors_err_domain_cannot_set_uncaught_exception_capture
-[`ERR_INVALID_REPL_INPUT`]: errors.md#errors_err_invalid_repl_input
 [`curl(1)`]: https://curl.haxx.se/docs/manpage.html
 [`domain`]: domain.md
-[`process.setUncaughtExceptionCaptureCallback()`]: process.md#process_process_setuncaughtexceptioncapturecallback_fn
 [`readline.InterfaceCompleter`]: readline.md#readline_use_of_the_completer_function
 [`repl.ReplServer`]: #repl_class_replserver
 [`repl.start()`]: #repl_repl_start_options
 [`reverse-i-search`]: #repl_reverse_i_search
+[`'uncaughtException'`]: process.md#process_event_uncaughtexception
 [`util.inspect()`]: util.md#util_util_inspect_object_options
 [stream]: stream.md

lib/domain.js

@@ -42,18 +42,19 @@ const {
 } = primordials;
 
 const EventEmitter = require('events');
-const {
-  ERR_DOMAIN_CALLBACK_NOT_AVAILABLE,
-  ERR_DOMAIN_CANNOT_SET_UNCAUGHT_EXCEPTION_CAPTURE,
-  ERR_UNHANDLED_ERROR
-} = require('internal/errors').codes;
+const { ERR_UNHANDLED_ERROR } = require('internal/errors').codes;
 const { createHook } = require('async_hooks');
 const { useDomainTrampoline } = require('internal/async_hooks');
 
 // TODO(addaleax): Use a non-internal solution for this.
 const kWeak = Symbol('kWeak');
 const { WeakReference } = internalBinding('util');
 
+// Communicate with events module, but don't require that
+// module to have to load this one, since this module has
+// a few side effects.
+EventEmitter.usingDomains = true;
+
 // Overwrite process.domain with a getter/setter that will allow for more
 // effective optimizations
 const _domain = [null];
@@ -105,23 +106,7 @@ const asyncHook = createHook({
   }
 });
 
-// When domains are in use, they claim full ownership of the
-// uncaught exception capture callback.
-if (process.hasUncaughtExceptionCaptureCallback()) {
-  throw new ERR_DOMAIN_CALLBACK_NOT_AVAILABLE();
-}
-
-// Get the stack trace at the point where `domain` was required.
-// eslint-disable-next-line no-restricted-syntax
-const domainRequireStack = new Error('require(`domain`) at this point').stack;
-
 const { setUncaughtExceptionCaptureCallback } = process;
-process.setUncaughtExceptionCaptureCallback = function(fn) {
-  const err = new ERR_DOMAIN_CANNOT_SET_UNCAUGHT_EXCEPTION_CAPTURE();
-  err.stack = err.stack + '\n' + '-'.repeat(40) + '\n' + domainRequireStack;
-  throw err;
-};
-
 
 let sendMakeCallbackDeprecation = false;
 function emitMakeCallbackDeprecation({ target, method }) {
@@ -151,8 +136,8 @@ function topLevelDomainCallback(cb, ...args) {
   return ret;
 }
 
-// It's possible to enter one domain while already inside
-// another one. The stack is each entered domain.
+// 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);
@@ -210,6 +195,8 @@ class Domain extends EventEmitter {
   }
 }
 
+Domain[EventEmitter.isDomainLike] = true;
+
 exports.Domain = Domain;
 
 exports.create = exports.createDomain = function createDomain() {
@@ -344,7 +331,7 @@ Domain.prototype.add = function(ee) {
   // d.add(e);
   // e.add(d);
   // e.emit('error', er); // RangeError, stack overflow!
-  if (this.domain && (ee instanceof Domain)) {
+  if (this.domain && ee[EventEmitter.isDomainLike]) {
     for (let d = this.domain; d; d = d.domain) {
       if (ee === d) return;
     }

lib/events.js

@@ -86,6 +86,7 @@ module.exports.getEventListeners = getEventListeners;
 EventEmitter.EventEmitter = EventEmitter;
 
 EventEmitter.usingDomains = false;
+EventEmitter.isDomainLike = Symbol('isDomainLike');
 
 EventEmitter.captureRejectionSymbol = kRejection;
 ObjectDefineProperty(EventEmitter, 'captureRejections', {
@@ -112,6 +113,7 @@ ObjectDefineProperty(EventEmitter.prototype, kCapture, {
   enumerable: false
 });
 
+EventEmitter.prototype.domain = undefined;
 EventEmitter.prototype._events = undefined;
 EventEmitter.prototype._eventsCount = 0;
 EventEmitter.prototype._maxListeners = undefined;
@@ -183,6 +185,13 @@ EventEmitter.setMaxListeners =
   };
 
 EventEmitter.init = function(opts) {
+  this.domain = null;
+  // If there is an active domain, then attach to it, except if the event
+  // emitter being constructed is itself an instance of a domain-like class.
+  // eslint-disable-next-line max-len
+  if (EventEmitter.usingDomains && !this.constructor[EventEmitter.isDomainLike]) {
+    this.domain = process.domain;
+  }
 
   if (this._events === undefined ||
       this._events === ObjectGetPrototypeOf(this)._events) {
@@ -326,12 +335,24 @@ EventEmitter.prototype.emit = function emit(type, ...args) {
   } else if (!doError)
     return false;
 
+  const domain = this.domain;
+
   // If there is no 'error' event listener then throw.
   if (doError) {
     let er;
     if (args.length > 0)
       er = args[0];
-    if (er instanceof Error) {
+    if (domain !== null && domain !== undefined) {
+      if (!er) {
+        er = new ERR_UNHANDLED_ERROR();
+      }
+      if (typeof er === 'object' && er !== null) {
+        er.domainEmitter = this;
+        er.domain = domain;
+        er.domainThrown = false;
+      }
+      domain.emit('error', er);
+    } else if (er instanceof Error) {
       try {
         const capture = {};
         ErrorCaptureStackTrace(capture, EventEmitter.prototype.emit);
@@ -344,27 +365,35 @@ EventEmitter.prototype.emit = function emit(type, ...args) {
       // Note: The comments on the `throw` lines are intentional, they show
       // up in Node's output if this results in an unhandled exception.
       throw er; // Unhandled 'error' event
-    }
+    } else {
+      let stringifiedEr;
+      const { inspect } = require('internal/util/inspect');
+      try {
+        stringifiedEr = inspect(er);
+      } catch {
+        stringifiedEr = er;
+      }
 
-    let stringifiedEr;
-    const { inspect } = require('internal/util/inspect');
-    try {
-      stringifiedEr = inspect(er);
-    } catch {
-      stringifiedEr = er;
+      // At least give some kind of context to the user
+      const err = new ERR_UNHANDLED_ERROR(stringifiedEr);
+      err.context = er;
+      throw err; // Unhandled 'error' event
     }
 
-    // At least give some kind of context to the user
-    const err = new ERR_UNHANDLED_ERROR(stringifiedEr);
-    err.context = er;
-    throw err; // Unhandled 'error' event
+    return false;
   }
 
   const handler = events[type];
 
   if (handler === undefined)
     return false;
 
+  let needDomainExit = false;
+  if (domain !== null && domain !== undefined && this !== process) {
+    domain.enter();
+    needDomainExit = true;
+  }
+
   if (typeof handler === 'function') {
     const result = ReflectApply(handler, this, args);
 
@@ -391,6 +420,9 @@ EventEmitter.prototype.emit = function emit(type, ...args) {
     }
   }
 
+  if (needDomainExit)
+    domain.exit();
+
   return true;
 };
 

lib/internal/errors.js

@@ -874,15 +874,6 @@ E('ERR_DIR_CONCURRENT_OPERATION',
   'asynchronous operations', Error);
 E('ERR_DNS_SET_SERVERS_FAILED', 'c-ares failed to set servers: "%s" [%s]',
   Error);
-E('ERR_DOMAIN_CALLBACK_NOT_AVAILABLE',
-  'A callback was registered through ' +
-     'process.setUncaughtExceptionCaptureCallback(), which is mutually ' +
-     'exclusive with using the `domain` module',
-  Error);
-E('ERR_DOMAIN_CANNOT_SET_UNCAUGHT_EXCEPTION_CAPTURE',
-  'The `domain` module is in use, which is mutually exclusive with calling ' +
-     'process.setUncaughtExceptionCaptureCallback()',
-  Error);
 E('ERR_ENCODING_INVALID_ENCODED_DATA', function(encoding, ret) {
   this.errno = ret;
   return `The encoded data was not valid for encoding ${encoding}`;

test/parallel/test-domain-load-after-set-uncaught-exception-capture.js

@@ -1,17 +1,6 @@
 'use strict';
 const common = require('../common');
-const assert = require('assert');
 
 process.setUncaughtExceptionCaptureCallback(common.mustNotCall());
 
-assert.throws(
-  () => require('domain'),
-  {
-    code: 'ERR_DOMAIN_CALLBACK_NOT_AVAILABLE',
-    name: 'Error',
-    message: /^A callback was registered.*with using the `domain` module/
-  }
-);
-
-process.setUncaughtExceptionCaptureCallback(null);
 require('domain'); // Should not throw.