repl: restore domain module compatibility

Add auxiliary callback mechanism to setUncaughtExceptionCaptureCallback
to allow REPL and domain module to coexist. The REPL uses the new
addUncaughtExceptionCaptureCallback API which doesn't conflict with
domain's use of the primary callback.

- Add addUncaughtExceptionCaptureCallback for non-exclusive callbacks
- Update REPL to check for active domain before handling errors
- Remove mutual exclusivity checks from domain module
- Restore test-repl-domain.js test
- Update domain coexistence tests

Author: mcollina

lib/domain.js

@@ -40,14 +40,11 @@ const {
   ReflectApply,
   SafeMap,
   SafeWeakMap,
-  StringPrototypeRepeat,
   Symbol,
 } = 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 { createHook } = require('async_hooks');
@@ -119,22 +116,9 @@ 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;
-
+// Domain uses the stacking capability of setUncaughtExceptionCaptureCallback
+// to coexist with other callbacks (e.g., REPL).
 const { setUncaughtExceptionCaptureCallback } = process;
-process.setUncaughtExceptionCaptureCallback = function(fn) {
-  const err = new ERR_DOMAIN_CANNOT_SET_UNCAUGHT_EXCEPTION_CAPTURE();
-  err.stack += `\n${StringPrototypeRepeat('-', 40)}\n${domainRequireStack}`;
-  throw err;
-};
 
 
 let sendMakeCallbackDeprecation = false;

lib/internal/bootstrap/node.js

