errors: improve ERR_INVALID_ARG_TYPE

ERR_INVALID_ARG_TYPE is the most common error used throughout the code base. This improves the error message by providing more details to the user and by indicating more precisely which values are allowed ones and which ones are not. It adds the actual input to the error message in case it's a primitive. If it's a class instance, it'll print the class name instead of "object" and "falsy" or similar entries are not named "type" anymore. PR-URL: #29675 Reviewed-By: Rich Trott <[email protected]>

Author: BridgeAR

lib/internal/errors.js

@@ -26,6 +26,21 @@ const{
 constmessages=newMap();
 constcodes={};
 
+constclassRegExp=/^([A-Z][a-z0-9]*)+$/;
+// Sorted by a rough estimate on most frequently used entries.
+constkTypes=[
+'string',
+'function',
+'number',
+'object',
+// Accept 'Function' and 'Object' as alternative to the lower cased version.
+'Function',
+'Object',
+'boolean',
+'bigint',
+'symbol'
+];
+
 const{ kMaxLength }=internalBinding('buffer');
 
 constMainContextError=Error;
@@ -616,26 +631,6 @@ function isStackOverflowError(err){
 err.message===maxStack_ErrorMessage;
 }
 
-functiononeOf(expected,thing){
-assert(typeofthing==='string','`thing` has to be of type string');
-if(ArrayIsArray(expected)){
-constlen=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];
-}elseif(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.
 functionaddNumericalSeparator(val){
 letres='';
@@ -934,27 +929,114 @@ E('ERR_INVALID_ADDRESS_FAMILY', function(addressType, host, port){
 E('ERR_INVALID_ARG_TYPE',
 (name,expected,actual)=>{
 assert(typeofname==='string',"'name' must be a string");
+if(!ArrayIsArray(expected)){
+expected=[expected];
+}
+
+letmsg='The ';
+if(name.endsWith(' argument')){
+// For cases like 'first argument'
+msg+=`${name} `;
+}else{
+consttype=name.includes('.') ? 'property' : 'argument';
+msg+=`"${name}" ${type} `;
+}
 
 // determiner: 'must be' or 'must not be'
-letdeterminer;
 if(typeofexpected==='string'&&expected.startsWith('not ')){
-determiner='must not be';
+msg+='must not be';
 expected=expected.replace(/^not/,'');
 }else{
-determiner='must be';
+msg+='must be';
 }
 
-letmsg;
-if(name.endsWith(' argument')){
-// For cases like 'first argument'
-msg=`The ${name}${determiner}${oneOf(expected,'type')}`;
-}else{
-consttype=name.includes('.') ? 'property' : 'argument';
-msg=`The "${name}" ${type}${determiner}${oneOf(expected,'type')}`;
+consttypes=[];
+constinstances=[];
+constother=[];
+
+for(constvalueofexpected){
+assert(typeofvalue==='string',
+'All expected entries have to be of type string');
+if(kTypes.includes(value)){
+types.push(value.toLowerCase());
+}elseif(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 ${typeofactual}`;
+// Special handle `object` in case other instances are allowed to outline
+// the differences between each other.
+if(instances.length>0){
+constpos=types.indexOf('object');
+if(pos!==-1){
+types.splice(pos,1);
+instances.push('Object');
+}
+}
+
+if(types.length>0){
+if(types.length>2){
+constlast=types.pop();
+msg+=`one of type ${types.join(', ')}, or ${last}`;
+}elseif(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){
+constlast=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){
+constlast=other.pop();
+msg+=`one of ${other.join(', ')}, or ${last}`;
+}elseif(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}`;
+}elseif(typeofactual==='function'&&actual.name){
+msg+=`. Received function ${actual.name}`;
+}elseif(typeofactual==='object'){
+if(actual.constructor&&actual.constructor.name){
+msg+=`. Received an instance of ${actual.constructor.name}`;
+}else{
+constinspected=lazyInternalUtilInspect()
+.inspect(actual,{depth: -1});
+msg+=`. Received ${inspected}`;
+}
+}else{
+letinspected=lazyInternalUtilInspect()
+.inspect(actual,{colors: false});
+if(inspected.length>25)
+inspected=`${inspected.slice(0,25)}...`;
+msg+=`. Received type ${typeofactual} (${inspected})`;
+}
 returnmsg;
 },TypeError);
 E('ERR_INVALID_ARG_VALUE',(name,value,reason='is invalid')=>{
@@ -1042,7 +1124,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(typeofexpected==='string')
+expected=[expected];
+assert(expected.length<=2);
+constres=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.
+functioninvalidArgTypeHelper(input){
+if(input==null){
+return` Received ${input}`;
+}
+if(typeofinput==='function'&&input.name){
+return` Received function ${input.name}`;
+}
+if(typeofinput==='object'){
+if(input.constructor&&input.constructor.name){
+return` Received an instance of ${input.constructor.name}`;
+}
+return` Received ${util.inspect(input,{depth: -1})}`;
+}
+letinspected=util.inspect(input,{colors: false});
+if(inspected.length>25)
+inspected=`${inspected.slice(0,25)}...`;
+return` Received type ${typeofinput} (${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 ${typeofrrtype}`
+`Received type ${typeofrrtype} (${rrtype})`
 }
 );
 }

test/parallel/test-assert-async.js

@@ -109,8 +109,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')"
 }
 ));
 
@@ -209,8 +209,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

@@ -403,8 +403,8 @@ assert.throws(
 {
 code: 'ERR_INVALID_ARG_TYPE',
 type: TypeError,
-message: 'The "fn" argument must be of type Function. Received '+
-`type ${typeoffn}`
+message: 'The "fn" argument must be of type function.'+
+common.invalidArgTypeHelper(fn)
 }
 );
 };
@@ -473,8 +473,8 @@ assert.throws(() =>{
 {
 code: 'ERR_INVALID_ARG_TYPE',
 name: 'TypeError',
-message: 'The "options" argument must be of type Object. '+
-`Received type ${typeofinput}`
+message: 'The "options" argument must be of type object.'+
+common.invalidArgTypeHelper(input)
 });
 });
 }
@@ -920,8 +920,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')"
 }
 );
 
