errors: improve ERR_INVALID_ARG_TYPE

lib/buffer.js

@@ -291,17 +291,21 @@ Buffer.from = function from(value, encodingOrOffset, length) {
       return fromArrayBuffer(value, encodingOrOffset, length);
 
     const valueOf = value.valueOf && value.valueOf();
-    if (valueOf !== null && valueOf !== undefined && valueOf !== value)
-      return Buffer.from(valueOf, encodingOrOffset, length);
+    if (valueOf != null &&
+        valueOf !== value &&
+        (typeof valueOf === 'string' || typeof valueOf === 'object')) {
+      return from(valueOf, encodingOrOffset, length);
+    }
 
     const b = fromObject(value);
     if (b)
       return b;
 
     if (typeof value[SymbolToPrimitive] === 'function') {
-      return Buffer.from(value[SymbolToPrimitive]('string'),
-                         encodingOrOffset,
-                         length);
+      const primitive = value[SymbolToPrimitive]('string');
+      if (typeof primitive === 'string') {
+        return fromString(primitive, encodingOrOffset);
+      }
     }
   }
 

lib/internal/errors.js

@@ -23,6 +23,21 @@ const {
 const messages = new Map();
 const codes = {};
 
+const classRegExp = /^([A-Z][a-z0-9]*)+$/;
+// Sorted by a rough estimate on most frequently used entries.
+const kTypes = [
+  'string',
+  'function',
+  'number',
+  'object',
+  // Accept 'Function' and 'Object' as alternative to the lower cased version.
+  'Function',
+  'Object',
+  'boolean',
+  'bigint',
+  'symbol'
+];
+
 const { kMaxLength } = internalBinding('buffer');
 
 const MainContextError = Error;
@@ -610,26 +625,6 @@ function isStackOverflowError(err) {
          err.message === maxStack_ErrorMessage;
 }
 
-function oneOf(expected, thing) {
-  assert(typeof thing === 'string', '`thing` has to be of type string');
-  if (ArrayIsArray(expected)) {
-    const len = expected.length;
-    assert(len > 0,
-           'At least one expected value needs to be specified');
-    expected = expected.map((i) => String(i));
-    if (len > 2) {
-      return `one of ${thing} ${expected.slice(0, len - 1).join(', ')}, or ` +
-             expected[len - 1];
-    } else if (len === 2) {
-      return `one of ${thing} ${expected[0]} or ${expected[1]}`;
-    } else {
-      return `of ${thing} ${expected[0]}`;
-    }
-  } else {
-    return `of ${thing} ${String(expected)}`;
-  }
-}
-
 // Only use this for integers! Decimal numbers do not work with this function.
 function addNumericalSeparator(val) {
   let res = '';
@@ -926,27 +921,114 @@ E('ERR_INVALID_ADDRESS_FAMILY', function(addressType, host, port) {
 E('ERR_INVALID_ARG_TYPE',
   (name, expected, actual) => {
     assert(typeof name === 'string', "'name' must be a string");
+    if (!ArrayIsArray(expected)) {
+      expected = [expected];
+    }
+
+    let msg = 'The ';
+    if (name.endsWith(' argument')) {
+      // For cases like 'first argument'
+      msg += `${name} `;
+    } else {
+      const type = name.includes('.') ? 'property' : 'argument';
+      msg += `"${name}" ${type} `;
+    }
 
     // determiner: 'must be' or 'must not be'
-    let determiner;
     if (typeof expected === 'string' && expected.startsWith('not ')) {
-      determiner = 'must not be';
+      msg += 'must not be ';
       expected = expected.replace(/^not /, '');
     } else {
-      determiner = 'must be';
+      msg += 'must be ';
     }
 
-    let msg;
-    if (name.endsWith(' argument')) {
-      // For cases like 'first argument'
-      msg = `The ${name} ${determiner} ${oneOf(expected, 'type')}`;
-    } else {
-      const type = name.includes('.') ? 'property' : 'argument';
-      msg = `The "${name}" ${type} ${determiner} ${oneOf(expected, 'type')}`;
+    const types = [];
+    const instances = [];
+    const other = [];
+
+    for (const value of expected) {
+      assert(typeof value === 'string',
+             'All expected entries have to be of type string');
+      if (kTypes.includes(value)) {
+        types.push(value.toLowerCase());
+      } else if (classRegExp.test(value)) {
+        instances.push(value);
+      } else {
+        assert(value !== 'object',
+               'The value "object" should be written as "Object"');
+        other.push(value);
+      }
     }
 
-    // TODO(BridgeAR): Improve the output by showing `null` and similar.
-    msg += `. Received type ${typeof actual}`;
+    // Special handle `object` in case other instances are allowed to outline
+    // the differences between each other.
+    if (instances.length > 0) {
+      const pos = types.indexOf('object');
+      if (pos !== -1) {
+        types.splice(pos, 1);
+        instances.push('Object');
+      }
+    }
+
+    if (types.length > 0) {
+      if (types.length > 2) {
+        const last = types.pop();
+        msg += `one of type ${types.join(', ')}, or ${last}`;
+      } else if (types.length === 2) {
+        msg += `one of type ${types[0]} or ${types[1]}`;
+      } else {
+        msg += `of type ${types[0]}`;
+      }
+      if (instances.length > 0 || other.length > 0)
+        msg += ' or ';
+    }
+
+    if (instances.length > 0) {
+      if (instances.length > 2) {
+        const last = instances.pop();
+        msg += `an instance of ${instances.join(', ')}, or ${last}`;
+      } else {
+        msg += `an instance of ${instances[0]}`;
+        if (instances.length === 2) {
+          msg += ` or ${instances[1]}`;
+        }
+      }
+      if (other.length > 0)
+        msg += ' or ';
+    }
+
+    if (other.length > 0) {
+      if (other.length > 2) {
+        const last = other.pop();
+        msg += `one of ${other.join(', ')}, or ${last}`;
+      } else if (other.length === 2) {
+        msg += `one of ${other[0]} or ${other[1]}`;
+      } else {
+        if (other[0].toLowerCase() !== other[0])
+          msg += 'an ';
+        msg += `${other[0]}`;
+      }
+    }
+
+    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 && 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 = `${inspected.slice(0, 25)}...`;
+      msg += `. Received type ${typeof actual} (${inspected})`;
+    }
     return msg;
   }, TypeError);
 E('ERR_INVALID_ARG_VALUE', (name, value, reason = 'is invalid') => {
@@ -1034,7 +1116,15 @@ E('ERR_INVALID_URL', function(input) {
   return `Invalid URL: ${input}`;
 }, TypeError);
 E('ERR_INVALID_URL_SCHEME',
-  (expected) => `The URL must be ${oneOf(expected, 'scheme')}`, TypeError);
+  (expected) => {
+    if (typeof expected === 'string')
+      expected = [expected];
+    assert(expected.length <= 2);
+    const res = expected.length === 2 ?
+      `one of scheme ${expected[0]} or ${expected[1]}` :
+      `of scheme ${expected[0]}`;
+    return `The URL must be ${res}`;
+  }, TypeError);
 E('ERR_IPC_CHANNEL_CLOSED', 'Channel closed', Error);
 E('ERR_IPC_DISCONNECTED', 'IPC channel is already disconnected', Error);
 E('ERR_IPC_ONE_PIPE', 'Child process can have only one IPC pipe', Error);

test/common/index.js

@@ -718,6 +718,26 @@ function runWithInvalidFD(func) {
   printSkipMessage('Could not generate an invalid fd');
 }
 
+// A helper function to simplify checking for ERR_INVALID_ARG_TYPE output.
+function invalidArgTypeHelper(input) {
+  if (input == null) {
+    return ` Received ${input}`;
+  }
+  if (typeof input === 'function' && input.name) {
+    return ` Received function ${input.name}`;
+  }
+  if (typeof input === 'object') {
+    if (input.constructor && input.constructor.name) {
+      return ` Received an instance of ${input.constructor.name}`;
+    }
+    return ` Received ${util.inspect(input, { depth: -1 })}`;
+  }
+  let inspected = util.inspect(input, { colors: false });
+  if (inspected.length > 25)
+    inspected = `${inspected.slice(0, 25)}...`;
+  return ` Received type ${typeof input} (${inspected})`;
+}
+
 module.exports = {
   allowGlobals,
   buildType,
@@ -735,6 +755,7 @@ module.exports = {
   hasIntl,
   hasCrypto,
   hasMultiLocalhost,
+  invalidArgTypeHelper,
   isAIX,
   isAlive,
   isFreeBSD,

test/es-module/test-esm-loader-modulemap.js

@@ -25,7 +25,8 @@ common.expectsError(
   {
     code: 'ERR_INVALID_ARG_TYPE',
     type: TypeError,
-    message: 'The "url" argument must be of type string. Received type number'
+    message: 'The "url" argument must be of type string. Received type number' +
+             ' (1)'
   }
 );
 
@@ -34,7 +35,8 @@ common.expectsError(
   {
     code: 'ERR_INVALID_ARG_TYPE',
     type: TypeError,
-    message: 'The "url" argument must be of type string. Received type number'
+    message: 'The "url" argument must be of type string. Received type number' +
+             ' (1)'
   }
 );
 
@@ -43,8 +45,8 @@ common.expectsError(
   {
     code: 'ERR_INVALID_ARG_TYPE',
     type: TypeError,
-    message: 'The "job" argument must be of type ModuleJob. ' +
-             'Received type string'
+    message: 'The "job" argument must be an instance of ModuleJob. ' +
+             "Received type string ('notamodulejob')"
   }
 );
 
@@ -53,6 +55,7 @@ common.expectsError(
   {
     code: 'ERR_INVALID_ARG_TYPE',
     type: TypeError,
-    message: 'The "url" argument must be of type string. Received type number'
+    message: 'The "url" argument must be of type string. Received type number' +
+             ' (1)'
   }
 );

test/internet/test-dns-promises-resolve.js

@@ -26,7 +26,7 @@ const dnsPromises = require('dns').promises;
       code: 'ERR_INVALID_ARG_TYPE',
       type: TypeError,
       message: 'The "rrtype" argument must be of type string. ' +
-               `Received type ${typeof rrtype}`
+               `Received type ${typeof rrtype} (${rrtype})`
     }
   );
 }

test/parallel/test-assert-async.js

@@ -125,8 +125,8 @@ promises.push(assert.rejects(
   assert.rejects('fail', {}),
   {
     code: 'ERR_INVALID_ARG_TYPE',
-    message: 'The "promiseFn" argument must be one of type ' +
-             'Function or Promise. Received type string'
+    message: 'The "promiseFn" argument must be of type function or an ' +
+             "instance of Promise. Received type string ('fail')"
   }
 ));
 
@@ -225,8 +225,8 @@ promises.push(assert.rejects(
     assert.doesNotReject(123),
     {
       code: 'ERR_INVALID_ARG_TYPE',
-      message: 'The "promiseFn" argument must be one of type ' +
-               'Function or Promise. Received type number'
+      message: 'The "promiseFn" argument must be of type ' +
+               'function or an instance of Promise. Received type number (123)'
     }
   ));
   /* eslint-enable no-restricted-syntax */

test/parallel/test-assert.js

@@ -414,8 +414,8 @@ assert.throws(
       {
         code: 'ERR_INVALID_ARG_TYPE',
         type: TypeError,
-        message: 'The "fn" argument must be of type Function. Received ' +
-                 `type ${typeof fn}`
+        message: 'The "fn" argument must be of type function.' +
+                 common.invalidArgTypeHelper(fn)
       }
     );
   };
@@ -484,8 +484,8 @@ assert.throws(() => {
       {
         code: 'ERR_INVALID_ARG_TYPE',
         name: 'TypeError',
-        message: 'The "options" argument must be of type Object. ' +
-                 `Received type ${typeof input}`
+        message: 'The "options" argument must be of type object.' +
+                 common.invalidArgTypeHelper(input)
       });
   });
 }
@@ -937,8 +937,9 @@ common.expectsError(
   {
     code: 'ERR_INVALID_ARG_TYPE',
     type: TypeError,
-    message: 'The "error" argument must be one of type Object, Error, ' +
-             'Function, or RegExp. Received type string'
+    message: 'The "error" argument must be of type function or ' +
+             'an instance of Error, RegExp, or Object. Received type string ' +
+             "('Error message')"
   }
 );
 
@@ -951,8 +952,9 @@ common.expectsError(
     () => assert.throws(() => {}, input),
     {
       code: 'ERR_INVALID_ARG_TYPE',
-      message: 'The "error" argument must be one of type Object, Error, ' +
-               `Function, or RegExp. Received type ${typeof input}`
+      message: 'The "error" argument must be of type function or ' +
+               'an instance of Error, RegExp, or Object.' +
+               common.invalidArgTypeHelper(input)
     }
   );
 });
@@ -1030,8 +1032,8 @@ common.expectsError(
     {
       type: TypeError,
       code: 'ERR_INVALID_ARG_TYPE',
-      message: 'The "expected" argument must be one of type Function or ' +
-               'RegExp. Received type object'
+      message: 'The "expected" argument must be of type function or an ' +
+               'instance of RegExp. Received an instance of Object'
     }
   );