repl: properly handle uncaughtException

doc/api/errors.md

@@ -1310,8 +1310,14 @@ An invalid `options.protocol` was passed.
 <a id="ERR_INVALID_REPL_EVAL_CONFIG"></a>
 ### ERR_INVALID_REPL_EVAL_CONFIG
 
-Both `breakEvalOnSigint` and `eval` options were set in the REPL config, which
-is not supported.
+Both `breakEvalOnSigint` and `eval` options were set in the [`REPL`][] config,
+which is not supported.
+
+<a id="ERR_INVALID_REPL_INPUT"></a>
+### ERR_INVALID_REPL_INPUT
+
+The input may not be used in the [`REPL`][]. All prohibited inputs are
+documented in the [`REPL`][]'s documentation.
 
 <a id="ERR_INVALID_RETURN_PROPERTY"></a>
 ### ERR_INVALID_RETURN_PROPERTY
@@ -2307,6 +2313,7 @@ such as `process.stdout.on('data')`.
 [`Class: assert.AssertionError`]: assert.html#assert_class_assert_assertionerror
 [`ERR_INVALID_ARG_TYPE`]: #ERR_INVALID_ARG_TYPE
 [`EventEmitter`]: events.html#events_class_eventemitter
+[`REPL`]: repl.html
 [`Writable`]: stream.html#stream_class_stream_writable
 [`child_process`]: child_process.html
 [`cipher.getAuthTag()`]: crypto.html#crypto_cipher_getauthtag

doc/api/repl.md

