Skip to content

Commit

Permalink
repl: don't use deprecated domain module
Browse files Browse the repository at this point in the history
  • Loading branch information
RedYetiDev committed Oct 1, 2024
1 parent 89a2f56 commit 2823b04
Show file tree
Hide file tree
Showing 13 changed files with 37 additions and 210 deletions.
7 changes: 0 additions & 7 deletions doc/api/errors.md
Original file line number Diff line number Diff line change
Expand Up @@ -2021,13 +2021,6 @@ An invalid `options.protocol` was passed to `http.request()`.
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`][]. The conditions under which this
error is used are described in the [`REPL`][] documentation.

<a id="ERR_INVALID_RETURN_PROPERTY"></a>

### `ERR_INVALID_RETURN_PROPERTY`
Expand Down
32 changes: 6 additions & 26 deletions doc/api/repl.md
Original file line number Diff line number Diff line change
Expand Up @@ -151,34 +151,18 @@ global or scoped variable, the input `fs` will be evaluated on-demand as

<!-- YAML
changes:
- version: REPLACEME
pr-url: https://github.com/nodejs/node/pull/PR_TODO
description: The ``process.setUncaughtExceptionCaptureCallback()`
function is now used instead of the `domain` module.
- version: v12.3.0
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 only emit the [`'uncaughtException'`][] event in the
standalone REPL. Adding a listener for this event in a REPL within
another Node.js program results in [`ERR_INVALID_REPL_INPUT`][].

```js
const r = repl.start();

r.write('process.on("uncaughtException", () => console.log("Foobar"));\n');
// Output stream includes:
// TypeError [ERR_INVALID_REPL_INPUT]: Listeners for `uncaughtException`
// cannot be used in the REPL

r.close();
```

* Trying to use [`process.setUncaughtExceptionCaptureCallback()`][] throws
an [`ERR_DOMAIN_CANNOT_SET_UNCAUGHT_EXCEPTION_CAPTURE`][] error.
The REPL uses the [`process.setUncaughtExceptionCaptureCallback()`][] to catch all uncaught exceptions for that
REPL session. For more information on potential side effects, refer to it's documentation.

#### Assignment of the `_` (underscore) variable

Expand Down Expand Up @@ -768,12 +752,8 @@ avoiding open network interfaces.