@@ -307,6 +307,7 @@ ObjectDefineProperty(process, 'features', {
   const {
     onGlobalUncaughtException,
     setUncaughtExceptionCaptureCallback,
+    addUncaughtExceptionCaptureCallback,
     hasUncaughtExceptionCaptureCallback,
   } = require('internal/process/execution');
 
@@ -319,6 +320,8 @@ ObjectDefineProperty(process, 'features', {
   process._fatalException = onGlobalUncaughtException;
   process.setUncaughtExceptionCaptureCallback =
     setUncaughtExceptionCaptureCallback;
+  process.addUncaughtExceptionCaptureCallback =
+    addUncaughtExceptionCaptureCallback;
   process.hasUncaughtExceptionCaptureCallback =
     hasUncaughtExceptionCaptureCallback;
 }

lib/internal/process/execution.js

@@ -1,6 +1,7 @@
 'use strict';
 
 const {
+  ArrayPrototypePush,
   RegExpPrototypeExec,
   StringPrototypeIndexOf,
   StringPrototypeSlice,
@@ -105,15 +106,18 @@ function evalScript(name, body, breakFirstLine, print, shouldLoadESM = false) {
 }
 
 const exceptionHandlerState = {
-  captureFn: null,
+  captureFn: null,           // Primary callback (for domain's exclusive use)
+  auxiliaryCallbacks: [],    // Auxiliary callbacks (for REPL, etc.) - always called
   reportFlag: false,
 };
 
 function setUncaughtExceptionCaptureCallback(fn) {
   if (fn === null) {
     exceptionHandlerState.captureFn = fn;
-    shouldAbortOnUncaughtToggle[0] = 1;
-    process.report.reportOnUncaughtException = exceptionHandlerState.reportFlag;
+    if (exceptionHandlerState.auxiliaryCallbacks.length === 0) {
+      shouldAbortOnUncaughtToggle[0] = 1;
+      process.report.reportOnUncaughtException = exceptionHandlerState.reportFlag;
+    }
     return;
   }
   if (typeof fn !== 'function') {
@@ -129,6 +133,23 @@ function setUncaughtExceptionCaptureCallback(fn) {
   process.report.reportOnUncaughtException = false;
 }
 
+// Add an auxiliary callback that coexists with the primary callback.
+// Auxiliary callbacks are called first; if any returns true, the error is handled.
+// Otherwise, the primary callback (if set) is called.
+function addUncaughtExceptionCaptureCallback(fn) {
+  if (typeof fn !== 'function') {
+    throw new ERR_INVALID_ARG_TYPE('fn', 'Function', fn);
+  }
+  if (exceptionHandlerState.auxiliaryCallbacks.length === 0 &&
+      exceptionHandlerState.captureFn === null) {
+    exceptionHandlerState.reportFlag =
+      process.report.reportOnUncaughtException === true;
+    process.report.reportOnUncaughtException = false;
+    shouldAbortOnUncaughtToggle[0] = 0;
+  }
+  ArrayPrototypePush(exceptionHandlerState.auxiliaryCallbacks, fn);
+}
+
 function hasUncaughtExceptionCaptureCallback() {
   return exceptionHandlerState.captureFn !== null;
 }
@@ -154,21 +175,33 @@ function createOnGlobalUncaughtException() {
 
     const type = fromPromise ? 'unhandledRejection' : 'uncaughtException';
     process.emit('uncaughtExceptionMonitor', er, type);
+    // Primary callback (e.g., domain) has priority and always handles the exception
     if (exceptionHandlerState.captureFn !== null) {
       exceptionHandlerState.captureFn(er);
-    } else if (!process.emit('uncaughtException', er, type)) {
-      // If someone handled it, then great. Otherwise, die in C++ land
-      // since that means that we'll exit the process, emit the 'exit' event.
-      try {
-        if (!process._exiting) {
-          process._exiting = true;
-          process.exitCode = kGenericUserError;
-          process.emit('exit', kGenericUserError);
+    } else {
+      // If no primary callback, try auxiliary callbacks (e.g., REPL)
+      // They must return true to indicate handling
+      let handled = false;
+      for (let i = exceptionHandlerState.auxiliaryCallbacks.length - 1; i >= 0; i--) {
+        if (exceptionHandlerState.auxiliaryCallbacks[i](er) === true) {
+          handled = true;
+          break;
+        }
+      }
+      if (!handled && !process.emit('uncaughtException', er, type)) {
+        // If someone handled it, then great. Otherwise, die in C++ land
+        // since that means that we'll exit the process, emit the 'exit' event.
+        try {
+          if (!process._exiting) {
+            process._exiting = true;
+            process.exitCode = kGenericUserError;
+            process.emit('exit', kGenericUserError);
+          }
+        } catch {
+          // Nothing to be done about it at this point.
         }
-      } catch {
-        // Nothing to be done about it at this point.
+        return false;
       }
-      return false;
     }
 
     // If we handled an error, then make sure any ticks get processed
@@ -477,5 +510,6 @@ module.exports = {
   evalScript,
   onGlobalUncaughtException: createOnGlobalUncaughtException(),
   setUncaughtExceptionCaptureCallback,
+  addUncaughtExceptionCaptureCallback,
   hasUncaughtExceptionCaptureCallback,
 };

lib/repl.js

@@ -187,36 +187,22 @@ let exceptionCaptureSetup = false;
 /**
  * Sets up the uncaught exception capture callback to route errors
  * to the appropriate REPL instance. This replaces domain-based error handling.
- * @returns {boolean} true if setup succeeded, false if capture callback already in use
+ * Uses addUncaughtExceptionCaptureCallback to coexist with the primary
+ * callback (e.g., domain module).
  */
 function setupExceptionCapture() {
-  if (exceptionCaptureSetup) return true;
+  if (exceptionCaptureSetup) return;
 
-  if (process.hasUncaughtExceptionCaptureCallback()) {
-    return false;
-  }
-
-  process.setUncaughtExceptionCaptureCallback((err) => {
+  process.addUncaughtExceptionCaptureCallback((err) => {
     const store = replContext.getStore();
     if (store?.replServer && !store.replServer.closed) {
       store.replServer._handleError(err);
-      return;
-    }
-    // No active REPL context - re-emit for normal process handling
-    process.setUncaughtExceptionCaptureCallback(null);
-    exceptionCaptureSetup = false;
-    try {
-      if (!process.emit('uncaughtException', err, 'uncaughtException')) {
-        // No handler, rethrow to exit process
-        throw err;
-      }
-    } finally {
-      setupExceptionCapture();
+      return true; // We handled it
     }
+    // No active REPL context - let other handlers try
   });
 
   exceptionCaptureSetup = true;
-  return true;
 }
 
 const kBufferedCommandSymbol = Symbol('bufferedCommand');
@@ -652,6 +638,12 @@ class REPLServer extends Interface {
           }
         } catch (e) {
           err = e;
+          // If there's an active domain with error listeners, let it handle the error
+          if (process.domain?.listenerCount('error') > 0) {
+            debug('domain handling error');
+            process.domain.emit('error', err);
+            return;
+          }
           // Handle non-recoverable errors directly
           debug('not recoverable, handle error');
           self._handleError(err);
@@ -681,9 +673,15 @@ class REPLServer extends Interface {
               const result = (await promise)?.value;
               finishExecution(null, result);
             } catch (err) {
-              // Handle non-recoverable async errors directly
-              debug('not recoverable, handle error');
-              self._handleError(err);
+              // If there's an active domain with error listeners, let it handle the error
+              if (process.domain?.listenerCount('error') > 0) {
+                debug('domain handling async error');
+                process.domain.emit('error', err);
+              } else {
+                // Handle non-recoverable async errors directly
+                debug('not recoverable, handle error');
+                self._handleError(err);
+              }
             } finally {
               // Remove prioritized SIGINT listener if it was not called.
               prioritizedSigintQueue.delete(sigintListener);

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

@@ -1,17 +1,22 @@
 'use strict';
+// Tests that domain can be loaded after setUncaughtExceptionCaptureCallback
+// has been called. This verifies that the mutual exclusivity has been removed.
 const common = require('../common');
-const assert = require('assert');
 
+// Set up a capture callback first
 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/
-  }
-);
+// Loading domain should not throw (coexistence is now supported)
+const domain = require('domain');
+
+// Verify domain module loaded successfully
+const assert = require('assert');
+assert.ok(domain);
+assert.ok(domain.create);
 
+// Clean up
 process.setUncaughtExceptionCaptureCallback(null);
-require('domain'); // Should not throw.
+
+// Domain should still be usable
+const d = domain.create();
+assert.ok(d);

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

@@ -1,30 +1,23 @@
 'use strict';
+// Tests that setUncaughtExceptionCaptureCallback can be called after domain
+// is loaded. This verifies that the mutual exclusivity has been removed.
 const common = require('../common');
 const assert = require('assert');
 
-Error.stackTraceLimit = Infinity;
+// Load domain first
+const domain = require('domain');
+assert.ok(domain);
 
-(function foobar() {
-  require('domain');
-})();
+// Setting callback should not throw (coexistence is now supported)
+process.setUncaughtExceptionCaptureCallback(common.mustNotCall());
 
-assert.throws(
-  () => process.setUncaughtExceptionCaptureCallback(common.mustNotCall()),
-  (err) => {
-    common.expectsError(
-      {
-        code: 'ERR_DOMAIN_CANNOT_SET_UNCAUGHT_EXCEPTION_CAPTURE',
-        name: 'Error',
-        message: /^The `domain` module is in use, which is mutually/
-      }
-    )(err);
+// Verify callback is registered
+assert.ok(process.hasUncaughtExceptionCaptureCallback());
 
-    assert(err.stack.includes('-'.repeat(40)),
-           `expected ${err.stack} to contain dashes`);
+// Clean up
+process.setUncaughtExceptionCaptureCallback(null);
+assert.ok(!process.hasUncaughtExceptionCaptureCallback());
 
-    const location = `at foobar (${__filename}:`;
-    assert(err.stack.includes(location),
-           `expected ${err.stack} to contain ${location}`);
-    return true;
-  }
-);
+// Domain should still be usable after callback operations
+const d = domain.create();
+assert.ok(d);

test/parallel/test-repl-domain.js

@@ -0,0 +1,49 @@
+// Copyright Joyent, Inc. and other Node contributors.
+//
+// Permission is hereby granted, free of charge, to any person obtaining a
+// copy of this software and associated documentation files (the
+// "Software"), to deal in the Software without restriction, including
+// without limitation the rights to use, copy, modify, merge, publish,
+// distribute, sublicense, and/or sell copies of the Software, and to permit
+// persons to whom the Software is furnished to do so, subject to the
+// following conditions:
+//
+// The above copyright notice and this permission notice shall be included
+// in all copies or substantial portions of the Software.
+//
+// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS
+// OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
+// MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN
+// NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM,
+// DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR
+// OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE
+// USE OR OTHER DEALINGS IN THE SOFTWARE.
+
+'use strict';
+require('../common');
+const { startNewREPLServer } = require('../common/repl');
+const ArrayStream = require('../common/arraystream');
+
+const stream = new ArrayStream();
+
+startNewREPLServer({
+  input: stream,
+  output: stream,
+  terminal: false,
+});
+
+stream.write = function(data) {
+  // Don't use assert for this because the domain might catch it, and
+  // give a false negative.  Don't throw, just print and exit.
+  if (data === 'OK\n') {
+    console.log('ok');
+  } else {
+    console.error(data);
+    process.exit(1);
+  }
+};
+
+stream.run([
+  'require("domain").create().on("error", function() { console.log("OK") })' +
+  '.run(function() { throw new Error("threw") })',
+]);