Skip to content
Permalink
Browse files

repl: handle uncaughtException properly

When running the REPL as standalone program it's now possible to use
`process.on('uncaughtException', listener)`. It is going to use those
listeners from now on and the regular error output is suppressed.

It also fixes the issue that REPL instances started inside of an
application would silence all application errors. It is now prohibited
to add the exception listener in such REPL instances. Trying to add
such listeners throws an `ERR_INVALID_REPL_INPUT` error.

Fixes: #19998

PR-URL: #27151
Fixes: #19998
Reviewed-By: Lance Ball <lball@redhat.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
  • Loading branch information...
BridgeAR committed May 17, 2018
1 parent 9a174db commit 422e8f762873aef4a37185f3237c0d666c929d8e
@@ -1310,8 +1310,14 @@ An invalid `options.protocol` was passed to `http.request()`.
<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
@@ -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
@@ -895,6 +895,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}.`;
@@ -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();
@@ -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
@@ -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);
@@ -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();
@@ -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);

0 comments on commit 422e8f7

Please sign in to comment.
You can’t perform that action at this time.