[TTY keybindings]: readline.md#tty-keybindings
[ZSH]: https://en.wikipedia.org/wiki/Z_shell
[`'uncaughtException'`]: process.md#event-uncaughtexception
[`--no-experimental-repl-await`]: cli.md#--no-experimental-repl-await
[`ERR_DOMAIN_CANNOT_SET_UNCAUGHT_EXCEPTION_CAPTURE`]: errors.md#err_domain_cannot_set_uncaught_exception_capture
[`ERR_INVALID_REPL_INPUT`]: errors.md#err_invalid_repl_input
[`curl(1)`]: https://curl.haxx.se/docs/manpage.html
[`domain`]: domain.md
[`process.setUncaughtExceptionCaptureCallback()`]: process.md#processsetuncaughtexceptioncapturecallbackfn
[`readline.InterfaceCompleter`]: readline.md#use-of-the-completer-function
[`repl.ReplServer`]: #class-replserver
Expand Down
1 change: 0 additions & 1 deletion lib/internal/errors.js
Original file line number Diff line number Diff line change
Expand Up @@ -1502,7 +1502,6 @@ 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}" hook but got ${determineSpecificType(value)}.`;
Expand Down
83 changes: 28 additions & 55 deletions lib/repl.js
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,7 @@ const {
ArrayPrototypeUnshift,
Boolean,
Error: MainContextError,
FunctionPrototypeApply,
FunctionPrototypeBind,
JSONStringify,
MathMaxApply,
Expand All @@ -78,7 +79,6 @@ const {
RegExpPrototypeExec,
SafePromiseRace,
SafeSet,
SafeWeakSet,
StringPrototypeCharAt,
StringPrototypeCodePointAt,
StringPrototypeEndsWith,
Expand Down Expand Up @@ -138,7 +138,6 @@ ArrayPrototypeForEach(
BuiltinModule.getSchemeOnlyModuleNames(),
(lib) => ArrayPrototypePush(nodeSchemeBuiltinLibs, `node:${lib}`),
);
const domain = require('domain');
let debug = require('internal/util/debuglog').debuglog('repl', (fn) => {
debug = fn;
});
Expand All @@ -147,7 +146,6 @@ const {
codes: {
ERR_CANNOT_WATCH_SIGINT,
ERR_INVALID_REPL_EVAL_CONFIG,
ERR_INVALID_REPL_INPUT,
ERR_MISSING_ARGS,
ERR_SCRIPT_EXECUTION_INTERRUPTED,
},
Expand Down Expand Up @@ -205,14 +203,11 @@ const globalBuiltins =
new SafeSet(vm.runInNewContext('Object.getOwnPropertyNames(globalThis)'));

const parentModule = module;
const domainSet = new SafeWeakSet();

const kBufferedCommandSymbol = Symbol('bufferedCommand');
const kContextId = Symbol('contextId');
const kLoadingSymbol = Symbol('loading');

let addedNewListener = false;

try {
// Hack for require.resolve("./relative") to work properly.
module.filename = path.resolve('repl');
Expand Down Expand Up @@ -347,7 +342,6 @@ function REPLServer(prompt,

this.allowBlockingCompletions = !!options.allowBlockingCompletions;
this.useColors = !!options.useColors;
this._domain = options.domain || domain.create();
this.useGlobal = !!useGlobal;
this.ignoreUndefined = !!ignoreUndefined;
this.replMode = replMode || module.exports.REPL_MODE_SLOPPY;
Expand All @@ -370,27 +364,10 @@ function REPLServer(prompt,
// It is possible to introspect the running REPL accessing this variable
// from inside the REPL. This is useful for anyone working on the REPL.
module.exports.repl = this;
} else if (!addedNewListener) {
// 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`.
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);
process.setUncaughtExceptionCaptureCallback(null);
process.setUncaughtExceptionCaptureCallback(handleEvaluationError);

const savedRegExMatches = ['', '', '', '', '', '', '', '', '', ''];
const sep = '\u0000\u0000\u0000';
Expand Down Expand Up @@ -613,13 +590,8 @@ function REPLServer(prompt,
}
} catch (e) {
err = e;

if (process.domain) {
debug('not recoverable, send to domain');
process.domain.emit('error', err);
process.domain.exit();
return;
}
handleEvaluationError(e);
return;
}

if (awaitPromise && !err) {
Expand All @@ -645,10 +617,8 @@ function REPLServer(prompt,
const result = (await promise)?.value;
finishExecution(null, result);
} catch (err) {
if (err && process.domain) {
debug('not recoverable, send to domain');
process.domain.emit('error', err);
process.domain.exit();
if (err) {
handleEvaluationError(err);
return;
}
finishExecution(err);
Expand All @@ -666,10 +636,17 @@ function REPLServer(prompt,
}
}

self.eval = self._domain.bind(eval_);
self.eval = function(...args) {
try {
FunctionPrototypeApply(eval_, this, args);
} catch (e) {
handleEvaluationError(e);
}
};

self._domain.on('error', function debugDomainError(e) {
debug('domain error');
function handleEvaluationError(e) {
self.emit('repl_error', e);
debug('eval error');
let errStack = '';

if (typeof e === 'object' && e !== null) {
Expand Down Expand Up @@ -697,11 +674,6 @@ function REPLServer(prompt,
});
decorateErrorStack(e);

if (e.domainThrown) {
delete e.domain;
delete e.domainThrown;
}

if (isError(e)) {
if (e.stack) {
if (e.name === 'SyntaxError') {
Expand Down Expand Up @@ -779,7 +751,7 @@ function REPLServer(prompt,
self.lines.level = [];
self.displayPrompt();
}
});
};

self.clearBufferedCommand();

Expand Down Expand Up @@ -952,7 +924,7 @@ function REPLServer(prompt,
self.displayPrompt();
return;
}
self._domain.emit('error', e.err || e);
handleEvaluationError(e.err || e);
}

// Clear buffer if no SyntaxErrors
Expand All @@ -972,8 +944,7 @@ function REPLServer(prompt,
self.output.write(self.writer(ret) + '\n');
}

// Display prompt again (unless we already did by emitting the 'error'
// event on the domain instance).
// Display prompt again
if (!e) {
self.displayPrompt();
}
Expand Down Expand Up @@ -1083,15 +1054,17 @@ REPLServer.prototype.clearBufferedCommand = function clearBufferedCommand() {
REPLServer.prototype.close = function close() {
if (this.terminal && this._flushing && !this._closingOnFlush) {
this._closingOnFlush = true;
this.once('flushHistory', () =>
ReflectApply(Interface.prototype.close, this, []),
);
this.once('flushHistory', () => {
process.setUncaughtExceptionCaptureCallback(null);
ReflectApply(Interface.prototype.close, this, []);
});

return;
}
process.nextTick(() =>
ReflectApply(Interface.prototype.close, this, []),
);
process.nextTick(() => {
process.setUncaughtExceptionCaptureCallback(null);
ReflectApply(Interface.prototype.close, this, []);
});
};

REPLServer.prototype.createContext = function() {
Expand Down
2 changes: 1 addition & 1 deletion test/fixtures/repl-tab-completion-nested-repls.js
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ const putIn = new ArrayStream();
const testMe = repl.start('', putIn);

// Some errors are passed to the domain, but do not callback.
testMe._domain.on('error', function(err) {
testMe.on('repl_error', function(err) {
throw err;
});

Expand Down
45 changes: 0 additions & 45 deletions test/parallel/test-repl-domain.js

This file was deleted.

7 changes: 0 additions & 7 deletions test/parallel/test-repl-save-load.js
Original file line number Diff line number Diff line change
Expand Up @@ -34,13 +34,6 @@ const works = [['inner.one'], 'inner.o'];
const putIn = new ArrayStream();
const testMe = repl.start('', putIn);

// Some errors might be passed to the domain.
testMe._domain.on('error', function(reason) {
const err = new Error('Test failed');
err.reason = reason;
throw err;
});

const testFile = [
'let inner = (function() {',
' return {one:1};',
Expand Down
3 changes: 0 additions & 3 deletions test/parallel/test-repl-tab-complete-import.js
Original file line number Diff line number Diff line change
Expand Up @@ -25,9 +25,6 @@ const testMe = repl.start({
allowBlockingCompletions: true
});

// Some errors are passed to the domain, but do not callback
testMe._domain.on('error', assert.ifError);

// Tab complete provides built in libs for import()
testMe.complete('import(\'', common.mustCall((error, data) => {
assert.strictEqual(error, null);
Expand Down
3 changes: 0 additions & 3 deletions test/parallel/test-repl-tab-complete.js
Original file line number Diff line number Diff line change
Expand Up @@ -60,9 +60,6 @@ const testMe = repl.start({
allowBlockingCompletions: true
});

// Some errors are passed to the domain, but do not callback
testMe._domain.on('error', assert.ifError);

// Tab Complete will not break in an object literal
putIn.run([
'var inner = {',
Expand Down
4 changes: 1 addition & 3 deletions test/parallel/test-repl-tab.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
'use strict';
const common = require('../common');
require('../common');
const assert = require('assert');
const repl = require('repl');
const zlib = require('zlib');
Expand All @@ -11,8 +11,6 @@ const testMe = repl.start('', putIn, function(cmd, context, filename,
callback(null, cmd);
});

testMe._domain.on('error', common.mustNotCall());

testMe.complete('', function(err, results) {
assert.strictEqual(err, null);
});
2 changes: 1 addition & 1 deletion test/parallel/test-repl-top-level-await.js
Original file line number Diff line number Diff line change
Expand Up @@ -174,7 +174,7 @@ async function ordinaryTests() {
'undefined',
]],
// Regression test for https://github.com/nodejs/node/issues/43777.
['await Promise.resolve(123), Promise.resolve(456)', 'Promise {', { line: 0 }],
['await Promise.resolve(123), Promise.resolve(456)', 'Promise { 456 }', { line: 0 }],
['await Promise.resolve(123), await Promise.resolve(456)', '456'],
['await (Promise.resolve(123), Promise.resolve(456))', '456'],
];
Expand Down
Loading

0 comments on commit 2823b04

Please sign in to comment.