Skip to content

Commit f85b9d9

Browse files
addaleaxaduh95
authored andcommitted
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 bd3cba5. PR-URL: #62188 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Chemi Atlow <chemi@atlow.co.il>
1 parent 47a2132 commit f85b9d9

File tree

3 files changed

+116
-7
lines changed

3 files changed

+116
-7
lines changed

doc/api/repl.md

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -709,6 +709,9 @@ npx codemod@latest @nodejs/repl-builtin-modules
709709
<!-- YAML
710710
added: v0.1.91
711711
changes:
712+
- version: REPLACEME
713+
pr-url: https://github.com/nodejs/node/pull/62188
714+
description: The `handleError` parameter has been added.
712715
- version: v24.1.0
713716
pr-url: https://github.com/nodejs/node/pull/58003
714717
description: Added the possibility to add/edit/remove multilines
@@ -787,6 +790,17 @@ changes:
787790
previews or not. **Default:** `true` with the default eval function and
788791
`false` in case a custom eval function is used. If `terminal` is falsy, then
789792
there are no previews and the value of `preview` has no effect.
793+
* `handleError` {Function} This function customizes error handling in the REPL.
794+
It receives the thrown exception as its first argument and must return one
795+
of the following values synchronously:
796+
* `'print'` to print the error to the output stream (default behavior).
797+
* `'ignore'` to skip all remaining error handling.
798+
* `'unhandled'` to treat the exception as fully unhandled. In this case,
799+
the error will be passed to process-wide exception handlers, such as
800+
the [`'uncaughtException'`][] event.
801+
The `'unhandled'` value may or may not be desirable in situations
802+
where the `REPLServer` instance has been closed, depending on the particular
803+
use case.
790804
* Returns: {repl.REPLServer}
791805

792806
The `repl.start()` method creates and starts a [`repl.REPLServer`][] instance.

lib/repl.js

Lines changed: 18 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -125,6 +125,7 @@ const {
125125
ERR_INVALID_ARG_VALUE,
126126
ERR_INVALID_REPL_EVAL_CONFIG,
127127
ERR_INVALID_REPL_INPUT,
128+
ERR_INVALID_STATE,
128129
ERR_MISSING_ARGS,
129130
ERR_SCRIPT_EXECUTION_INTERRUPTED,
130131
},
@@ -195,14 +196,9 @@ function setupExceptionCapture() {
195196

196197
process.addUncaughtExceptionCaptureCallback((err) => {
197198
const store = replContext.getStore();
198-
// TODO(addaleax): Add back a `store.replServer.closed` check here
199-
// as a semver-major change.
200-
// This check may need to allow for an opt-out, since the change in
201-
// behavior could lead to DoS vulnerabilities (e.g. in the case of
202-
// the net-based REPL described in our docs).
203199
if (store?.replServer) {
204-
store.replServer._handleError(err);
205-
return true; // We handled it
200+
const result = store.replServer._handleError(err);
201+
return result !== 'unhandled'; // We handled it
206202
}
207203
// No active REPL context - let other handlers try
208204
});
@@ -387,6 +383,7 @@ class REPLServer extends Interface {
387383
this.lastError = undefined;
388384
this.breakEvalOnSigint = !!options.breakEvalOnSigint;
389385
this.editorMode = false;
386+
this._userErrorHandler = options.handleError;
390387
// Context id for use with the inspector protocol.
391388
this[kContextId] = undefined;
392389
this[kLastCommandErrored] = false;
@@ -987,6 +984,20 @@ class REPLServer extends Interface {
987984
}
988985
_handleError(e) {
989986
debug('handle error');
987+
if (this._userErrorHandler) {
988+
const state = this._userErrorHandler(e);
989+
if (state !== 'ignore' && state !== 'print' && state !== 'unhandled') {
990+
throw new ERR_INVALID_STATE(
991+
'External REPL error handler must return either "ignore", "print"' +
992+
`, or "unhandled", but received: ${state}`);
993+
}
994+
if (state === 'ignore') {
995+
return;
996+
}
997+
if (state === 'unhandled') {
998+
return 'unhandled';
999+
}
1000+
}
9901001
let errStack = '';
9911002

9921003
if (typeof e === 'object' && e !== null) {
Lines changed: 84 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,84 @@
1+
'use strict';
2+
const common = require('../common');
3+
const { start } = require('node:repl');
4+
const assert = require('node:assert');
5+
const { PassThrough } = require('node:stream');
6+
const { once } = require('node:events');
7+
const test = require('node:test');
8+
const { spawn } = require('node:child_process');
9+
10+
function* generateCases() {
11+
for (const async of [false, true]) {
12+
for (const handleErrorReturn of ['ignore', 'print', 'unhandled', 'badvalue']) {
13+
if (handleErrorReturn === 'badvalue' && async) {
14+
// Handled through a separate test using a child process
15+
continue;
16+
}
17+
yield { async, handleErrorReturn };
18+
}
19+
}
20+
}
21+
22+
for (const { async, handleErrorReturn } of generateCases()) {
23+
test(`async: ${async}, handleErrorReturn: ${handleErrorReturn}`, async () => {
24+
let err;
25+
const options = {
26+
input: new PassThrough(),
27+
output: new PassThrough().setEncoding('utf8'),
28+
handleError: common.mustCall((e) => {
29+
err = e;
30+
queueMicrotask(() => repl.emit('handled-error'));
31+
return handleErrorReturn;
32+
})
33+
};
34+
35+
let uncaughtExceptionEvent;
36+
if (handleErrorReturn === 'unhandled' && async) {
37+
process.removeAllListeners('uncaughtException'); // Remove the test runner's handler
38+
uncaughtExceptionEvent = once(process, 'uncaughtException');
39+
}
40+
41+
const repl = start(options);
42+
const inputString = async ?
43+
'setImmediate(() => { throw new Error("testerror") })\n42\n' :
44+
'throw new Error("testerror")\n42\n';
45+
if (handleErrorReturn === 'badvalue') {
46+
assert.throws(() => options.input.end(inputString), /ERR_INVALID_STATE/);
47+
return;
48+
}
49+
options.input.end(inputString);
50+
51+
await once(repl, 'handled-error');
52+
assert.strictEqual(err.message, 'testerror');
53+
const outputString = options.output.read();
54+
assert.match(outputString, /42/);
55+
56+
if (handleErrorReturn === 'print') {
57+
assert.match(outputString, /testerror/);
58+
} else {
59+
assert.doesNotMatch(outputString, /testerror/);
60+
}
61+
62+
if (uncaughtExceptionEvent) {
63+
const [uncaughtErr] = await uncaughtExceptionEvent;
64+
assert.strictEqual(uncaughtErr, err);
65+
}
66+
});
67+
}
68+
69+
test('async: true, handleErrorReturn: badvalue', async () => {
70+
// Can't test this the same way as the other combinations
71+
// since this will take the process down in a way that
72+
// cannot be caught.
73+
const proc = spawn(process.execPath, ['-e', `
74+
require('node:repl').start({
75+
handleError: () => 'badvalue'
76+
})
77+
`], { encoding: 'utf8', stdio: 'pipe' });
78+
proc.stdin.end('throw new Error("foo");');
79+
let stderr = '';
80+
proc.stderr.setEncoding('utf8').on('data', (data) => stderr += data);
81+
const [exit] = await once(proc, 'close');
82+
assert.strictEqual(exit, 1);
83+
assert.match(stderr, /ERR_INVALID_STATE.+badvalue/);
84+
});

0 commit comments

Comments
 (0)