Skip to content

Commit bd3cba5

Browse files
authored
repl: handle exceptions from async context after close
a9da9ff recently restructured async context handling in the REPL source so that it no longer uses the domain module, and instead performs its own async context tracking. That change was not intended to be breaking, but it affected behavior for uncaught exceptions thrown after the REPL was closed. Those would now be treated as uncaught exceptions on the process level, which is probably not intentional in most situations. While it's understandably not great that we handle execptions after closing the REPL instance, I am confident that it's best to restore the previous behavior for now and add more granular handling options separately and intentionally in a (potentially semver-major) follow-up change. Refs: #61227 PR-URL: #62165 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
1 parent 70b3570 commit bd3cba5

File tree

2 files changed

+29
-1
lines changed

2 files changed

+29
-1
lines changed

lib/repl.js

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -195,7 +195,12 @@ function setupExceptionCapture() {
195195

196196
process.addUncaughtExceptionCaptureCallback((err) => {
197197
const store = replContext.getStore();
198-
if (store?.replServer && !store.replServer.closed) {
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).
203+
if (store?.replServer) {
199204
store.replServer._handleError(err);
200205
return true; // We handled it
201206
}
Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
'use strict';
2+
const common = require('../common');
3+
const { start } = require('node:repl');
4+
const { PassThrough } = require('node:stream');
5+
const assert = require('node:assert');
6+
7+
// This test verifies that uncaught exceptions in the REPL
8+
// do not bring down the process, even if stdin may already
9+
// have been ended at that point (and the REPL closed as
10+
// a result of that).
11+
const input = new PassThrough();
12+
const output = new PassThrough().setEncoding('utf8');
13+
start({
14+
input,
15+
output,
16+
terminal: false,
17+
});
18+
19+
input.end('setImmediate(() => { throw new Error("test"); });\n');
20+
21+
setImmediate(common.mustCall(() => {
22+
assert.match(output.read(), /Uncaught Error: test/);
23+
}));

0 commit comments

Comments
 (0)