errors: improve performance of determine-specific-type

PR-URL: #49696 Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com> Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
nodejs · Nov 11, 2023 · c255575 · c255575
1 parent 5131fde
commit c255575
Show file tree

Hide file tree

Showing 5 changed files with 167 additions and 17 deletions.
diff --git a/benchmark/error/determine-specific-type.js b/benchmark/error/determine-specific-type.js
@@ -0,0 +1,58 @@
+'use strict';
+
+const common = require('../common');
+
+const bench = common.createBenchmark(main, {
+  n: [1e6],
+  v: [
+    '() => 1n',
+    '() => true',
+    '() => false',
+    '() => 2',
+    '() => +0',
+    '() => -0',
+    '() => NaN',
+    '() => Infinity',
+    '() => ""',
+    '() => "\'"',
+    '() => Symbol("foo")',
+    '() => function foo() {}',
+    '() => null',
+    '() => undefined',
+    '() => new Array()',
+    '() => new BigInt64Array()',
+    '() => new BigUint64Array()',
+    '() => new Int8Array()',
+    '() => new Int16Array()',
+    '() => new Int32Array()',
+    '() => new Float32Array()',
+    '() => new Float64Array()',
+    '() => new Uint8Array()',
+    '() => new Uint8ClampedArray()',
+    '() => new Uint16Array()',
+    '() => new Uint32Array()',
+    '() => new Date()',
+    '() => new Map()',
+    '() => new WeakMap()',
+    '() => new Object()',
+    '() => Promise.resolve("foo")',
+    '() => new Set()',
+    '() => new WeakSet()',
+    '() => ({ __proto__: null })',
+  ],
+}, {
+  flags: ['--expose-internals'],
+});
+
+function main({ n, v }) {
+  const {
+    determineSpecificType,
+  } = require('internal/errors');
+
+  const value = eval(v)();
+
+  bench.start();
+  for (let i = 0; i < n; ++i)
+    determineSpecificType(value);
+  bench.end(n);
+}
diff --git a/lib/internal/errors.js b/lib/internal/errors.js
@@ -48,6 +48,7 @@ const {
   String,
   StringPrototypeEndsWith,
   StringPrototypeIncludes,
+  StringPrototypeIndexOf,
   StringPrototypeSlice,
   StringPrototypeSplit,
   StringPrototypeStartsWith,
@@ -939,23 +940,53 @@ const genericNodeError = hideStackFrames(function genericNodeError(message, erro
  * @returns {string}
  */
 function determineSpecificType(value) {
-  if (value == null) {
-    return '' + value;
+  if (value === null) {
+    return 'null';
+  } else if (value === undefined) {
+    return 'undefined';
   }
-  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})`;
+  const type = typeof value;
+
+  switch (type) {
+    case 'bigint':
+      return `type bigint (${value}n)`;
+    case 'number':
+      if (value === 0) {
+        return 1 / value === -Infinity ? 'type number (-0)' : 'type number (0)';
+      } else if (value !== value) { // eslint-disable-line no-self-compare
+        return 'type number (NaN)';
+      } else if (value === Infinity) {
+        return 'type number (Infinity)';
+      } else if (value === -Infinity) {
+        return 'type number (-Infinity)';
+      }
+      return `type number (${value})`;
+    case 'boolean':
+      return value ? 'type boolean (true)' : 'type boolean (false)';
+    case 'symbol':
+      return `type symbol (${String(value)})`;
+    case 'function':
+      return `function ${value.name}`;
+    case 'object':
+      if (value.constructor && 'name' in value.constructor) {
+        return `an instance of ${value.constructor.name}`;
+      }
+      return `${lazyInternalUtilInspect().inspect(value, { depth: -1 })}`;
+    case 'string':
+      value.length > 28 && (value = `${StringPrototypeSlice(value, 0, 25)}...`);
+      if (StringPrototypeIndexOf(value, "'") === -1) {
+        return `type string ('${value}')`;
+      }
+      return `type string (${JSONStringify(value)})`;
+    default:
+      value = lazyInternalUtilInspect().inspect(value, { colors: false });
+      if (value.length > 28) {
+        value = `${StringPrototypeSlice(value, 0, 25)}...`;
+      }
+
+      return `type ${type} (${value})`;
+  }
 }
 
 /**

diff --git a/test/common/index.js b/test/common/index.js
@@ -808,7 +808,7 @@ function invalidArgTypeHelper(input) {
   if (input == null) {
     return ` Received ${input}`;
   }
-  if (typeof input === 'function' && input.name) {
+  if (typeof input === 'function') {
     return ` Received function ${input.name}`;
   }
   if (typeof input === 'object') {

diff --git a/test/parallel/test-error-value-type-detection.mjs b/test/parallel/test-error-value-type-detection.mjs
@@ -12,6 +12,10 @@ strictEqual(
   'type bigint (1n)',
 );
 
+strictEqual(
+  determineSpecificType(true),
+  'type boolean (true)',
+);
 strictEqual(
   determineSpecificType(false),
   'type boolean (false)',
@@ -42,6 +46,27 @@ strictEqual(
   "type string ('')",
 );
 
+strictEqual(
+  determineSpecificType(''),
+  "type string ('')",
+);
+strictEqual(
+  determineSpecificType("''"),
+  "type string (\"''\")",
+);
+strictEqual(
+  determineSpecificType('Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do eiusmod tempor'),
+  "type string ('Lorem ipsum dolor sit ame...')",
+);
+strictEqual(
+  determineSpecificType("Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do eiusmod tempor'"),
+  "type string ('Lorem ipsum dolor sit ame...')",
+);
+strictEqual(
+  determineSpecificType("Lorem' ipsum dolor sit amet, consectetur adipiscing elit, sed do eiusmod tempor"),
+  "type string (\"Lorem' ipsum dolor sit am...\")",
+);
+
 strictEqual(
   determineSpecificType(Symbol('foo')),
   'type symbol (Symbol(foo))',
@@ -52,6 +77,38 @@ strictEqual(
   'function foo',
 );
 
+const implicitlyNamed = function() {}; // eslint-disable-line func-style
+strictEqual(
+  determineSpecificType(implicitlyNamed),
+  'function implicitlyNamed',
+);
+strictEqual(
+  determineSpecificType(() => {}),
+  'function ',
+);
+function noName() {}
+delete noName.name;
+strictEqual(
+  noName.name,
+  '',
+);
+strictEqual(
+  determineSpecificType(noName),
+  'function ',
+);
+
+function * generatorFn() {}
+strictEqual(
+  determineSpecificType(generatorFn),
+  'function generatorFn',
+);
+
+async function asyncFn() {}
+strictEqual(
+  determineSpecificType(asyncFn),
+  'function asyncFn',
+);
+
 strictEqual(
   determineSpecificType(null),
   'null',
@@ -134,6 +191,10 @@ strictEqual(
   determineSpecificType({}),
   'an instance of Object',
 );
+strictEqual(
+  determineSpecificType(new Object()),
+  'an instance of Object',
+);
 
 strictEqual(
   determineSpecificType(Promise.resolve('foo')),

diff --git a/test/parallel/test-fs-readfile-error.js b/test/parallel/test-fs-readfile-error.js
@@ -61,7 +61,7 @@ assert.throws(
   {
     code: 'ERR_INVALID_ARG_TYPE',
     message: 'The "path" argument must be of type string or an instance of ' +
-             'Buffer or URL. Received type function ([Function (anonymous)])',
+             'Buffer or URL. Received function ',
     name: 'TypeError'
   }
 );