Repository: nodejs/node
Pull request: util: safely inspect getter errors whose message throws

Fixes: #60683

- Handle errors whose getters throw
- Include the full error (stack, message, etc.) respecting inspect options
- Fix indentation when getter throws

Previously, when a getter threw, only the message was output. Now `inspect` correctly outputs the full error according to the specified options.

<!--
Before submitting a pull request, please read:

- the CONTRIBUTING guide at https://github.com/nodejs/node/blob/HEAD/CONTRIBUTING.md
- the commit message formatting guidelines at
  https://github.com/nodejs/node/blob/HEAD/doc/contributing/pull-requests.md#commit-message-guidelines

For code changes:
1. Include tests for any bug fixes or new features.
2. Update documentation if relevant.
3. Ensure that `make -j4 test` (UNIX), or `vcbuild test` (Windows) passes.

If you believe this PR should be highlighted in the Node.js CHANGELOG
please add the `notable-change` label.

Developer's Certificate of Origin 1.1

By making a contribution to this project, I certify that:

(a) The contribution was created in whole or in part by me and I
    have the right to submit it under the open source license
    indicated in the file; or

(b) The contribution is based upon previous work that, to the best
    of my knowledge, is covered under an appropriate open source
    license and I have the right under that license to submit that
    work with modifications, whether created in whole or in part
    by me, under the same open source license (unless I am
    permitted to submit under a different license), as indicated
    in the file; or

(c) The contribution was provided directly to me by some other
    person who certified (a), (b) or (c) and I have not modified
    it.

(d) I understand and agree that this project and the contribution
    are public and that a record of the contribution (including all
    personal information I submit with it, including my sign-off) is
    maintained indefinitely and may be redistributed consistent with
    this project or the open source license(s) involved.
-->


