errors: extract type detection & use in ERR_INVALID_RETURN_VALUE

PR-URL: #43558 Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de> Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
nodejs · Jul 31, 2022 · 9f75f26 · 9f75f26
1 parent 78bedcd
commit 9f75f26
Show file tree

Hide file tree

Showing 4 changed files with 187 additions and 30 deletions.
diff --git a/lib/internal/errors.js b/lib/internal/errors.js
@@ -845,6 +845,31 @@ const genericNodeError = hideStackFrames(function genericNodeError(message, erro
   return err;
 });
 
+/**
+ * Determine the specific type of a value for type-mismatch errors.
+ * @param {*} value
+ * @returns {string}
+ */
+function determineSpecificType(value) {
+  if (value == null) {
+    return '' + value;
+  }
+  if (typeof value === 'function' && value.name) {
+    return `function ${value.name}`;
+  }
+  if (typeof value === 'object') {
+    if (value.constructor?.name) {
+      return `an instance of ${value.constructor.name}`;
+    }
+    return `${lazyInternalUtilInspect().inspect(value, { depth: -1 })}`;
+  }
+  let inspected = lazyInternalUtilInspect()
+    .inspect(value, { colors: false });
+  if (inspected.length > 28) { inspected = `${StringPrototypeSlice(inspected, 0, 25)}...`; }
+
+  return `type ${typeof value} (${inspected})`;
+}
+
 module.exports = {
   AbortError,
   aggregateTwoErrors,
@@ -853,6 +878,7 @@ module.exports = {
   connResetException,
   dnsException,
   // This is exported only to facilitate testing.
+  determineSpecificType,
   E,
   errnoException,
   exceptionWithHostPort,
@@ -1221,25 +1247,8 @@ E('ERR_INVALID_ARG_TYPE',
       }
     }
 
-    if (actual == null) {
-      msg += `. Received ${actual}`;
-    } else if (typeof actual === 'function' && actual.name) {
-      msg += `. Received function ${actual.name}`;
-    } else if (typeof actual === 'object') {
-      if (actual.constructor?.name) {
-        msg += `. Received an instance of ${actual.constructor.name}`;
-      } else {
-        const inspected = lazyInternalUtilInspect()
-          .inspect(actual, { depth: -1 });
-        msg += `. Received ${inspected}`;
-      }
-    } else {
-      let inspected = lazyInternalUtilInspect()
-        .inspect(actual, { colors: false });
-      if (inspected.length > 25)
-        inspected = `${StringPrototypeSlice(inspected, 0, 25)}...`;
-      msg += `. Received type ${typeof actual} (${inspected})`;
-    }
+    msg += `. Received ${determineSpecificType(actual)}`;
+
     return msg;
   }, TypeError);
 E('ERR_INVALID_ARG_VALUE', (name, value, reason = 'is invalid') => {
@@ -1321,12 +1330,8 @@ E('ERR_INVALID_RETURN_PROPERTY_VALUE', (input, name, prop, value) => {
          ` "${name}" function but got ${type}.`;
 }, TypeError);
 E('ERR_INVALID_RETURN_VALUE', (input, name, value) => {
-  let type;
-  if (value?.constructor?.name) {
-    type = `instance of ${value.constructor.name}`;
-  } else {
-    type = `type ${typeof value}`;
-  }
+  const type = determineSpecificType(value);
+
   return `Expected ${input} to be returned from the "${name}"` +
          ` function but got ${type}.`;
 }, TypeError, RangeError);

diff --git a/test/common/index.js b/test/common/index.js
@@ -716,14 +716,15 @@ function invalidArgTypeHelper(input) {
     return ` Received function ${input.name}`;
   }
   if (typeof input === 'object') {
-    if (input.constructor && input.constructor.name) {
+    if (input.constructor?.name) {
       return ` Received an instance of ${input.constructor.name}`;
     }
     return ` Received ${inspect(input, { depth: -1 })}`;
   }
+
   let inspected = inspect(input, { colors: false });
-  if (inspected.length > 25)
-    inspected = `${inspected.slice(0, 25)}...`;
+  if (inspected.length > 28) { inspected = `${inspected.slice(inspected, 0, 25)}...`; }
+
   return ` Received type ${typeof input} (${inspected})`;
 }
 

diff --git a/test/errors/test-error-value-type-detection.mjs b/test/errors/test-error-value-type-detection.mjs
@@ -0,0 +1,150 @@
+// Flags: --expose-internals
+
+import '../common/index.mjs';
+import { strictEqual } from 'node:assert';
+import errorsModule from 'internal/errors';
+
+
+const { determineSpecificType } = errorsModule;
+
+strictEqual(
+  determineSpecificType(1n),
+  'type bigint (1n)',
+);
+
+strictEqual(
+  determineSpecificType(false),
+  'type boolean (false)',
+);
+
+strictEqual(
+  determineSpecificType(2),
+  'type number (2)',
+);
+
+strictEqual(
+  determineSpecificType(NaN),
+  'type number (NaN)',
+);
+
+strictEqual(
+  determineSpecificType(Infinity),
+  'type number (Infinity)',
+);
+
+strictEqual(
+  determineSpecificType(Object.create(null)),
+  '[Object: null prototype] {}',
+);
+
+strictEqual(
+  determineSpecificType(''),
+  "type string ('')",
+);
+
+strictEqual(
+  determineSpecificType(Symbol('foo')),
+  'type symbol (Symbol(foo))',
+);
+
+strictEqual(
+  determineSpecificType(function foo() {}),
+  'function foo',
+);
+
+strictEqual(
+  determineSpecificType(null),
+  'null',
+);
+
+strictEqual(
+  determineSpecificType(undefined),
+  'undefined',
+);
+
+strictEqual(
+  determineSpecificType([]),
+  'an instance of Array',
+);
+
+strictEqual(
+  determineSpecificType(new Array()),
+  'an instance of Array',
+);
+strictEqual(
+  determineSpecificType(new BigInt64Array()),
+  'an instance of BigInt64Array',
+);
+strictEqual(
+  determineSpecificType(new BigUint64Array()),
+  'an instance of BigUint64Array',
+);
+strictEqual(
+  determineSpecificType(new Int8Array()),
+  'an instance of Int8Array',
+);
+strictEqual(
+  determineSpecificType(new Int16Array()),
+  'an instance of Int16Array',
+);
+strictEqual(
+  determineSpecificType(new Int32Array()),
+  'an instance of Int32Array',
+);
+strictEqual(
+  determineSpecificType(new Float32Array()),
+  'an instance of Float32Array',
+);
+strictEqual(
+  determineSpecificType(new Float64Array()),
+  'an instance of Float64Array',
+);
+strictEqual(
+  determineSpecificType(new Uint8Array()),
+  'an instance of Uint8Array',
+);
+strictEqual(
+  determineSpecificType(new Uint8ClampedArray()),
+  'an instance of Uint8ClampedArray',
+);
+strictEqual(
+  determineSpecificType(new Uint16Array()),
+  'an instance of Uint16Array',
+);
+strictEqual(
+  determineSpecificType(new Uint32Array()),
+  'an instance of Uint32Array',
+);
+
+strictEqual(
+  determineSpecificType(new Date()),
+  'an instance of Date',
+);
+
+strictEqual(
+  determineSpecificType(new Map()),
+  'an instance of Map',
+);
+strictEqual(
+  determineSpecificType(new WeakMap()),
+  'an instance of WeakMap',
+);
+
+strictEqual(
+  determineSpecificType({}),
+  'an instance of Object',
+);
+
+strictEqual(
+  determineSpecificType(Promise.resolve('foo')),
+  'an instance of Promise',
+);
+
+strictEqual(
+  determineSpecificType(new Set()),
+  'an instance of Set',
+);
+strictEqual(
+  determineSpecificType(new WeakSet()),
+  'an instance of WeakSet',
+);
diff --git a/test/parallel/test-assert-async.js b/test/parallel/test-assert-async.js
@@ -103,8 +103,9 @@ const invalidThenableFunc = () => {
   promises.push(assert.rejects(promise, {
     name: 'TypeError',
     code: 'ERR_INVALID_RETURN_VALUE',
+    // FIXME(JakobJingleheimer): This should match on key words, like /Promise/ and /undefined/.
     message: 'Expected instance of Promise to be returned ' +
-             'from the "promiseFn" function but got type undefined.'
+             'from the "promiseFn" function but got undefined.'
   }));
 
   promise = assert.rejects(Promise.resolve(), common.mustNotCall());
@@ -162,7 +163,7 @@ promises.push(assert.rejects(
   let promise = assert.doesNotReject(() => new Map(), common.mustNotCall());
   promises.push(assert.rejects(promise, {
     message: 'Expected instance of Promise to be returned ' +
-             'from the "promiseFn" function but got instance of Map.',
+             'from the "promiseFn" function but got an instance of Map.',
     code: 'ERR_INVALID_RETURN_VALUE',
     name: 'TypeError'
   }));