@@ -934,8 +935,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 ${typeofinput}`
+message: 'The "error" argument must be of type function or '+
+'an instance of Error, RegExp, or Object.'+
+common.invalidArgTypeHelper(input)
 }
 );
 });
@@ -1013,8 +1015,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'
 }
 );
 

test/parallel/test-buffer-alloc.js

@@ -967,19 +967,19 @@ common.expectsError(
 {
 code: 'ERR_INVALID_ARG_TYPE',
 type: TypeError,
-message: 'The "target" argument must be one of type Buffer or Uint8Array.'+
-' Received type undefined'
+message: 'The "target" argument must be an instance of Buffer or '+
+'Uint8Array. Received undefined'
 });
 
 assert.throws(()=>Buffer.from(),{
 name: 'TypeError',
-message: 'The first argument must be one of type string, Buffer, '+
-'ArrayBuffer, Array, or Array-like Object. Received type undefined'
+message: 'The first argument must be of type string or an instance of '+
+'Buffer, ArrayBuffer, or Array or an Array-like Object. Received undefined'
 });
 assert.throws(()=>Buffer.from(null),{
 name: 'TypeError',
-message: 'The first argument must be one of type string, Buffer, '+
-'ArrayBuffer, Array, or Array-like Object. Received type object'
+message: 'The first argument must be of type string or an instance of '+
+'Buffer, ArrayBuffer, or Array or an Array-like Object. Received null'
 });
 
 // Test prototype getters don't throw

test/parallel/test-buffer-arraybuffer.js

@@ -43,8 +43,9 @@ assert.throws(function(){
 },{
 code: 'ERR_INVALID_ARG_TYPE',
 name: 'TypeError',
-message: 'The first argument must be one of type string, Buffer,'+
-' ArrayBuffer, Array, or Array-like Object. Received type object'
+message: 'The first argument must be of type string or an instance of '+
+'Buffer, ArrayBuffer, or Array or an Array-like Object. Received '+
+'an instance of AB'
 });
 
 // Test the byteOffset and length arguments