Repository: nodejs/node
Pull request: repl: add customizable error handling

Whether or not an exception should be handled when it was thrown in the REPL's context but the REPL is already closed is something that will depend on the details of the specific REPL instance's use case.

Adding this option lets the REPL's creator control the details of this behavior.

This resolves a TODO recently added in bd3cba5633e8a36.

<!--
Before submitting a pull request, please read:

- the CONTRIBUTING guide at https://github.com/nodejs/node/blob/HEAD/CONTRIBUTING.md
- the commit message formatting guidelines at
  https://github.com/nodejs/node/blob/HEAD/doc/contributing/pull-requests.md#commit-message-guidelines

For code changes:
1. Include tests for any bug fixes or new features.
2. Update documentation if relevant.
3. Ensure that `make -j4 test` (UNIX), or `vcbuild test` (Windows) passes.

If you believe this PR should be highlighted in the Node.js CHANGELOG
please add the `notable-change` label.

Developer's Certificate of Origin 1.1

By making a contribution to this project, I certify that:

(a) The contribution was created in whole or in part by me and I
    have the right to submit it under the open source license
    indicated in the file; or

(b) The contribution is based upon previous work that, to the best
    of my knowledge, is covered under an appropriate open source
    license and I have the right under that license to submit that
    work with modifications, whether created in whole or in part
    by me, under the same open source license (unless I am
    permitted to submit under a different license), as indicated
    in the file; or

(c) The contribution was provided directly to me by some other
    person who certified (a), (b) or (c) and I have not modified
    it.

(d) I understand and agree that this project and the contribution
    are public and that a record of the contribution (including all
    personal information I submit with it, including my sign-off) is
    maintained indefinitely and may be redistributed consistent with
    this project or the open source license(s) involved.
-->


Changed files:
filename: doc/api/repl.md
patch: @@ -709,6 +709,9 @@ npx codemod@latest @nodejs/repl-builtin-modules
 <!-- YAML
 added: v0.1.91
 changes:
+  - version: REPLACEME
+    pr-url: https://github.com/nodejs/node/pull/62188
+    description: The `handleError` parameter has been added.
   - version: v24.1.0
     pr-url: https://github.com/nodejs/node/pull/58003
     description: Added the possibility to add/edit/remove multilines
@@ -787,6 +790,17 @@ changes:
     previews or not. **Default:** `true` with the default eval function and
     `false` in case a custom eval function is used. If `terminal` is falsy, then
     there are no previews and the value of `preview` has no effect.
+  * `handleError` {Function} This function customizes error handling in the REPL.
+    It receives the thrown exception as its first argument and must return one
+    of the following values synchronously:
+    * `'print'` to print the error to the output stream (default behavior).
+    * `'ignore'` to skip all remaining error handling.
+    * `'unhandled'` to treat the exception as fully unhandled. In this case,
+      the error will be passed to process-wide exception handlers, such as
+      the [`'uncaughtException'`][] event.
+      The `'unhandled'` value may or may not be desirable in situations
+      where the `REPLServer` instance has been closed, depending on the particular
+      use case.
 * Returns: {repl.REPLServer}
 
 The `repl.start()` method creates and starts a [`repl.REPLServer`][] instance.

filename: lib/repl.js
patch: @@ -125,6 +125,7 @@ const {
     ERR_INVALID_ARG_VALUE,
     ERR_INVALID_REPL_EVAL_CONFIG,
     ERR_INVALID_REPL_INPUT,
+    ERR_INVALID_STATE,
     ERR_MISSING_ARGS,
     ERR_SCRIPT_EXECUTION_INTERRUPTED,
   },
