From 5903929628e5aec1f99fa55099c92c7dfeba7399 Mon Sep 17 00:00:00 2001
From: Rich Trott <rtrott@gmail.com>
Date: Fri, 21 Sep 2018 11:55:55 -0700
Subject: [PATCH 1/2] repl: do not swallow errors in nested REPLs

For tab completion, a REPLServer instance will sometimes create another
REPLServer instance. If a callback is sent to the `.complete()` function
and that callback throws an error, it will be swallowed by the nested
REPLs domain. Re-throw the error so that processes don't silently exit
without any indication of an error (including a status code).

Fixes: https://github.com/nodejs/node/issues/21586
---
 lib/repl.js                                   |  1 +
 .../repl-tab-completion-nested-repls.js       | 50 +++++++++++++++++++
 .../test-repl-tab-complete-nested-repls.js    | 21 ++++++++
 test/parallel/test-repl-tab-complete.js       |  4 +-
 4 files changed, 74 insertions(+), 2 deletions(-)
 create mode 100644 test/fixtures/repl-tab-completion-nested-repls.js
 create mode 100644 test/parallel/test-repl-tab-complete-nested-repls.js

diff --git a/lib/repl.js b/lib/repl.js
index 3f3d33175c57f2..72328a1d0f7e04 100644
--- a/lib/repl.js
+++ b/lib/repl.js
@@ -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);
     }
   }
diff --git a/test/fixtures/repl-tab-completion-nested-repls.js b/test/fixtures/repl-tab-completion-nested-repls.js
new file mode 100644
index 00000000000000..832734a22a5024
--- /dev/null
+++ b/test/fixtures/repl-tab-completion-nested-repls.js
@@ -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'); });
diff --git a/test/parallel/test-repl-tab-complete-nested-repls.js b/test/parallel/test-repl-tab-complete-nested-repls.js
new file mode 100644
index 00000000000000..36547e8d9fb5be
--- /dev/null
+++ b/test/parallel/test-repl-tab-complete-nested-repls.js
@@ -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');
diff --git a/test/parallel/test-repl-tab-complete.js b/test/parallel/test-repl-tab-complete.js
index 784bcade77e6fa..45915f94c93819 100644
--- a/test/parallel/test-repl-tab-complete.js
+++ b/test/parallel/test-repl-tab-complete.js
@@ -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 {

From cac968ab90e068328cdecbc09eda9df39c9bb9cb Mon Sep 17 00:00:00 2001
From: Rich Trott <rtrott@gmail.com>
Date: Tue, 2 Oct 2018 03:53:36 -0700
Subject: [PATCH 2/2] squash! remove extra newline

---
 test/parallel/test-repl-tab-complete.js | 1 -
 1 file changed, 1 deletion(-)

diff --git a/test/parallel/test-repl-tab-complete.js b/test/parallel/test-repl-tab-complete.js
index 45915f94c93819..378072ecb8c3e7 100644
--- a/test/parallel/test-repl-tab-complete.js
+++ b/test/parallel/test-repl-tab-complete.js
@@ -158,7 +158,6 @@ 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 {