From 7ad5ca4de6cc9fb50cc4240f4eb6441c1e01f296 Mon Sep 17 00:00:00 2001 From: ejose19 <8742215+ejose19@users.noreply.github.com> Date: Wed, 14 Jul 2021 19:16:38 -0300 Subject: [PATCH 01/10] repl: limit line processing to one at a time --- doc/api/repl.md | 4 +- lib/repl.js | 121 ++++++++++++++++---------- test/parallel/test-repl-autolibs.js | 2 +- test/parallel/test-repl-empty.js | 3 +- test/parallel/test-repl-eval.js | 3 +- test/parallel/test-repl-line-queue.js | 26 ++++++ 6 files changed, 110 insertions(+), 49 deletions(-) create mode 100644 test/parallel/test-repl-line-queue.js diff --git a/doc/api/repl.md b/doc/api/repl.md index 409f70b2b39ab8..5d215db39d037e 100644 --- a/doc/api/repl.md +++ b/doc/api/repl.md @@ -604,7 +604,9 @@ changes: * `eval` {Function} The function to be used when evaluating each given line of input. **Default:** an async wrapper for the JavaScript `eval()` function. An `eval` function can error with `repl.Recoverable` to indicate - the input was incomplete and prompt for additional lines. + the input was incomplete and prompt for additional lines. If a custom + `eval` function is provided, `callback` must be invoked to allow processing + next command. * `useColors` {boolean} If `true`, specifies that the default `writer` function should include ANSI color styling to REPL output. If a custom `writer` function is provided then this has no effect. **Default:** checking diff --git a/lib/repl.js b/lib/repl.js index 4ee8e24d47588c..12e3db71ec1f58 100644 --- a/lib/repl.js +++ b/lib/repl.js @@ -635,6 +635,7 @@ function REPLServer(prompt, self._domain.on('error', function debugDomainError(e) { debug('domain error'); let errStack = ''; + self.processing = false; if (typeof e === 'object' && e !== null) { overrideStackTrace.set(e, (error, stackFrames) => { @@ -808,6 +809,8 @@ function REPLServer(prompt, let sawCtrlD = false; const prioritizedSigintQueue = new SafeSet(); self.on('SIGINT', function onSigInt() { + self.processing = false; + if (prioritizedSigintQueue.size > 0) { for (const task of prioritizedSigintQueue) { task(); @@ -839,7 +842,18 @@ function REPLServer(prompt, self.displayPrompt(); }); - self.on('line', function onLine(cmd) { + self.processing = false; + self.queuedLines = []; + self.skipQueue = false; + + function _onLine(cmd) { + if (self.processing && !self.skipQueue) { + debug('queued line %j', cmd); + ArrayPrototypePush(self.queuedLines, cmd); + return; + } + self.skipQueue = false; + self.processing = true; debug('line %j', cmd); cmd = cmd || ''; sawSIGINT = false; @@ -857,6 +871,7 @@ function REPLServer(prompt, self.cursor = prefix.length; } ReflectApply(_memory, self, [cmd]); + self.processing = false; return; } @@ -873,6 +888,7 @@ function REPLServer(prompt, const keyword = matches && matches[1]; const rest = matches && matches[2]; if (ReflectApply(_parseREPLKeyword, self, [keyword, rest]) === true) { + self.processing = false; return; } if (!self[kBufferedCommandSymbol]) { @@ -889,56 +905,70 @@ function REPLServer(prompt, self.eval(evalCmd, self.context, getREPLResourceName(), finish); function finish(e, ret) { - debug('finish', e, ret); - ReflectApply(_memory, self, [cmd]); + try { + (() => { + debug('finish', e, ret); + ReflectApply(_memory, self, [cmd]); + + if (e && !self[kBufferedCommandSymbol] && + StringPrototypeStartsWith(StringPrototypeTrim(cmd), 'npm ')) { + self.output.write('npm should be run outside of the ' + + 'Node.js REPL, in your normal shell.\n' + + '(Press Ctrl+D to exit.)\n'); + self.displayPrompt(); + return; + } - if (e && !self[kBufferedCommandSymbol] && - StringPrototypeStartsWith(StringPrototypeTrim(cmd), 'npm ')) { - self.output.write('npm should be run outside of the ' + - 'Node.js REPL, in your normal shell.\n' + - '(Press Ctrl+D to exit.)\n'); - self.displayPrompt(); - return; - } + // If error was SyntaxError and not JSON.parse error + if (e) { + if (e instanceof Recoverable && !sawCtrlD) { + // Start buffering data like that: + // { + // ... x: 1 + // ... } + self[kBufferedCommandSymbol] += cmd + '\n'; + self.displayPrompt(); + return; + } + self._domain.emit('error', e.err || e); + } - // If error was SyntaxError and not JSON.parse error - if (e) { - if (e instanceof Recoverable && !sawCtrlD) { - // Start buffering data like that: - // { - // ... x: 1 - // ... } - self[kBufferedCommandSymbol] += cmd + '\n'; - self.displayPrompt(); - return; - } - self._domain.emit('error', e.err || e); - } + // Clear buffer if no SyntaxErrors + self.clearBufferedCommand(); + sawCtrlD = false; + + // If we got any output - print it (if no error) + if (!e && + // When an invalid REPL command is used, error message is printed + // immediately. We don't have to print anything else. + // So, only when the second argument to this function is there, + // print it. + arguments.length === 2 && + (!self.ignoreUndefined || ret !== undefined)) { + if (!self.underscoreAssigned) { + self.last = ret; + } + self.output.write(self.writer(ret) + '\n'); + } - // Clear buffer if no SyntaxErrors - self.clearBufferedCommand(); - sawCtrlD = false; - - // If we got any output - print it (if no error) - if (!e && - // When an invalid REPL command is used, error message is printed - // immediately. We don't have to print anything else. So, only when - // the second argument to this function is there, print it. - arguments.length === 2 && - (!self.ignoreUndefined || ret !== undefined)) { - if (!self.underscoreAssigned) { - self.last = ret; + // Display prompt again (unless we already did by emitting the 'error' + // event on the domain instance). + if (!e) { + self.displayPrompt(); + } + })(); + } finally { + if (self.queuedLines.length) { + self.skipQueue = true; + _onLine(ArrayPrototypeShift(self.queuedLines)); + } else { + self.processing = false; } - self.output.write(self.writer(ret) + '\n'); - } - - // Display prompt again (unless we already did by emitting the 'error' - // event on the domain instance). - if (!e) { - self.displayPrompt(); } } - }); + } + + self.on('line', _onLine); self.on('SIGCONT', function onSigCont() { if (self.editorMode) { @@ -1752,6 +1782,7 @@ function defineDefaultCommands(repl) { if (stats && stats.isFile()) { _turnOnEditorMode(this); const data = fs.readFileSync(file, 'utf8'); + repl.skipQueue = true; this.write(data); _turnOffEditorMode(this); this.write('\n'); diff --git a/test/parallel/test-repl-autolibs.js b/test/parallel/test-repl-autolibs.js index 5cf3b1497221d0..e6ae4f251a4d8c 100644 --- a/test/parallel/test-repl-autolibs.js +++ b/test/parallel/test-repl-autolibs.js @@ -42,7 +42,7 @@ function test1() { `${util.inspect(require('fs'), null, 2, false)}\n`); // Globally added lib matches required lib assert.strictEqual(global.fs, require('fs')); - test2(); + process.nextTick(test2); } }; assert(!gotWrite); diff --git a/test/parallel/test-repl-empty.js b/test/parallel/test-repl-empty.js index 44281f117f0bba..75f42ff36a5c05 100644 --- a/test/parallel/test-repl-empty.js +++ b/test/parallel/test-repl-empty.js @@ -7,10 +7,11 @@ const repl = require('repl'); let evalCalledWithExpectedArgs = false; const options = { - eval: common.mustCall((cmd, context) => { + eval: common.mustCall((cmd, _context, _file, cb) => { // Assertions here will not cause the test to exit with an error code // so set a boolean that is checked later instead. evalCalledWithExpectedArgs = (cmd === '\n'); + cb(); }) }; diff --git a/test/parallel/test-repl-eval.js b/test/parallel/test-repl-eval.js index d775423fb74a52..b120286c65edf8 100644 --- a/test/parallel/test-repl-eval.js +++ b/test/parallel/test-repl-eval.js @@ -7,11 +7,12 @@ const repl = require('repl'); let evalCalledWithExpectedArgs = false; const options = { - eval: common.mustCall((cmd, context) => { + eval: common.mustCall((cmd, context, _file, cb) => { // Assertions here will not cause the test to exit with an error code // so set a boolean that is checked later instead. evalCalledWithExpectedArgs = (cmd === 'function f() {}\n' && context.foo === 'bar'); + cb(); }) }; diff --git a/test/parallel/test-repl-line-queue.js b/test/parallel/test-repl-line-queue.js new file mode 100644 index 00000000000000..e5ad4d81800500 --- /dev/null +++ b/test/parallel/test-repl-line-queue.js @@ -0,0 +1,26 @@ +'use strict'; +require('../common'); +const ArrayStream = require('../common/arraystream'); + +const assert = require('assert'); +const repl = require('repl'); + +// Flags: --expose-internals --experimental-repl-await + +const putIn = new ArrayStream(); +repl.start({ + input: putIn, + output: putIn, + useGlobal: false +}); + +let expectedIndex = -1; +const expected = ['undefined', '> ', '1', '> ']; + +putIn.write = function(data) { + assert.strict(data, expected[expectedIndex += 1]); +}; + +putIn.run([ + 'const x = await new Promise((r) => setTimeout(() => r(1), 500));\nx;', +]); From b77043c07d567ee7bbd02d096fab3ebe6eda7f0b Mon Sep 17 00:00:00 2001 From: ejose19 <8742215+ejose19@users.noreply.github.com> Date: Thu, 15 Jul 2021 13:20:09 -0300 Subject: [PATCH 02/10] repl: change IIFE to single iteration loop --- lib/repl.js | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/lib/repl.js b/lib/repl.js index 12e3db71ec1f58..9a9da04f8fe5ee 100644 --- a/lib/repl.js +++ b/lib/repl.js @@ -906,7 +906,7 @@ function REPLServer(prompt, function finish(e, ret) { try { - (() => { + do { debug('finish', e, ret); ReflectApply(_memory, self, [cmd]); @@ -916,7 +916,7 @@ function REPLServer(prompt, 'Node.js REPL, in your normal shell.\n' + '(Press Ctrl+D to exit.)\n'); self.displayPrompt(); - return; + break; } // If error was SyntaxError and not JSON.parse error @@ -928,7 +928,7 @@ function REPLServer(prompt, // ... } self[kBufferedCommandSymbol] += cmd + '\n'; self.displayPrompt(); - return; + break; } self._domain.emit('error', e.err || e); } @@ -956,7 +956,7 @@ function REPLServer(prompt, if (!e) { self.displayPrompt(); } - })(); + } while (0); } finally { if (self.queuedLines.length) { self.skipQueue = true; From 65ab5828403461e9b11b60deb36a872ebda8eda5 Mon Sep 17 00:00:00 2001 From: ejose19 <8742215+ejose19@users.noreply.github.com> Date: Thu, 15 Jul 2021 13:33:25 -0300 Subject: [PATCH 03/10] repl: adjust autolibs test --- test/parallel/test-repl-autolibs.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/parallel/test-repl-autolibs.js b/test/parallel/test-repl-autolibs.js index e6ae4f251a4d8c..c84ecd1ad0cb99 100644 --- a/test/parallel/test-repl-autolibs.js +++ b/test/parallel/test-repl-autolibs.js @@ -42,7 +42,7 @@ function test1() { `${util.inspect(require('fs'), null, 2, false)}\n`); // Globally added lib matches required lib assert.strictEqual(global.fs, require('fs')); - process.nextTick(test2); + test2(); } }; assert(!gotWrite); @@ -66,5 +66,5 @@ function test2() { common.allowGlobals(val); assert(!gotWrite); putIn.run(['url']); - assert(gotWrite); + process.nextTick(() => assert(gotWrite)); } From 4074c385f65c0ba16fad7720b45673dc53ffd355 Mon Sep 17 00:00:00 2001 From: ejose19 <8742215+ejose19@users.noreply.github.com> Date: Thu, 15 Jul 2021 15:03:31 -0300 Subject: [PATCH 04/10] repl: fix line-queue tests --- test/parallel/test-repl-line-queue.js | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/test/parallel/test-repl-line-queue.js b/test/parallel/test-repl-line-queue.js index e5ad4d81800500..dd793a0b54eddd 100644 --- a/test/parallel/test-repl-line-queue.js +++ b/test/parallel/test-repl-line-queue.js @@ -1,11 +1,18 @@ 'use strict'; +// Flags: --expose-internals --experimental-repl-await + require('../common'); const ArrayStream = require('../common/arraystream'); const assert = require('assert'); const repl = require('repl'); -// Flags: --expose-internals --experimental-repl-await +function* expectedLines(lines) { + for (const line of lines) { + yield line; + } + throw new Error('Requested more lines than expected'); +} const putIn = new ArrayStream(); repl.start({ @@ -14,11 +21,9 @@ repl.start({ useGlobal: false }); -let expectedIndex = -1; -const expected = ['undefined', '> ', '1', '> ']; - +const expectedOutput = expectedLines(['undefined\n', '> ', '1\n', '> ']); putIn.write = function(data) { - assert.strict(data, expected[expectedIndex += 1]); + assert.strictEqual(data, expectedOutput.next().value); }; putIn.run([ From 569eb6848959180cfeacd020c9b6a4f2eff051bf Mon Sep 17 00:00:00 2001 From: ejose19 <8742215+ejose19@users.noreply.github.com> Date: Thu, 15 Jul 2021 18:26:40 -0300 Subject: [PATCH 05/10] repl: improve line queue tests --- test/parallel/test-repl-line-queue.js | 119 ++++++++++++++++++++++++-- 1 file changed, 110 insertions(+), 9 deletions(-) diff --git a/test/parallel/test-repl-line-queue.js b/test/parallel/test-repl-line-queue.js index dd793a0b54eddd..7d5b28e1e442f7 100644 --- a/test/parallel/test-repl-line-queue.js +++ b/test/parallel/test-repl-line-queue.js @@ -5,27 +5,128 @@ require('../common'); const ArrayStream = require('../common/arraystream'); const assert = require('assert'); -const repl = require('repl'); function* expectedLines(lines) { for (const line of lines) { yield line; } - throw new Error('Requested more lines than expected'); } +const expectedDebug = expectedLines([ + [ + 'line %j', + 'const x = await new Promise((r) => setTimeout(() => r(1), 500));', + ], + [ + 'eval %j', + 'const x = await new Promise((r) => setTimeout(() => r(1), 500));\n', + ], + ['queued line %j', 'x;'], + ['finish', null, undefined], + ['line %j', 'x;'], + ['eval %j', 'x;\n'], + ['finish', null, 1], + ['line %j', 'const y = 3;'], + ['eval %j', 'const y = 3;\n'], + ['finish', null, undefined], + ['line %j', 'x + y;'], + ['eval %j', 'x + y;\n'], + ['finish', null, 4], + ['line %j', 'const z = 4;'], + ['eval %j', 'const z = 4;\n'], + ['finish', null, undefined], + ['queued line %j', 'z + 1'], + ['line %j', 'z + 1'], + ['eval %j', 'z + 1\n'], + ['finish', null, 5], +]); + +let calledDebug = false; +require('internal/util/debuglog').debuglog = () => { + return (...args) => { + calledDebug = true; + assert.deepStrictEqual(args, expectedDebug.next().value); + }; +}; + +// Import `repl` after replacing original `debuglog` +const repl = require('repl'); + const putIn = new ArrayStream(); repl.start({ input: putIn, output: putIn, - useGlobal: false + useGlobal: false, }); -const expectedOutput = expectedLines(['undefined\n', '> ', '1\n', '> ']); -putIn.write = function(data) { +const expectedOutput = expectedLines([ + 'undefined\n', + '> ', + '1\n', + '> ', + 'undefined\n', + '> ', + '4\n', + '> ', + 'undefined\n', + '> ', + '5\n', + '> ', +]); + +let calledOutput = false; +function writeCallback(data) { + calledOutput = true; assert.strictEqual(data, expectedOutput.next().value); -}; +} -putIn.run([ - 'const x = await new Promise((r) => setTimeout(() => r(1), 500));\nx;', -]); +putIn.write = writeCallback; + +function clearCalled() { + calledDebug = false; + calledOutput = false; +} + +// Lines sent after an async command that hasn't finished +// in the same write are enqueued +function test1() { + putIn.run([ + 'const x = await new Promise((r) => setTimeout(() => r(1), 500));\nx;', + ]); + assert(calledDebug); + + setTimeout(() => { + assert(calledOutput); + clearCalled(); + + test2(); + }, 1000); +} + +// Lines sent after a sync command in the same write are not enqueued +function test2() { + putIn.run(['const y = 3;\nx + y;']); + + assert(calledDebug); + assert(calledOutput); + clearCalled(); + + test3(); +} + +// Lines sent during an output write event of a previous command +// are enqueued +function test3() { + putIn.write = (data) => { + writeCallback(data); + putIn.write = writeCallback; + putIn.run(['z + 1']); + }; + + putIn.run(['const z = 4;']); + + assert(calledDebug); + assert(calledOutput); +} + +test1(); From 10dc12a8e3dbb7b7007919eda380f20377f5a59c Mon Sep 17 00:00:00 2001 From: ejose19 <8742215+ejose19@users.noreply.github.com> Date: Sat, 17 Jul 2021 14:55:57 -0300 Subject: [PATCH 06/10] repl: improve queue debug messages --- lib/repl.js | 8 ++++---- test/parallel/test-repl-line-queue.js | 6 +++--- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/lib/repl.js b/lib/repl.js index 9a9da04f8fe5ee..b052c6c1ccd1a3 100644 --- a/lib/repl.js +++ b/lib/repl.js @@ -846,15 +846,15 @@ function REPLServer(prompt, self.queuedLines = []; self.skipQueue = false; - function _onLine(cmd) { + function _onLine(cmd, queued = false) { if (self.processing && !self.skipQueue) { - debug('queued line %j', cmd); + debug('queuing line %j', cmd); ArrayPrototypePush(self.queuedLines, cmd); return; } self.skipQueue = false; self.processing = true; - debug('line %j', cmd); + debug((queued ? 'queued ' : '') + 'line %j', cmd) cmd = cmd || ''; sawSIGINT = false; @@ -960,7 +960,7 @@ function REPLServer(prompt, } finally { if (self.queuedLines.length) { self.skipQueue = true; - _onLine(ArrayPrototypeShift(self.queuedLines)); + _onLine(ArrayPrototypeShift(self.queuedLines), true); } else { self.processing = false; } diff --git a/test/parallel/test-repl-line-queue.js b/test/parallel/test-repl-line-queue.js index 7d5b28e1e442f7..e74d3649cf9beb 100644 --- a/test/parallel/test-repl-line-queue.js +++ b/test/parallel/test-repl-line-queue.js @@ -21,9 +21,9 @@ const expectedDebug = expectedLines([ 'eval %j', 'const x = await new Promise((r) => setTimeout(() => r(1), 500));\n', ], - ['queued line %j', 'x;'], + ['queuing line %j', 'x;'], ['finish', null, undefined], - ['line %j', 'x;'], + ['queued line %j', 'x;'], ['eval %j', 'x;\n'], ['finish', null, 1], ['line %j', 'const y = 3;'], @@ -35,8 +35,8 @@ const expectedDebug = expectedLines([ ['line %j', 'const z = 4;'], ['eval %j', 'const z = 4;\n'], ['finish', null, undefined], + ['queuing line %j', 'z + 1'], ['queued line %j', 'z + 1'], - ['line %j', 'z + 1'], ['eval %j', 'z + 1\n'], ['finish', null, 5], ]); From 6b11537b0a293dd88265ced59dd7162f17d96488 Mon Sep 17 00:00:00 2001 From: ejose19 <8742215+ejose19@users.noreply.github.com> Date: Sat, 17 Jul 2021 15:03:10 -0300 Subject: [PATCH 07/10] repl: improve eval doc entry --- doc/api/repl.md | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/doc/api/repl.md b/doc/api/repl.md index 5d215db39d037e..fd538c7d110824 100644 --- a/doc/api/repl.md +++ b/doc/api/repl.md @@ -605,8 +605,9 @@ changes: of input. **Default:** an async wrapper for the JavaScript `eval()` function. An `eval` function can error with `repl.Recoverable` to indicate the input was incomplete and prompt for additional lines. If a custom - `eval` function is provided, `callback` must be invoked to allow processing - next command. + `eval` function is provided, `callback` argument must be invoked + to allow processing next command (See [`repl.customEval`][] for + arguments passed to `eval`). * `useColors` {boolean} If `true`, specifies that the default `writer` function should include ANSI color styling to REPL output. If a custom `writer` function is provided then this has no effect. **Default:** checking @@ -775,6 +776,7 @@ For an example of running a REPL instance over [`curl(1)`][], see: [`process.setUncaughtExceptionCaptureCallback()`]: process.md#processsetuncaughtexceptioncapturecallbackfn [`readline.InterfaceCompleter`]: readline.md#use-of-the-completer-function [`repl.ReplServer`]: #class-replserver +[`repl.customEval`]: #custom-evaluation-functions [`repl.start()`]: #replstartoptions [`reverse-i-search`]: #reverse-i-search [`util.inspect()`]: util.md#utilinspectobject-options From 77271001f15fa39a158c9c0a2ada03e13aa7395d Mon Sep 17 00:00:00 2001 From: ejose19 <8742215+ejose19@users.noreply.github.com> Date: Sat, 17 Jul 2021 15:12:14 -0300 Subject: [PATCH 08/10] repl: apply eslint fixes --- lib/repl.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/repl.js b/lib/repl.js index b052c6c1ccd1a3..07643d98d7f937 100644 --- a/lib/repl.js +++ b/lib/repl.js @@ -854,7 +854,7 @@ function REPLServer(prompt, } self.skipQueue = false; self.processing = true; - debug((queued ? 'queued ' : '') + 'line %j', cmd) + debug((queued ? 'queued ' : '') + 'line %j', cmd); cmd = cmd || ''; sawSIGINT = false; From 4cb521533f3cffd139c28027c08a0fa7f7ca201b Mon Sep 17 00:00:00 2001 From: ejose19 <8742215+ejose19@users.noreply.github.com> Date: Mon, 25 Oct 2021 12:28:36 -0300 Subject: [PATCH 09/10] Update test/parallel/test-repl-line-queue.js Co-authored-by: Antoine du Hamel --- test/parallel/test-repl-line-queue.js | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/test/parallel/test-repl-line-queue.js b/test/parallel/test-repl-line-queue.js index e74d3649cf9beb..54901b2411de7e 100644 --- a/test/parallel/test-repl-line-queue.js +++ b/test/parallel/test-repl-line-queue.js @@ -7,9 +7,7 @@ const ArrayStream = require('../common/arraystream'); const assert = require('assert'); function* expectedLines(lines) { - for (const line of lines) { - yield line; - } + yield* line; } const expectedDebug = expectedLines([ From d2dcacd6dea34105ffb786e9118799738fd8f340 Mon Sep 17 00:00:00 2001 From: ejose19 <8742215+ejose19@users.noreply.github.com> Date: Mon, 25 Oct 2021 13:14:35 -0300 Subject: [PATCH 10/10] Update test/parallel/test-repl-line-queue.js Co-authored-by: Antoine du Hamel --- test/parallel/test-repl-line-queue.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/parallel/test-repl-line-queue.js b/test/parallel/test-repl-line-queue.js index 54901b2411de7e..b609317d82ea65 100644 --- a/test/parallel/test-repl-line-queue.js +++ b/test/parallel/test-repl-line-queue.js @@ -7,7 +7,7 @@ const ArrayStream = require('../common/arraystream'); const assert = require('assert'); function* expectedLines(lines) { - yield* line; + yield* lines; } const expectedDebug = expectedLines([