Changed files:
filename: lib/internal/util/inspect.js
patch: @@ -2544,9 +2544,9 @@ function formatProperty(ctx, value, recurseTimes, key, type, desc,
     if (ctx.getters && (ctx.getters === true ||
           (ctx.getters === 'get' && desc.set === undefined) ||
           (ctx.getters === 'set' && desc.set !== undefined))) {
+      ctx.indentationLvl += 2;
       try {
         const tmp = FunctionPrototypeCall(desc.get, original);
-        ctx.indentationLvl += 2;
         if (tmp === null) {
           str = `${s(`[${label}:`, sp)} ${s('null', 'null')}${s(']', sp)}`;
         } else if (typeof tmp === 'object') {
@@ -2555,11 +2555,11 @@ function formatProperty(ctx, value, recurseTimes, key, type, desc,
           const primitive = formatPrimitive(s, tmp, ctx);
           str = `${s(`[${label}:`, sp)} ${primitive}${s(']', sp)}`;
         }
-        ctx.indentationLvl -= 2;
       } catch (err) {
-        const message = `<Inspection threw (${err.message})>`;
+        const message = `<Inspection threw (${formatValue(ctx, err, recurseTimes)})>`;
         str = `${s(`[${label}:`, sp)} ${message}${s(']', sp)}`;
       }
+      ctx.indentationLvl -= 2;
     } else {
       str = ctx.stylize(`[${label}]`, sp);
     }

filename: test/parallel/test-util-inspect.js
patch: @@ -690,7 +690,9 @@ assert.strictEqual(util.inspect(-5e-324), '-5e-324');
 
 {
   const tmp = Error.stackTraceLimit;
-  Error.stackTraceLimit = 0;
+  // Force stackTraceLimit = 0 for this test, but make it non-enumerable
+  // so it doesn't appear in inspect() output when inspecting Error in other tests.
+  Object.defineProperty(Error, 'stackTraceLimit', { value: 0, enumerable: false });
   const err = new Error('foo');
   const err2 = new Error('foo\nbar');
   assert.strictEqual(util.inspect(err, { compact: true }), '[Error: foo]');
@@ -2527,14 +2529,10 @@ assert.strictEqual(
     set foo(val) { foo = val; },
     get inc() { return ++foo; }
   };
-  const thrower = { get foo() { throw new Error('Oops'); } };
   assert.strictEqual(
     inspect(get, { getters: true, colors: true }),
     '{ foo: \u001b[36m[Getter:\u001b[39m ' +
       '\u001b[33m1\u001b[39m\u001b[36m]\u001b[39m }');
-  assert.strictEqual(
-    inspect(thrower, { getters: true }),
-    '{ foo: [Getter: <Inspection threw (Oops)>] }');
   assert.strictEqual(
     inspect(getset, { getters: true }),
     '{ foo: [Getter/Setter: 1], inc: [Getter: 2] }');
@@ -2551,6 +2549,239 @@ assert.strictEqual(
       "'foobar', { x: 1 } },\n  inc: [Getter: NaN]\n}");
 }
 
+// Property getter throwing an error.
+{
+  const error = new Error('Oops');
+  error.stack = [
+    'Error: Oops',
+    '    at get foo (/foo/node_modules/foo.js:2:7)',
+    '    at get bar (/foo/node_modules/bar.js:827:30)',
+  ].join('\n');
+
+  const thrower = {
+    get foo() { throw error; }
+  };
+
+  assert.strictEqual(
+    inspect(thrower, { getters: true }),
+    '{\n' +
+    '  foo: [Getter: <Inspection threw (Error: Oops\n' +
+    '      at get foo (/foo/node_modules/foo.js:2:7)\n' +
+    '      at get bar (/foo/node_modules/bar.js:827:30))>]\n' +
+    '}',
+  );
+};
+
+// Property getter throwing an error with getters that throws.
+// https://github.com/nodejs/node/issues/60683
+{
+  const badError = new Error();
+
+  const innerError = new Error('Oops');
+  innerError.stack = [
+    'Error: Oops',
+    '    at get foo (/foo/node_modules/foo.js:2:7)',
+    '    at get bar (/foo/node_modules/bar.js:827:30)',
+  ].join('\n');
+
+  const throwingGetter = {
+    __proto__: null,
+    get() {
+      throw innerError;
+    },
+    configurable: true,
+    enumerable: true,
+  };
+
+  Object.defineProperties(badError, {
+    name: throwingGetter,
+    message: throwingGetter,
+    stack: throwingGetter,
+    cause: throwingGetter,
+  });
+
+  const thrower = {
+    get foo() { throw badError; }
+  };
+
+  assert.strictEqual(
+    inspect(thrower, { getters: true }),
+    '{\n' +
+    '  foo: [Getter: <Inspection threw ([object Error] {\n' +
+    '    stack: [Getter/Setter: <Inspection threw (Error: Oops\n' +
+    '        at get foo (/foo/node_modules/foo.js:2:7)\n' +
+    '        at get bar (/foo/node_modules/bar.js:827:30))>],\n' +
+    '    name: [Getter: <Inspection threw (Error: Oops\n' +
+    '        at get foo (/foo/node_modules/foo.js:2:7)\n' +
+    '        at get bar (/foo/node_modules/bar.js:827:30))>],\n' +
+    '    message: [Getter: <Inspection threw (Error: Oops\n' +
+    '        at get foo (/foo/node_modules/foo.js:2:7)\n' +
+    '        at get bar (/foo/node_modules/bar.js:827:30))>],\n' +
+    '    cause: [Getter: <Inspection threw (Error: Oops\n' +
+    '        at get foo (/foo/node_modules/foo.js:2:7)\n' +
+    '        at get bar (/foo/node_modules/bar.js:827:30))>]\n' +
+    '  })>]\n' +
+    '}'
+  );
+}
+
+// Property getter throwing an error with getters that throws recursivly.
+{
+  const recursivelyThrowingErrorDesc = {
+    __proto__: null,
+    // eslint-disable-next-line no-restricted-syntax
+    get() { throw createRecursivelyThrowingError(); },
+    configurable: true,
+    enumerable: true,
+  };
+  const createRecursivelyThrowingError = () =>
+    Object.defineProperties(new Error(), {
+      cause: recursivelyThrowingErrorDesc,
+      name: recursivelyThrowingErrorDesc,
+      message: recursivelyThrowingErrorDesc,
+      stack: recursivelyThrowingErrorDesc,
+    });
+  const thrower = Object.defineProperty({}, 'foo', recursivelyThrowingErrorDesc);
+
+  assert.strictEqual(
+    inspect(thrower, { getters: true, depth: 1 }),
+    '{\n' +
+    '  foo: [Getter: <Inspection threw ([object Error] {\n' +
+    '    stack: [Getter/Setter: <Inspection threw ([Error])>],\n' +
+    '    cause: [Getter: <Inspection threw ([Error])>],\n' +
+    '    name: [Getter: <Inspection threw ([Error])>],\n' +
+    '    message: [Getter: <Inspection threw ([Error])>]\n' +
+    '  })>]\n' +
+    '}'
+  );
+
+  [{ getters: true, depth: 2 }, { getters: true }].forEach((options) => {
+    assert.strictEqual(
+      inspect(thrower, options),
+      '{\n' +
+      '  foo: [Getter: <Inspection threw ([object Error] {\n' +
+      '    stack: [Getter/Setter: <Inspection threw ([object Error] {\n' +
+      '      stack: [Getter/Setter: <Inspection threw ([Error])>],\n' +
+      '      cause: [Getter: <Inspection threw ([Error])>],\n' +
+      '      name: [Getter: <Inspection threw ([Error])>],\n' +
+      '      message: [Getter: <Inspection threw ([Error])>]\n' +
+      '    })>],\n' +
+      '    cause: [Getter: <Inspection threw ([object Error] {\n' +
+      '      stack: [Getter/Setter: <Inspection threw ([Error])>],\n' +
+      '      cause: [Getter: <Inspection threw ([Error])>],\n' +
+      '      name: [Getter: <Inspection threw ([Error])>],\n' +
+      '      message: [Getter: <Inspection threw ([Error])>]\n' +
+      '    })>],\n' +
+      '    name: [Getter: <Inspection threw ([object Error] {\n' +
+      '      stack: [Getter/Setter: <Inspection threw ([Error])>],\n' +
+      '      cause: [Getter: <Inspection threw ([Error])>],\n' +
+      '      name: [Getter: <Inspection threw ([Error])>],\n' +
+      '      message: [Getter: <Inspection threw ([Error])>]\n' +
+      '    })>],\n' +
+      '    message: [Getter: <Inspection threw ([object Error] {\n' +
+      '      stack: [Getter/Setter: <Inspection threw ([Error])>],\n' +
+      '      cause: [Getter: <Inspection threw ([Error])>],\n' +
+      '      name: [Getter: <Inspection threw ([Error])>],\n' +
+      '      message: [Getter: <Inspection threw ([Error])>]\n' +
+      '    })>]\n' +
+      '  })>]\n' +
+      '}'
+    );
+  });
+}
+
+// Property getter throwing an error whose own getters throw that same error (infinite recursion).
+{
+  const badError = new Error();
+
+  const throwingGetter = {
+    __proto__: null,
+    get() {
+      throw badError;
+    },
+    configurable: true,
+    enumerable: true,
+  };
+
+  Object.defineProperties(badError, {
+    name: throwingGetter,
+    message: throwingGetter,
+    stack: throwingGetter,
+    cause: throwingGetter,
+  });
+
+  const thrower = {
+    get foo() { throw badError; }
+  };
+
+  assert.strictEqual(
+    inspect(thrower, { getters: true, depth: Infinity }),
+    '{\n' +
+    '  foo: [Getter: <Inspection threw (<ref *1> [object Error] {\n' +
+    '    stack: [Getter/Setter: <Inspection threw ([Circular *1])>],\n' +
+    '    name: [Getter: <Inspection threw ([Circular *1])>],\n' +
+    '    message: [Getter: <Inspection threw ([Circular *1])>],\n' +
+    '    cause: [Getter: <Inspection threw ([Circular *1])>]\n' +
+    '  })>]\n' +
+    '}'
+  );
+}
+
+// Property getter throwing uncommon values.
+[
+  {
+    val: undefined,
+    expected: '{ foo: [Getter: <Inspection threw (undefined)>] }'
+  },
+  {
+    val: null,
+    expected: '{ foo: [Getter: <Inspection threw (null)>] }'
+  },
+  {
+    val: true,
+    expected: '{ foo: [Getter: <Inspection threw (true)>] }'
+  },
+  {
+    val: 1,
+    expected: '{ foo: [Getter: <Inspection threw (1)>] }'
+  },
+  {
+    val: 1n,
+    expected: '{ foo: [Getter: <Inspection threw (1n)>] }'
+  },
+  {
+    val: Symbol(),
+    expected: '{ foo: [Getter: <Inspection threw (Symbol())>] }'
+  },
+  {
+    val: () => {},
+    expected: '{ foo: [Getter: <Inspection threw ([Function: val])>] }'
+  },
+  {
+    val: 'string',
+    expected: "{ foo: [Getter: <Inspection threw ('string')>] }"
+  },
+  {
+    val: [],
+    expected: '{ foo: [Getter: <Inspection threw ([])>] }'
+  },
+  {
+    val: { get message() { return 'Oops'; } },
+    expected: "{ foo: [Getter: <Inspection threw ({ message: [Getter: 'Oops'] })>] }"
+  },
+  {
+    val: Error,
+    expected: '{ foo: [Getter: <Inspection threw ([Function: Error])>] }'
+  },
+].forEach(({ val, expected }) => {
+  assert.strictEqual(
+    inspect({
+      get foo() { throw val; }
+    }, { getters: true }),
+    expected,
+  );
+});
+
 // Check compact number mode.
 {
   let obj = {
@@ -3231,25 +3462,26 @@ assert.strictEqual(
       '\x1B[2mdef: \x1B[33m5\x1B[39m\x1B[22m }'
   );
 
-  assert.strictEqual(
+  assert.match(
     inspect(Object.getPrototypeOf(bar), { showHidden: true, getters: true }),
-    '<ref *1> Foo [Map] {\n' +
-    '    [constructor]: [class Bar extends Foo] {\n' +
-    '      [length]: 0,\n' +
-    "      [name]: 'Bar',\n" +
-    '      [prototype]: [Circular *1],\n' +
-    '      [Symbol(Symbol.species)]: [Getter: <Inspection threw ' +
-      "(Symbol.prototype.toString requires that 'this' be a Symbol)>]\n" +
-    '    },\n' +
-    "    [xyz]: [Getter: 'YES!'],\n" +
-    '    [Symbol(nodejs.util.inspect.custom)]: ' +
-      '[Function: [nodejs.util.inspect.custom]] {\n' +
-    '      [length]: 0,\n' +
-    "      [name]: '[nodejs.util.inspect.custom]'\n" +
-    '    },\n' +
-    '    [abc]: [Getter: true],\n' +
-    '    [def]: [Getter/Setter: false]\n' +
-    '  }'
+    new RegExp('^' + RegExp.escape(
+      '<ref *1> Foo [Map] {\n' +
+      '  [constructor]: [class Bar extends Foo] {\n' +
+      '    [length]: 0,\n' +
+      "    [name]: 'Bar',\n" +
+      '    [prototype]: [Circular *1],\n' +
+      '    [Symbol(Symbol.species)]: [Getter: <Inspection threw ' +
+      "(TypeError: Symbol.prototype.toString requires that 'this' be a Symbol") + '.*' + RegExp.escape(')>]\n' +
+      '  },\n' +
+      "  [xyz]: [Getter: 'YES!'],\n" +
+      '  [Symbol(nodejs.util.inspect.custom)]: [Function: [nodejs.util.inspect.custom]] {\n' +
+      '    [length]: 0,\n' +
+      "    [name]: '[nodejs.util.inspect.custom]'\n" +
+      '  },\n' +
+      '  [abc]: [Getter: true],\n' +
+      '  [def]: [Getter/Setter: false]\n' +
+      '}'
+    ) + '$', 's')
   );
 
   assert.strictEqual(