domain: allow concurrent user-land impl

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
@@ -748,11 +742,8 @@ For an example of running a REPL instance over [`curl(1)`][], see:
 [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

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) {
@@ -331,40 +340,60 @@ EventEmitter.prototype.emit = function emit(type, ...args) {
     let er;
     if (args.length > 0)
       er = args[0];
-    if (er instanceof Error) {
-      try {
-        const capture = {};
-        ErrorCaptureStackTrace(capture, EventEmitter.prototype.emit);
-        ObjectDefineProperty(er, kEnhanceStackBeforeInspector, {
-          value: enhanceStackTrace.bind(this, er, capture),
-          configurable: true
-        });
-      } catch {}
-
-      // 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
-    }
+    if (!this.domain) {
+      if (er instanceof Error) {
+        try {
+          const capture = {};
+          ErrorCaptureStackTrace(capture, EventEmitter.prototype.emit);
+          ObjectDefineProperty(er, kEnhanceStackBeforeInspector, {
+            value: enhanceStackTrace.bind(this, er, capture),
+            configurable: true
+          });
+        } catch {}
+
+        // 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
+      }
+    } else {
+      if (!er) {
+        er = new ERR_UNHANDLED_ERROR();
+      }
+      if (typeof er === 'object') {
+        er.domainEmitter = this;
+        er.domain = this.domain;
+        er.domainThrown = false;
+      }
+      this.domain.emit('error', 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
+    return false;
   }
 
   const handler = events[type];
 
   if (handler === undefined)
     return false;
 
+  let needDomainExit = false;
+  if (this.domain && this !== process) {
+    this.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)
+    this.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/message/events_unhandled_error_common_trace.out

@@ -1,6 +1,6 @@
 node:events:*
-      throw er; // Unhandled 'error' event
-      ^
+        throw er; // Unhandled 'error' event
+        ^
 
 Error: foo:bar
     at bar (*events_unhandled_error_common_trace.js:*:*)

test/message/events_unhandled_error_nexttick.out

@@ -1,6 +1,6 @@
 node:events:*
-      throw er; // Unhandled 'error' event
-      ^
+        throw er; // Unhandled 'error' event
+        ^
 
 Error
     at Object.<anonymous> (*events_unhandled_error_nexttick.js:*:*)

test/message/events_unhandled_error_sameline.out

@@ -1,6 +1,6 @@
 node:events:*
-      throw er; // Unhandled 'error' event
-      ^
+        throw er; // Unhandled 'error' event
+        ^
 
 Error
     at Object.<anonymous> (*events_unhandled_error_sameline.js:*:*)

test/message/events_unhandled_error_subclass.out

@@ -1,6 +1,6 @@
 node:events:*
-      throw er; // Unhandled 'error' event
-      ^
+        throw er; // Unhandled 'error' event
+        ^
 
 Error
     at Object.<anonymous> (*events_unhandled_error_subclass.js:*:*)

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.