errors: extract type detection & use in ERR_INVALID_RETURN_VALUE

nodejs · Jun 26, 2022 · 74be6fe · 74be6fe
1 parent 3507b3f
commit 74be6fe
Show file tree

Hide file tree

Showing 3 changed files with 195 additions and 27 deletions.
diff --git a/lib/internal/errors.js b/lib/internal/errors.js
@@ -858,6 +858,44 @@ 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) {
+  let type = '';
+
+  if (value == null) {
+    type += value;
+  } else if (typeof value === 'function' && value.name) {
+    type = `function ${value.name}`;
+  } else if (typeof value === 'object') {
+    if (value.constructor?.name) {
+      type = `an instance of ${value.constructor.name}`;
+    } else {
+      const inspected = lazyInternalUtilInspect()
+        .inspect(value, { depth: -1 });
+
+      if (StringPrototypeIncludes(inspected, '[Object: null prototype]')) {
+        type = 'an instance of Object';
+      } else {
+        type = inspected;
+      }
+    }
+  } else {
+    let inspected = lazyInternalUtilInspect()
+      .inspect(value, { colors: false });
+    if (inspected.length > 25) {
+      inspected = `${StringPrototypeSlice(inspected, 0, 25)}...`;
+    }
+
+    type = `type ${typeof value} (${inspected})`;
+  }
+
+  return type;
+}
+
 module.exports = {
   AbortError,
   aggregateTwoErrors,
@@ -866,6 +904,7 @@ module.exports = {
   connResetException,
   dnsException,
   // This is exported only to facilitate testing.
+  determineSpecificType,
   E,
   errnoException,
   exceptionWithHostPort,
@@ -1237,25 +1276,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') => {
@@ -1335,12 +1357,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/errors/test-error-value-type-detection.mjs b/test/errors/test-error-value-type-detection.mjs
@@ -0,0 +1,149 @@
+// 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(''),
+  "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(0)),
+  'an instance of Array',
+);
+strictEqual(
+  determineSpecificType(new BigInt64Array(0)),
+  'an instance of BigInt64Array',
+);
+strictEqual(
+  determineSpecificType(new BigUint64Array(0)),
+  'an instance of BigUint64Array',
+);
+strictEqual(
+  determineSpecificType(new Int8Array(0)),
+  'an instance of Int8Array',
+);
+strictEqual(
+  determineSpecificType(new Int16Array(0)),
+  'an instance of Int16Array',
+);
+strictEqual(
+  determineSpecificType(new Int32Array(0)),
+  'an instance of Int32Array',
+);
+strictEqual(
+  determineSpecificType(new Float32Array(0)),
+  'an instance of Float32Array',
+);
+strictEqual(
+  determineSpecificType(new Float64Array(0)),
+  'an instance of Float64Array',
+);
+strictEqual(
+  determineSpecificType(new Uint8Array(0)),
+  'an instance of Uint8Array',
+);
+strictEqual(
+  determineSpecificType(new Uint8ClampedArray(0)),
+  'an instance of Uint8ClampedArray',
+);
+strictEqual(
+  determineSpecificType(new Uint16Array(0)),
+  'an instance of Uint16Array',
+);
+strictEqual(
+  determineSpecificType(new Uint32Array(0)),
+  '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(Object.create(null)),
+  'an instance of Object',
+);
+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: This should use substring matching for 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'
   }));