@@ -195,14 +196,9 @@ function setupExceptionCapture() {
 
   process.addUncaughtExceptionCaptureCallback((err) => {
     const store = replContext.getStore();
-    // TODO(addaleax): Add back a `store.replServer.closed` check here
-    // as a semver-major change.
-    // This check may need to allow for an opt-out, since the change in
-    // behavior could lead to DoS vulnerabilities (e.g. in the case of
-    // the net-based REPL described in our docs).
     if (store?.replServer) {
-      store.replServer._handleError(err);
-      return true; // We handled it
+      const result = store.replServer._handleError(err);
+      return result !== 'unhandled'; // We handled it
     }
     // No active REPL context - let other handlers try
   });
@@ -387,6 +383,7 @@ class REPLServer extends Interface {
     this.lastError = undefined;
     this.breakEvalOnSigint = !!options.breakEvalOnSigint;
     this.editorMode = false;
+    this._userErrorHandler = options.handleError;
     // Context id for use with the inspector protocol.
     this[kContextId] = undefined;
     this[kLastCommandErrored] = false;
@@ -987,6 +984,20 @@ class REPLServer extends Interface {
   }
   _handleError(e) {
     debug('handle error');
+    if (this._userErrorHandler) {
+      const state = this._userErrorHandler(e);
+      if (state !== 'ignore' && state !== 'print' && state !== 'unhandled') {
+        throw new ERR_INVALID_STATE(
+          'External REPL error handler must return either "ignore", "print"' +
+          `, or "unhandled", but received: ${state}`);
+      }
+      if (state === 'ignore') {
+        return;
+      }
+      if (state === 'unhandled') {
+        return 'unhandled';
+      }
+    }
     let errStack = '';
 
     if (typeof e === 'object' && e !== null) {

filename: test/parallel/test-repl-user-error-handler.js
patch: @@ -0,0 +1,84 @@
+'use strict';
+const common = require('../common');
+const { start } = require('node:repl');
+const assert = require('node:assert');
+const { PassThrough } = require('node:stream');
+const { once } = require('node:events');
+const test = require('node:test');
+const { spawn } = require('node:child_process');
+
+function* generateCases() {
+  for (const async of [false, true]) {
+    for (const handleErrorReturn of ['ignore', 'print', 'unhandled', 'badvalue']) {
+      if (handleErrorReturn === 'badvalue' && async) {
+        // Handled through a separate test using a child process
+        continue;
+      }
+      yield { async, handleErrorReturn };
+    }
+  }
+}
+
+for (const { async, handleErrorReturn } of generateCases()) {
+  test(`async: ${async}, handleErrorReturn: ${handleErrorReturn}`, async () => {
+    let err;
+    const options = {
+      input: new PassThrough(),
+      output: new PassThrough().setEncoding('utf8'),
+      handleError: common.mustCall((e) => {
+        err = e;
+        queueMicrotask(() => repl.emit('handled-error'));
+        return handleErrorReturn;
+      })
+    };
+
+    let uncaughtExceptionEvent;
+    if (handleErrorReturn === 'unhandled' && async) {
+      process.removeAllListeners('uncaughtException'); // Remove the test runner's handler
+      uncaughtExceptionEvent = once(process, 'uncaughtException');
+    }
+
+    const repl = start(options);
+    const inputString = async ?
+      'setImmediate(() => { throw new Error("testerror") })\n42\n' :
+      'throw new Error("testerror")\n42\n';
+    if (handleErrorReturn === 'badvalue') {
+      assert.throws(() => options.input.end(inputString), /ERR_INVALID_STATE/);
+      return;
+    }
+    options.input.end(inputString);
+
+    await once(repl, 'handled-error');
+    assert.strictEqual(err.message, 'testerror');
+    const outputString = options.output.read();
+    assert.match(outputString, /42/);
+
+    if (handleErrorReturn === 'print') {
+      assert.match(outputString, /testerror/);
+    } else {
+      assert.doesNotMatch(outputString, /testerror/);
+    }
+
+    if (uncaughtExceptionEvent) {
+      const [uncaughtErr] = await uncaughtExceptionEvent;
+      assert.strictEqual(uncaughtErr, err);
+    }
+  });
+}
+
+test('async: true, handleErrorReturn: badvalue', async () => {
+  // Can't test this the same way as the other combinations
+  // since this will take the process down in a way that
+  // cannot be caught.
+  const proc = spawn(process.execPath, ['-e', `
+    require('node:repl').start({
+      handleError: () => 'badvalue'
+    })
+  `], { encoding: 'utf8', stdio: 'pipe' });
+  proc.stdin.end('throw new Error("foo");');
+  let stderr = '';
+  proc.stderr.setEncoding('utf8').on('data', (data) => stderr += data);
+  const [exit] = await once(proc, 'close');
+  assert.strictEqual(exit, 1);
+  assert.match(stderr, /ERR_INVALID_STATE.+badvalue/);
+});