From 9fbd211aaf9a4a4983d645282fea27ab61b35085 Mon Sep 17 00:00:00 2001 From: islandryu Date: Sun, 13 Jul 2025 15:50:48 +0900 Subject: [PATCH 1/2] repl: handle errors from getters during completion --- lib/repl.js | 15 +++- .../test-repl-tab-complete-getter-error.js | 80 +++++++++++++++++++ 2 files changed, 92 insertions(+), 3 deletions(-) create mode 100644 test/parallel/test-repl-tab-complete-getter-error.js diff --git a/lib/repl.js b/lib/repl.js index d6a3b8a1d6fda5..784ab849b5f73a 100644 --- a/lib/repl.js +++ b/lib/repl.js @@ -1836,7 +1836,7 @@ function includesProxiesOrGetters(expr, exprStr, evalFn, ctx, callback) { if (astProp.type === 'Literal') { // We have something like `obj['foo'].x` where `x` is the literal - if (isProxy(obj[astProp.value])) { + if (safeIsProxyAccess(obj, astProp.value)) { return cb(true); } @@ -1854,7 +1854,7 @@ function includesProxiesOrGetters(expr, exprStr, evalFn, ctx, callback) { ) { // We have something like `obj.foo.x` where `foo` is the identifier - if (isProxy(obj[astProp.name])) { + if (safeIsProxyAccess(obj, astProp.name)) { return cb(true); } @@ -1881,7 +1881,7 @@ function includesProxiesOrGetters(expr, exprStr, evalFn, ctx, callback) { } if (typeof evaledProp === 'string') { - if (isProxy(obj[evaledProp])) { + if (safeIsProxyAccess(obj, evaledProp)) { return cb(true); } @@ -1898,6 +1898,15 @@ 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]); + } catch { + return false; + } + } + return callback(false); } diff --git a/test/parallel/test-repl-tab-complete-getter-error.js b/test/parallel/test-repl-tab-complete-getter-error.js new file mode 100644 index 00000000000000..8583e96feb9e45 --- /dev/null +++ b/test/parallel/test-repl-tab-complete-getter-error.js @@ -0,0 +1,80 @@ +'use strict'; + +const common = require('../common'); +const repl = require('repl'); +const ArrayStream = require('../common/arraystream'); +const assert = require('assert'); + +(async function() { + await runTest(); +})().then(common.mustCall()); + +async function runTest() { + const input = new ArrayStream(); + const output = new ArrayStream(); + + const replServer = repl.start({ + prompt: '', + input, + output: output, + allowBlockingCompletions: true, + terminal: true + }); + + replServer._domain.on('error', (e) => { + assert.fail(`Error in REPL domain: ${e}`); + }); + await new Promise((resolve, reject) => { + replServer.eval(` +class Person1 { + constructor(name) { this.name = name; } + get name() { throw new Error(); } set name(value) { this._name = value; } +}; +const foo = new Person1("Alice") + `, replServer.context, '', (err) => { + if (err) { + reject(err); + } else { + resolve(); + } + }); + }); + replServer.complete( + 'foo.name.', + common.mustCall((error, data) => { + assert.strictEqual(error, null); + assert.strictEqual(data.length, 2); + assert.strictEqual(data[1], 'foo.name.'); + }) + ); + + replServer.complete( + 'foo["name"].', + common.mustCall((error, data) => { + assert.strictEqual(error, null); + assert.strictEqual(data.length, 2); + assert.strictEqual(data[1], 'foo["name"].'); + }) + ); + await new Promise((resolve, reject) => { + replServer.eval(` +function getNameText() { + return "name"; +} + `, replServer.context, '', (err) => { + if (err) { + reject(err); + } else { + resolve(); + } + }); + }); + replServer.complete( + 'foo[getNameText()].', + common.mustCall((error, data) => { + assert.strictEqual(error, null); + assert.strictEqual(data.length, 2); + assert.strictEqual(data[1], 'foo[getNameText()].'); + }) + ); +} From 11222b56ca5d58afeea547e3c7bd6e3ecbf41392 Mon Sep 17 00:00:00 2001 From: islandryu Date: Sun, 13 Jul 2025 20:38:42 +0900 Subject: [PATCH 2/2] fix test --- .../test-repl-tab-complete-getter-error.js | 55 +++++-------------- 1 file changed, 13 insertions(+), 42 deletions(-) diff --git a/test/parallel/test-repl-tab-complete-getter-error.js b/test/parallel/test-repl-tab-complete-getter-error.js index 8583e96feb9e45..e2e36b85c58baa 100644 --- a/test/parallel/test-repl-tab-complete-getter-error.js +++ b/test/parallel/test-repl-tab-complete-getter-error.js @@ -24,14 +24,12 @@ async function runTest() { replServer._domain.on('error', (e) => { assert.fail(`Error in REPL domain: ${e}`); }); + await new Promise((resolve, reject) => { replServer.eval(` -class Person1 { - constructor(name) { this.name = name; } - get name() { throw new Error(); } set name(value) { this._name = value; } -}; -const foo = new Person1("Alice") - `, replServer.context, '', (err) => { + const getNameText = () => "name"; + const foo = { get name() { throw new Error(); } }; + `, replServer.context, '', (err) => { if (err) { reject(err); } else { @@ -39,42 +37,15 @@ const foo = new Person1("Alice") } }); }); - replServer.complete( - 'foo.name.', - common.mustCall((error, data) => { - assert.strictEqual(error, null); - assert.strictEqual(data.length, 2); - assert.strictEqual(data[1], 'foo.name.'); - }) - ); - replServer.complete( - 'foo["name"].', - common.mustCall((error, data) => { - assert.strictEqual(error, null); - assert.strictEqual(data.length, 2); - assert.strictEqual(data[1], 'foo["name"].'); - }) - ); - await new Promise((resolve, reject) => { - replServer.eval(` -function getNameText() { - return "name"; -} - `, replServer.context, '', (err) => { - if (err) { - reject(err); - } else { - resolve(); - } - }); + ['foo.name.', 'foo["name"].', 'foo[getNameText()].'].forEach((test) => { + replServer.complete( + test, + common.mustCall((error, data) => { + assert.strictEqual(error, null); + assert.strictEqual(data.length, 2); + assert.strictEqual(data[1], test); + }) + ); }); - replServer.complete( - 'foo[getNameText()].', - common.mustCall((error, data) => { - assert.strictEqual(error, null); - assert.strictEqual(data.length, 2); - assert.strictEqual(data[1], 'foo[getNameText()].'); - }) - ); }