repl: limit line processing to one at a time

doc/api/repl.md

@@ -604,7 +604,10 @@ 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` 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
@@ -773,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

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,8 +842,19 @@ function REPLServer(prompt,
     self.displayPrompt();
   });
 
-  self.on('line', function onLine(cmd) {
-    debug('line %j', cmd);
+  self.processing = false;
+  self.queuedLines = [];
+  self.skipQueue = false;
+
+  function _onLine(cmd, queued = false) {
+    if (self.processing && !self.skipQueue) {
+      debug('queuing line %j', cmd);
+      ArrayPrototypePush(self.queuedLines, cmd);
+      return;
+    }
+    self.skipQueue = false;
+    self.processing = true;
+    debug((queued ? 'queued ' : '') + '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 {
+        do {
+          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();
+            break;
+          }
 
-      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();
+              break;
+            }
+            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();
+          }
+        } while (0);
+      } finally {
+        if (self.queuedLines.length) {
+          self.skipQueue = true;
+          _onLine(ArrayPrototypeShift(self.queuedLines), true);
+        } 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');

test/parallel/test-repl-autolibs.js

@@ -66,5 +66,5 @@ function test2() {
   common.allowGlobals(val);
   assert(!gotWrite);
   putIn.run(['url']);
-  assert(gotWrite);
+  process.nextTick(() => assert(gotWrite));
 }

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();
     })
   };
 

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();
     })
   };
 

test/parallel/test-repl-line-queue.js

@@ -0,0 +1,130 @@
+'use strict';
+// Flags: --expose-internals --experimental-repl-await
+
+require('../common');
+const ArrayStream = require('../common/arraystream');
+
+const assert = require('assert');
+
+function* expectedLines(lines) {
+  yield* line;
+}
+
+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',
+  ],
+  ['queuing line %j', 'x;'],
+  ['finish', null, undefined],
+  ['queued 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],
+  ['queuing line %j', 'z + 1'],
+  ['queued 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,
+});
+
+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.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();