util: use more defensive code when inspecting error objects

Author: aduh95

lib/internal/util.js

@@ -8,6 +8,7 @@ const {
   Error,
   ErrorCaptureStackTrace,
   FunctionPrototypeCall,
+  FunctionPrototypeSymbolHasInstance,
   NumberParseInt,
   ObjectDefineProperties,
   ObjectDefineProperty,
@@ -96,7 +97,7 @@ function isError(e) {
   // An error could be an instance of Error while not being a native error
   // or could be from a different realm and not be instance of Error but still
   // be a native error.
-  return isNativeError(e) || e instanceof Error;
+  return isNativeError(e) || FunctionPrototypeSymbolHasInstance(Error, e);
 }
 
 // Keep a list of deprecation codes that have been warned on so we only warn on

lib/internal/util/inspect.js

@@ -72,6 +72,7 @@ const {
   ObjectPrototype,
   ObjectPrototypeHasOwnProperty,
   ObjectPrototypePropertyIsEnumerable,
+  ObjectPrototypeToString,
   ObjectSeal,
   ObjectSetPrototypeOf,
   Promise,
@@ -1703,13 +1704,19 @@ function getDuplicateErrorFrameRanges(frames) {
 }
 
 function getStackString(ctx, error) {
-  if (error.stack) {
-    if (typeof error.stack === 'string') {
-      return error.stack;
+  let stack;
+  try {
+    stack = error.stack;
+  } catch {
+    // If stack is getter that throws, we ignore the error.
+  }
+  if (stack) {
+    if (typeof stack === 'string') {
+      return stack;
     }
     ctx.seen.push(error);
     ctx.indentationLvl += 4;
-    const result = formatValue(ctx, error.stack);
+    const result = formatValue(ctx, stack);
     ctx.indentationLvl -= 4;
     ctx.seen.pop();
     return `${ErrorPrototypeToString(error)}\n    ${result}`;
@@ -1805,18 +1812,6 @@ function improveStack(stack, constructor, name, tag) {
   return stack;
 }
 
-function removeDuplicateErrorKeys(ctx, keys, err, stack) {
-  if (!ctx.showHidden && keys.length !== 0) {
-    for (const name of ['name', 'message', 'stack']) {
-      const index = ArrayPrototypeIndexOf(keys, name);
-      // Only hide the property if it's a string and if it's part of the original stack
-      if (index !== -1 && (typeof err[name] !== 'string' || StringPrototypeIncludes(stack, err[name]))) {
-        ArrayPrototypeSplice(keys, index, 1);
-      }
-    }
-  }
-}
-
 function markNodeModules(ctx, line) {
   let tempLine = '';
   let lastPos = 0;
@@ -1899,28 +1894,67 @@ function safeGetCWD() {
 }
 
 function formatError(err, constructor, tag, ctx, keys) {
-  const name = err.name != null ? err.name : 'Error';
-  let stack = getStackString(ctx, err);
-
-  removeDuplicateErrorKeys(ctx, keys, err, stack);
+  let message, name, stack;
+  try {
+    stack = getStackString(ctx, err);
+    if (!ctx.showHidden && keys.length !== 0) {
+      const index = ArrayPrototypeIndexOf(keys, 'stack');
+      // Only hide the property if it's a string and if it's part of the original stack
+      if (index !== -1) {
+        ArrayPrototypeSplice(keys, index, 1);
+      }
+    }
+  } catch {
+    return ObjectPrototypeToString(err);
+  }
+  try {
+    message = err.message;
+    if (!ctx.showHidden && keys.length !== 0) {
+      const index = ArrayPrototypeIndexOf(keys, 'message');
+      // Only hide the property if it's a string and if it's part of the original stack
+      if (index !== -1 && (typeof message !== 'string' || StringPrototypeIncludes(stack, message))) {
+        ArrayPrototypeSplice(keys, index, 1);
+      }
+    }
+  } catch {
+    // If message is a getter that throws, we ignore the error.
+  }
+  try {
+    name = err.name;
+    if (!ctx.showHidden && keys.length !== 0) {
+      const index = ArrayPrototypeIndexOf(keys, 'name');
+      // Only hide the property if it's a string and if it's part of the original stack
+      if (index !== -1 && (typeof name !== 'string' || StringPrototypeIncludes(stack, name))) {
+        ArrayPrototypeSplice(keys, index, 1);
+      }
+    }
+    name ??= 'Error';
+  } catch {
+    name = 'Error';
+  }
 
   if ('cause' in err &&
       (keys.length === 0 || !ArrayPrototypeIncludes(keys, 'cause'))) {
     ArrayPrototypePush(keys, 'cause');
   }
 
   // Print errors aggregated into AggregateError
-  if (ArrayIsArray(err.errors) &&
+  try {
+    const errors = err.errors;
+    if (ArrayIsArray(errors) &&
       (keys.length === 0 || !ArrayPrototypeIncludes(keys, 'errors'))) {
-    ArrayPrototypePush(keys, 'errors');
+      ArrayPrototypePush(keys, 'errors');
+    }
+  } catch {
+    // If errors is a getter that throws, we ignore the error.
   }
 
   stack = improveStack(stack, constructor, name, tag);
 
   // Ignore the error message if it's contained in the stack.
-  let pos = (err.message && StringPrototypeIndexOf(stack, err.message)) || -1;
+  let pos = (message && StringPrototypeIndexOf(stack, message)) || -1;
   if (pos !== -1)
-    pos += err.message.length;
+    pos += message.length;
   // Wrap the error in brackets in case it has no stack trace.
   const stackStart = StringPrototypeIndexOf(stack, '\n    at', pos);
   if (stackStart === -1) {

test/parallel/test-util-inspect.js

@@ -3705,3 +3705,28 @@ ${error.stack.split('\n').slice(1).join('\n')}`,
 
   assert.strictEqual(inspect(error), '[Error: foo\n    [Error: bar\n        [Circular *1]]]');
 }
+
+{
+  Object.defineProperty(Error, Symbol.hasInstance,
+                        { __proto__: null, value: common.mustNotCall(), configurable: true });
+  const error = new Error();
+
+  const throwingGetter = {
+    __proto__: null,
+    get() {
+      throw error;
+    },
+    configurable: true,
+    enumerable: true,
+  };
+
+  Object.defineProperties(error, {
+    name: throwingGetter,
+    stack: throwingGetter,
+    cause: throwingGetter,
+  });
+
+  assert.strictEqual(inspect(error), `[object Error] {\n  stack: [Getter/Setter],\n  name: [Getter],\n  cause: [Getter]\n}`);
+  assert.match(inspect(DOMException.prototype), /^\[object DOMException\] \{/);
+  delete Error[Symbol.hasInstance];
+}