Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

repl: do not swallow errors in nested REPLs #23004

Closed
wants to merge 2 commits into from
Closed
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions lib/repl.js
Original file line number Diff line number Diff line change
Expand Up @@ -1002,6 +1002,7 @@ function complete(line, callback) {
// all this is only profitable if the nested REPL
// does not have a bufferedCommand
if (!magic[kBufferedCommandSymbol]) {
magic._domain.on('error', (err) => { throw err; });
return magic.complete(line, callback);
}
}
Expand Down
50 changes: 50 additions & 0 deletions test/fixtures/repl-tab-completion-nested-repls.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
// Tab completion sometimes uses a separate REPL instance under the hood.
// That REPL instance has its own domain. Make sure domain errors trickle back
// up to the main REPL.
//
// Ref: https://github.com/nodejs/node/issues/21586

'use strict';

const { Stream } = require('stream');
const { inherits } = require('util');
function noop() {}

// A stream to push an array into a REPL
function ArrayStream() {
this.run = function(data) {
data.forEach((line) => {
this.emit('data', `${line}\n`);
});
};
}

inherits(ArrayStream, Stream);
ArrayStream.prototype.readable = true;
ArrayStream.prototype.writable = true;
ArrayStream.prototype.pause = noop;
ArrayStream.prototype.resume = noop;
ArrayStream.prototype.write = noop;

const repl = require('repl');

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) {
throw err;
});

// Nesting of structures causes REPL to use a nested REPL for completion.
putIn.run([
'var top = function() {',
'r = function test (',
' one, two) {',
'var inner = {',
' one:1',
'};'
]);

// In Node.js 10.11.0, this next line will terminate the repl silently...
testMe.complete('inner.o', () => { throw new Error('fhqwhgads'); });
21 changes: 21 additions & 0 deletions test/parallel/test-repl-tab-complete-nested-repls.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
// Tab completion sometimes uses a separate REPL instance under the hood.
// That REPL instance has its own domain. Make sure domain errors trickle back
// up to the main REPL.
//
// Ref: https://github.com/nodejs/node/issues/21586

'use strict';

require('../common');
const fixtures = require('../common/fixtures');

const assert = require('assert');
const { spawnSync } = require('child_process');

const testFile = fixtures.path('repl-tab-completion-nested-repls.js');
const result = spawnSync(process.execPath, [testFile]);

// The spawned process will fail. In Node.js 10.11.0, it will fail silently. The
// test here is to make sure that the error information bubbles up to the
// calling process.
assert.ok(result.status, 'testFile swallowed its error');
4 changes: 2 additions & 2 deletions test/parallel/test-repl-tab-complete.js
Original file line number Diff line number Diff line change
Expand Up @@ -154,11 +154,11 @@ putIn.run([
' one:1',
'};'
]);
// See: https://github.com/nodejs/node/issues/21586
// testMe.complete('inner.o', getNoResultsFunction());
testMe.complete('inner.o', common.mustCall(function(error, data) {
assert.deepStrictEqual(data, works);
}));


putIn.run(['.clear']);

// currently does not work, but should not break, not the {
Expand Down