repl: catch promise errors during eval in completion

Fixes: #58903

Author: islandryu

lib/repl.js

@@ -43,6 +43,7 @@
 'use strict';
 
 const {
+  ArrayIsArray,
   ArrayPrototypeAt,
   ArrayPrototypeFilter,
   ArrayPrototypeFindLastIndex,
@@ -98,6 +99,7 @@ const {
 
 const {
   isProxy,
+  isPromise,
 } = require('internal/util/types');
 
 const { BuiltinModule } = require('internal/bootstrap/realm');
@@ -351,6 +353,7 @@ function REPLServer(prompt,
   this.allowBlockingCompletions = !!options.allowBlockingCompletions;
   this.useColors = !!options.useColors;
   this._domain = options.domain || domain.create();
+  this._completeDomain = domain.create();
   this.useGlobal = !!useGlobal;
   this.ignoreUndefined = !!ignoreUndefined;
   this.replMode = replMode || module.exports.REPL_MODE_SLOPPY;
@@ -668,6 +671,8 @@ function REPLServer(prompt,
   }
 
   self.eval = self._domain.bind(eval_);
+  self.completeEval = self._completeDomain.bind(eval_);
+  self._completeDomain.on('error', (err) => { });
 
   self._domain.on('error', function debugDomainError(e) {
     debug('domain error');
@@ -1541,7 +1546,7 @@ function complete(line, callback) {
     return includesProxiesOrGetters(
       completeTargetAst.body[0].expression,
       parsableCompleteTarget,
-      this.eval,
+      this.completeEval,
       this.context,
       (includes) => {
         if (includes) {
@@ -1558,8 +1563,10 @@ function complete(line, callback) {
 
         const memberGroups = [];
         const evalExpr = `try { ${expr} } catch {}`;
-        this.eval(evalExpr, this.context, getREPLResourceName(), (e, obj) => {
+        this.completeEval(evalExpr, this.context, getREPLResourceName(), (e, obj) => {
           try {
+            reclusiveCatchPromise(obj);
+
             let p;
             if ((typeof obj === 'object' && obj !== null) ||
               typeof obj === 'function') {
@@ -1800,6 +1807,7 @@ function includesProxiesOrGetters(expr, exprStr, evalFn, ctx, callback) {
   // is the property identifier/literal)
   if (expr.object.type === 'Identifier') {
     return evalFn(`try { ${expr.object.name} } catch {}`, ctx, getREPLResourceName(), (err, obj) => {
+      reclusiveCatchPromise(obj);
       if (err) {
         return callback(false);
       }
@@ -1815,6 +1823,7 @@ function includesProxiesOrGetters(expr, exprStr, evalFn, ctx, callback) {
 
         return evalFn(
           `try { ${exprStr} } catch {} `, ctx, getREPLResourceName(), (err, obj) => {
+            reclusiveCatchPromise(obj);
             if (err) {
               return callback(false);
             }
@@ -1877,6 +1886,7 @@ function includesProxiesOrGetters(expr, exprStr, evalFn, ctx, callback) {
       ctx,
       getREPLResourceName(),
       (err, evaledProp) => {
+        reclusiveCatchPromise(evaledProp);
         if (err) {
           return callback(false);
         }
@@ -1902,7 +1912,9 @@ function includesProxiesOrGetters(expr, exprStr, evalFn, ctx, callback) {
   function safeIsProxyAccess(obj, prop) {
     // Accessing `prop` may trigger a getter that throws, so we use try-catch to guard against it
     try {
-      return isProxy(obj[prop]);
+      const value = obj[prop];
+      reclusiveCatchPromise(value);
+      return isProxy(value);
     } catch {
       return false;
     }
@@ -1911,6 +1923,29 @@ function includesProxiesOrGetters(expr, exprStr, evalFn, ctx, callback) {
   return callback(false);
 }
 
+function reclusiveCatchPromise(obj, seen = new SafeWeakSet()) {
+  if (isPromise(obj)) {
+    return obj.catch(() => {});
+  } else if (ArrayIsArray(obj)) {
+    obj.forEach((item) => {
+      reclusiveCatchPromise(item, seen);
+    });
+  } else if (obj && typeof obj === 'object') {
+    if (seen.has(obj)) return;
+    seen.add(obj);
+
+    let props;
+    try {
+      props = ObjectGetOwnPropertyNames(obj);
+    } catch {
+      return;
+    }
+    props.forEach((key) => {
+      reclusiveCatchPromise(obj[key], seen);
+    });
+  }
+}
+
 REPLServer.prototype.completeOnEditorMode = (callback) => (err, results) => {
   if (err) return callback(err);
 

test/fixtures/repl-tab-completion-nested-repls.js

@@ -32,7 +32,7 @@ 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) {
+testMe._completeDomain.on('error', function(err) {
   throw err;
 });
 

test/parallel/test-repl-tab-complete-promises.js

@@ -0,0 +1,97 @@
+'use strict';
+
+const common = require('../common');
+const repl = require('repl');
+const ArrayStream = require('../common/arraystream');
+const assert = require('assert');
+
+const completionTests = [
+  { send: 'Promise.reject().' },
+  { send: 'let p = Promise.reject().' },
+  { send: 'Promise.resolve().' },
+  { send: 'Promise.resolve().then(() => {}).' },
+  { send: `async function f() {throw new Error('test');}; f().` },
+  { send: `async function f() {}; f().` },
+  { send: 'const foo = { bar: Promise.reject() }; foo.bar.' },
+  // Test for that reclusiveCatchPromise does not infinitely recurse
+  // see lib/repl.js:reclusiveCatchPromise
+  { send: 'const a = {}; a.self = a; a.self.' },
+  { run: `const foo = { get name() { return Promise.reject(); } };`,
+    send: `foo.name` },
+  { run: 'const baz = { get bar() { return ""; } }; const getPropText = () => Promise.reject();',
+    send: 'baz[getPropText()].',
+    completeError: true },
+  {
+    send: 'const quux = { bar: { return Promise.reject(); } }; const getPropText = () => "bar"; quux[getPropText()].',
+  },
+];
+
+(async function() {
+  await runReplCompleteTests(completionTests);
+})().then(common.mustCall());
+
+async function runReplCompleteTests(tests) {
+  const input = new ArrayStream();
+  const output = new ArrayStream();
+
+  const replServer = repl.start({
+    prompt: '',
+    input,
+    output: output,
+    allowBlockingCompletions: true,
+    terminal: true
+  });
+
+  for (const { send, run, completeError = false } of tests) {
+    if (run) {
+      await new Promise((resolve, reject) => {
+        replServer.eval(run, replServer.context, '', (err) => {
+          if (err) {
+            reject(err);
+          } else {
+            resolve();
+          }
+        });
+      });
+    }
+
+    const onError = (e) => {
+      assert.fail(`Unexpected error: ${e.message}`);
+    };
+
+    let completeErrorPromise = Promise.resolve();
+
+    if (completeError) {
+      completeErrorPromise = new Promise((resolve) => {
+        const handleError = () => {
+          replServer._completeDomain.removeListener('error', handleError);
+          resolve();
+        };
+        replServer._completeDomain.on('error', handleError);
+      });
+    } else {
+      replServer._completeDomain.on('error', onError);
+    }
+
+    await replServer.complete(
+      send,
+      common.mustCall((error, data) => {
+        assert.strictEqual(error, null);
+        assert.strictEqual(data.length, 2);
+        assert.strictEqual(typeof data[1], 'string');
+        assert.ok(send.includes(data[1]));
+      })
+    );
+
+    await completeErrorPromise;
+
+    if (!completeError) {
+      await new Promise((resolve) => {
+        setImmediate(() => {
+          replServer._completeDomain.removeListener('error', onError);
+          resolve();
+        });
+      });
+    }
+  }
+}

test/parallel/test-repl-tab-complete.js

@@ -44,7 +44,7 @@ function prepareREPL() {
   });
 
   // Some errors are passed to the domain, but do not callback
-  replServer._domain.on('error', assert.ifError);
+  replServer._completeDomain.on('error', assert.ifError);
 
   return { replServer, input };
 }