@@ -138,16 +138,47 @@ global or scoped variable, the input `fs` will be evaluated on-demand as
 ```
 
 #### Global Uncaught Exceptions
+<!-- YAML
+changes:
+  - version: REPLACEME
+    pr-url: https://github.com/nodejs/node/pull/27151
+    description: The `'uncaughtException'` event is from now on triggered if the
+                 repl is used as standalone program.
+-->
 
 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:
 
-* Uncaught exceptions do not emit the [`'uncaughtException'`][] event.
+* Uncaught exceptions only emit the [`'uncaughtException'`][] event if the
+  `repl` is used as standalone program. If the `repl` is included anywhere in
+  another application, adding a listener for this event will throw an
+  [`ERR_INVALID_REPL_INPUT`][] exception.
 * Trying to use [`process.setUncaughtExceptionCaptureCallback()`][] throws
   an [`ERR_DOMAIN_CANNOT_SET_UNCAUGHT_EXCEPTION_CAPTURE`][] error.
 
+As standalone program:
+
+```js
+process.on('uncaughtException', () => console.log('Uncaught'));
+
+throw new Error('foobar');
+// Uncaught
+```
+
+When used in another application:
+
+```js
+process.on('uncaughtException', () => console.log('Uncaught'));
+// TypeError [ERR_INVALID_REPL_INPUT]: Listeners for `uncaughtException`
+// cannot be used in the REPL
+
+throw new Error('foobar');
+// Thrown:
+// Error: foobar
+```
+
 #### Assignment of the `_` (underscore) variable
 <!-- YAML
 changes:
@@ -661,6 +692,7 @@ For an example of running a REPL instance over [curl(1)][], see:
 [`'uncaughtException'`]: process.html#process_event_uncaughtexception
 [`--experimental-repl-await`]: cli.html#cli_experimental_repl_await
 [`ERR_DOMAIN_CANNOT_SET_UNCAUGHT_EXCEPTION_CAPTURE`]: errors.html#errors_err_domain_cannot_set_uncaught_exception_capture
+[`ERR_INVALID_REPL_INPUT`]: errors.html#errors_err_invalid_repl_input
 [`domain`]: domain.html
 [`process.setUncaughtExceptionCaptureCallback()`]: process.html#process_process_setuncaughtexceptioncapturecallback_fn
 [`readline.InterfaceCompleter`]: readline.html#readline_use_of_the_completer_function

lib/internal/errors.js

@@ -884,6 +884,7 @@ E('ERR_INVALID_PROTOCOL',
   TypeError);
 E('ERR_INVALID_REPL_EVAL_CONFIG',
   'Cannot specify both "breakEvalOnSigint" and "eval" for REPL', TypeError);
+E('ERR_INVALID_REPL_INPUT', '%s', TypeError);
 E('ERR_INVALID_RETURN_PROPERTY', (input, name, prop, value) => {
   return `Expected a valid ${input} to be returned for the "${prop}" from the` +
          ` "${name}" function but got ${value}.`;

lib/repl.js

@@ -72,6 +72,7 @@ const {
   ERR_CANNOT_WATCH_SIGINT,
   ERR_INVALID_ARG_TYPE,
   ERR_INVALID_REPL_EVAL_CONFIG,
+  ERR_INVALID_REPL_INPUT,
   ERR_SCRIPT_EXECUTION_INTERRUPTED
 } = require('internal/errors').codes;
 const { sendInspectorCommand } = require('internal/util/inspector');
@@ -101,10 +102,13 @@ let processTopLevelAwait;
 
 const parentModule = module;
 const replMap = new WeakMap();
+const domainSet = new WeakSet();
 
 const kBufferedCommandSymbol = Symbol('bufferedCommand');
 const kContextId = Symbol('contextId');
 
+let addedNewListener = false;
+
 try {
   // Hack for require.resolve("./relative") to work properly.
   module.filename = path.resolve('repl');
@@ -204,6 +208,28 @@ function REPLServer(prompt,
     throw new ERR_INVALID_REPL_EVAL_CONFIG();
   }
 
+  // Add this listener only once and use a WeakSet that contains the REPLs
+  // domains. Otherwise we'd have to add a single listener to each REPL instance
+  // and that could trigger the `MaxListenersExceededWarning`.
+  if (!options[kStandaloneREPL] && !addedNewListener) {
+    process.prependListener('newListener', (event, listener) => {
+      if (event === 'uncaughtException' &&
+          process.domain &&
+          listener.name !== 'domainUncaughtExceptionClear' &&
+          domainSet.has(process.domain)) {
+        // Throw an error so that the event will not be added and the current
+        // domain takes over. That way the user is notified about the error
+        // and the current code evaluation is stopped, just as any other code
+        // that contains an error.
+        throw new ERR_INVALID_REPL_INPUT(
+          'Listeners for `uncaughtException` cannot be used in the REPL');
+      }
+    });
+    addedNewListener = true;
+  }
+
+  domainSet.add(this._domain);
+
   let rli = this;
   Object.defineProperty(this, 'rli', {
     get: deprecate(() => rli,
@@ -264,7 +290,7 @@ function REPLServer(prompt,
       // statement rather than an object literal.  So, we first try
       // to wrap it in parentheses, so that it will be interpreted as
       // an expression.  Note that if the above condition changes,
-      // lib/internal/repl/recoverable.js needs to be changed to match.
+      // lib/internal/repl/utils.js needs to be changed to match.
       code = `(${code.trim()})\n`;
       wrappedCmd = true;
     }
@@ -461,22 +487,31 @@ function REPLServer(prompt,
       }
     }
 
-    if (errStack === '') {
-      errStack = `Thrown: ${self.writer(e)}\n`;
-    } else {
-      const ln = errStack.endsWith('\n') ? '' : '\n';
-      errStack = `Thrown:\n${errStack}${ln}`;
-    }
-
     if (!self.underscoreErrAssigned) {
       self.lastError = e;
     }
 
     const top = replMap.get(self);
-    top.outputStream.write(errStack);
-    top.clearBufferedCommand();
-    top.lines.level = [];
-    top.displayPrompt();
+    if (options[kStandaloneREPL] &&
+        process.listenerCount('uncaughtException') !== 0) {
+      process.nextTick(() => {
+        process.emit('uncaughtException', e);
+        top.clearBufferedCommand();
+        top.lines.level = [];
+        top.displayPrompt();
+      });
+    } else {
+      if (errStack === '') {
+        errStack = `Thrown: ${self.writer(e)}\n`;
+      } else {
+        const ln = errStack.endsWith('\n') ? '' : '\n';
+        errStack = `Thrown:\n${errStack}${ln}`;
+      }
+      top.outputStream.write(errStack);
+      top.clearBufferedCommand();
+      top.lines.level = [];
+      top.displayPrompt();
+    }
   });
 
   self.resetContext();

test/parallel/test-repl-pretty-custom-stack.js

@@ -40,8 +40,6 @@ process.on('uncaughtException', (e) => {
   throw e;
 });
 
-process.on('exit', () => (Error.prepareStackTrace = origPrepareStackTrace));
-
 const tests = [
   {
     // test .load for a file that throws

test/parallel/test-repl-uncaught-exception-async.js

@@ -0,0 +1,44 @@
+'use strict';
+
+// This verifies that adding an `uncaughtException` listener in an REPL instance
+// does not suppress errors in the whole application. Adding such listener
+// should throw.
+
+require('../common');
+const ArrayStream = require('../common/arraystream');
+const repl = require('repl');
+const assert = require('assert');
+
+let accum = '';
+
+const output = new ArrayStream();
+output.write = (data) => accum += data.replace('\r', '');
+
+const r = repl.start({
+  prompt: '',
+  input: new ArrayStream(),
+  output,
+  terminal: false,
+  useColors: false,
+  global: false
+});
+
+r.write(
+  'process.nextTick(() => {\n' +
+  '  process.on("uncaughtException", () => console.log("Foo"));\n' +
+  '  throw new TypeError("foobar");\n' +
+  '});\n'
+);
+r.write(
+  'setTimeout(() => {\n' +
+  '  throw new RangeError("abc");\n' +
+  '}, 1);console.log()\n'
+);
+r.close();
+
+setTimeout(() => {
+  const len = process.listenerCount('uncaughtException');
+  process.removeAllListeners('uncaughtException');
+  assert.strictEqual(len, 0);
+  assert(/ERR_INVALID_REPL_INPUT.*(?!Type)RangeError: abc/s.test(accum));
+}, 2);

test/parallel/test-repl-uncaught-exception-standalone.js

@@ -0,0 +1,38 @@
+'use strict';
+const common = require('../common');
+const assert = require('assert');
+const cp = require('child_process');
+const child = cp.spawn(process.execPath, ['-i']);
+let output = '';
+
+child.stdout.setEncoding('utf8');
+child.stdout.on('data', (data) => {
+  output += data;
+});
+
+child.on('exit', common.mustCall(() => {
+  const results = output.split('\n');
+  results.shift();
+  assert.deepStrictEqual(
+    results,
+    [
+      'Type ".help" for more information.',
+      // x\n
+      '> Thrown:',
+      'ReferenceError: x is not defined',
+      // Added `uncaughtException` listener.
+      '> short',
+      'undefined',
+      // x\n
+      '> Foobar',
+      '> '
+    ]
+  );
+}));
+
+child.stdin.write('x\n');
+child.stdin.write(
+  'process.on("uncaughtException", () => console.log("Foobar"));' +
+  'console.log("short")\n');
+child.stdin.write('x\n');
+child.stdin.end();

test/parallel/test-repl-uncaught-exception.js

@@ -0,0 +1,72 @@
+'use strict';
+require('../common');
+const ArrayStream = require('../common/arraystream');
+const assert = require('assert');
+const repl = require('repl');
+
+let count = 0;
+
+function run({ command, expected }) {
+  let accum = '';
+
+  const output = new ArrayStream();
+  output.write = (data) => accum += data.replace('\r', '');
+
+  const r = repl.start({
+    prompt: '',
+    input: new ArrayStream(),
+    output,
+    terminal: false,
+    useColors: false
+  });
+
+  r.write(`${command}\n`);
+  if (typeof expected === 'string') {
+    assert.strictEqual(accum, expected);
+  } else {
+    assert(expected.test(accum), accum);
+  }
+
+  // Verify that the repl is still working as expected.
+  accum = '';
+  r.write('1 + 1\n');
+  assert.strictEqual(accum, '2\n');
+  r.close();
+  count++;
+}
+
+const tests = [
+  {
+    command: 'x',
+    expected: 'Thrown:\n' +
+              'ReferenceError: x is not defined\n'
+  },
+  {
+    command: 'process.on("uncaughtException", () => console.log("Foobar"));\n',
+    expected: /^Thrown:\nTypeError \[ERR_INVALID_REPL_INPUT]: Listeners for `/
+  },
+  {
+    command: 'x;\n',
+    expected: 'Thrown:\n' +
+              'ReferenceError: x is not defined\n'
+  },
+  {
+    command: 'process.on("uncaughtException", () => console.log("Foobar"));' +
+             'console.log("Baz");\n',
+    expected: /^Thrown:\nTypeError \[ERR_INVALID_REPL_INPUT]: Listeners for `/
+  },
+  {
+    command: 'console.log("Baz");' +
+             'process.on("uncaughtException", () => console.log("Foobar"));\n',
+    expected: /^Baz\nThrown:\nTypeError \[ERR_INVALID_REPL_INPUT]:.*uncaughtException/
+  }
+];
+
+process.on('exit', () => {
+  // To actually verify that the test passed we have to make sure no
+  // `uncaughtException` listeners exist anymore.
+  process.removeAllListeners('uncaughtException');
+  assert.strictEqual(count, tests.length);
+});
+
+tests.forEach(run);