errors: improve ERR_INVALID_OPT_VALUE error

* use util.inspect for value presentation * allow to optionally specify error reason PR-URL: #34671 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: Mary Marchini <oss@mmarchini.me>
nodejs · Aug 15, 2020 · 8da8ec9 · 8da8ec9
1 parent 0347574
commit 8da8ec9
Show file tree

Hide file tree

Showing 9 changed files with 45 additions and 26 deletions.
diff --git a/lib/internal/child_process.js b/lib/internal/child_process.js
@@ -938,7 +938,7 @@ function getValidStdio(stdio, sync) {
   if (typeof stdio === 'string') {
     stdio = stdioStringToArray(stdio);
   } else if (!ArrayIsArray(stdio)) {
-    throw new ERR_INVALID_OPT_VALUE('stdio', inspect(stdio));
+    throw new ERR_INVALID_OPT_VALUE('stdio', stdio);
   }
 
   // At least 3 stdio will be created
@@ -1023,7 +1023,7 @@ function getValidStdio(stdio, sync) {
     } else {
       // Cleanup
       cleanup();
-      throw new ERR_INVALID_OPT_VALUE('stdio', inspect(stdio));
+      throw new ERR_INVALID_OPT_VALUE('stdio', stdio);
     }
 
     return acc;

diff --git a/lib/internal/errors.js b/lib/internal/errors.js
@@ -1112,10 +1112,13 @@ E('ERR_INVALID_MODULE_SPECIFIER', (request, reason, base = undefined) => {
   return `Invalid module "${request}" ${reason}${base ?
     ` imported from ${base}` : ''}`;
 }, TypeError);
-E('ERR_INVALID_OPT_VALUE', (name, value) =>
-  `The value "${String(value)}" is invalid for option "${name}"`,
-  TypeError,
-  RangeError);
+E('ERR_INVALID_OPT_VALUE', (name, value, reason = '') => {
+  let inspected = typeof value === 'string' ?
+    value : lazyInternalUtilInspect().inspect(value);
+  if (inspected.length > 128) inspected = `${inspected.slice(0, 128)}...`;
+  if (reason) reason = '. ' + reason;
+  return `The value "${inspected}" is invalid for option "${name}"` + reason;
+}, TypeError, RangeError);
 E('ERR_INVALID_OPT_VALUE_ENCODING',
   'The value "%s" is invalid for option "encoding"', TypeError);
 E('ERR_INVALID_PACKAGE_CONFIG', (path, base, message) => {

diff --git a/lib/net.js b/lib/net.js
@@ -34,7 +34,6 @@ const {
 
 const EventEmitter = require('events');
 const stream = require('stream');
-const { inspect } = require('internal/util/inspect');
 let debug = require('internal/util/debuglog').debuglog('net', (fn) => {
   debug = fn;
 });
@@ -1461,7 +1460,7 @@ Server.prototype.listen = function(...args) {
                                     'must have the property "port" or "path"');
   }
 
-  throw new ERR_INVALID_OPT_VALUE('options', inspect(options));
+  throw new ERR_INVALID_OPT_VALUE('options', options);
 };
 
 function lookupAndListen(self, port, address, backlog, exclusive, flags) {

diff --git a/test/parallel/test-crypto-keygen.js b/test/parallel/test-crypto-keygen.js
@@ -16,7 +16,7 @@ const {
   sign,
   verify
 } = require('crypto');
-const { promisify } = require('util');
+const { inspect, promisify } = require('util');
 
 // Asserts that the size of the given key (in chars or bytes) is within 10% of
 // the expected size.
@@ -705,13 +705,14 @@ const sec1EncExp = (cipher) => getRegExpForPEM('EC PRIVATE KEY', cipher);
     }), {
       name: 'TypeError',
       code: 'ERR_INVALID_OPT_VALUE',
-      message: `The value "${type}" is invalid for option ` +
+      message: `The value "${inspect(type)}" is invalid for option ` +
                '"publicKeyEncoding.type"'
     });
   }
 
   // Missing / invalid publicKeyEncoding.format.
   for (const format of [undefined, null, 0, false, 'a', {}]) {
+    const expected = typeof format === 'string' ? format : inspect(format);
     assert.throws(() => generateKeyPairSync('rsa', {
       modulusLength: 4096,
       publicKeyEncoding: {
@@ -725,7 +726,7 @@ const sec1EncExp = (cipher) => getRegExpForPEM('EC PRIVATE KEY', cipher);
     }), {
       name: 'TypeError',
       code: 'ERR_INVALID_OPT_VALUE',
-      message: `The value "${format}" is invalid for option ` +
+      message: `The value "${expected}" is invalid for option ` +
                '"publicKeyEncoding.format"'
     });
   }
@@ -761,13 +762,14 @@ const sec1EncExp = (cipher) => getRegExpForPEM('EC PRIVATE KEY', cipher);
     }), {
       name: 'TypeError',
       code: 'ERR_INVALID_OPT_VALUE',
-      message: `The value "${type}" is invalid for option ` +
+      message: `The value "${inspect(type)}" is invalid for option ` +
                '"privateKeyEncoding.type"'
     });
   }
 
   // Missing / invalid privateKeyEncoding.format.
   for (const format of [undefined, null, 0, false, 'a', {}]) {
+    const expected = typeof format === 'string' ? format : inspect(format);
     assert.throws(() => generateKeyPairSync('rsa', {
       modulusLength: 4096,
       publicKeyEncoding: {
@@ -781,7 +783,7 @@ const sec1EncExp = (cipher) => getRegExpForPEM('EC PRIVATE KEY', cipher);
     }), {
       name: 'TypeError',
       code: 'ERR_INVALID_OPT_VALUE',
-      message: `The value "${format}" is invalid for option ` +
+      message: `The value "${expected}" is invalid for option ` +
                '"privateKeyEncoding.format"'
     });
   }
@@ -802,7 +804,7 @@ const sec1EncExp = (cipher) => getRegExpForPEM('EC PRIVATE KEY', cipher);
     }), {
       name: 'TypeError',
       code: 'ERR_INVALID_OPT_VALUE',
-      message: `The value "${cipher}" is invalid for option ` +
+      message: `The value "${inspect(cipher)}" is invalid for option ` +
                '"privateKeyEncoding.cipher"'
     });
   }
@@ -865,25 +867,29 @@ const sec1EncExp = (cipher) => getRegExpForPEM('EC PRIVATE KEY', cipher);
 {
   // Test invalid modulus lengths.
   for (const modulusLength of [undefined, null, 'a', true, {}, [], 512.1, -1]) {
+    const expected = typeof modulusLength === 'string' ?
+      modulusLength : inspect(modulusLength);
     assert.throws(() => generateKeyPair('rsa', {
       modulusLength
     }), {
       name: 'TypeError',
       code: 'ERR_INVALID_OPT_VALUE',
-      message: `The value "${modulusLength}" is invalid for option ` +
+      message: `The value "${expected}" is invalid for option ` +
                '"modulusLength"'
     });
   }
 
   // Test invalid exponents.
   for (const publicExponent of ['a', true, {}, [], 3.5, -1]) {
+    const expected = typeof publicExponent === 'string' ?
+      publicExponent : inspect(publicExponent);
     assert.throws(() => generateKeyPair('rsa', {
       modulusLength: 4096,
       publicExponent
     }), {
       name: 'TypeError',
       code: 'ERR_INVALID_OPT_VALUE',
-      message: `The value "${publicExponent}" is invalid for option ` +
+      message: `The value "${expected}" is invalid for option ` +
                '"publicExponent"'
     });
   }
@@ -893,25 +899,29 @@ const sec1EncExp = (cipher) => getRegExpForPEM('EC PRIVATE KEY', cipher);
 {
   // Test invalid modulus lengths.
   for (const modulusLength of [undefined, null, 'a', true, {}, [], 4096.1]) {
+    const expected = typeof modulusLength === 'string' ?
+      modulusLength : inspect(modulusLength);
     assert.throws(() => generateKeyPair('dsa', {
       modulusLength
     }), {
       name: 'TypeError',
       code: 'ERR_INVALID_OPT_VALUE',
-      message: `The value "${modulusLength}" is invalid for option ` +
+      message: `The value "${expected}" is invalid for option ` +
                '"modulusLength"'
     });
   }
 
   // Test invalid divisor lengths.
   for (const divisorLength of ['a', true, {}, [], 4096.1]) {
+    const expected = typeof divisorLength === 'string' ?
+      divisorLength : inspect(divisorLength);
     assert.throws(() => generateKeyPair('dsa', {
       modulusLength: 2048,
       divisorLength
     }), {
       name: 'TypeError',
       code: 'ERR_INVALID_OPT_VALUE',
-      message: `The value "${divisorLength}" is invalid for option ` +
+      message: `The value "${expected}" is invalid for option ` +
                '"divisorLength"'
     });
   }
@@ -942,7 +952,7 @@ const sec1EncExp = (cipher) => getRegExpForPEM('EC PRIVATE KEY', cipher);
     }, {
       name: 'TypeError',
       code: 'ERR_INVALID_OPT_VALUE',
-      message: `The value "${namedCurve}" is invalid for option ` +
+      message: `The value "${inspect(namedCurve)}" is invalid for option ` +
                '"namedCurve"'
     });
   }
@@ -1079,7 +1089,7 @@ const sec1EncExp = (cipher) => getRegExpForPEM('EC PRIVATE KEY', cipher);
     }, {
       name: 'TypeError',
       code: 'ERR_INVALID_OPT_VALUE',
-      message: `The value "${hashValue}" is invalid for option "hash"`
+      message: `The value "${inspect(hashValue)}" is invalid for option "hash"`
     });
   }
 
@@ -1182,6 +1192,7 @@ const sec1EncExp = (cipher) => getRegExpForPEM('EC PRIVATE KEY', cipher);
   );
 
   for (const mgf1Hash of [null, 0, false, {}, []]) {
+    const expected = inspect(mgf1Hash);
     assert.throws(
       () => {
         generateKeyPair('rsa-pss', {
@@ -1194,7 +1205,7 @@ const sec1EncExp = (cipher) => getRegExpForPEM('EC PRIVATE KEY', cipher);
       {
         name: 'TypeError',
         code: 'ERR_INVALID_OPT_VALUE',
-        message: `The value "${mgf1Hash}" is invalid for option "mgf1Hash"`
+        message: `The value "${expected}" is invalid for option "mgf1Hash"`
       }
     );
   }

diff --git a/test/parallel/test-http2-client-request-options-errors.js b/test/parallel/test-http2-client-request-options-errors.js
@@ -5,6 +5,7 @@ if (!common.hasCrypto)
   common.skip('missing crypto');
 const assert = require('assert');
 const http2 = require('http2');
+const { inspect } = require('util');
 
 // Check if correct errors are emitted when wrong type of data is passed
 // to certain options of ClientHttp2Session request method
@@ -48,7 +49,7 @@ server.listen(0, common.mustCall(() => {
           }), {
             name: 'TypeError',
             code: 'ERR_INVALID_OPT_VALUE',
-            message: `The value "${String(types[type])}" is invalid ` +
+            message: `The value "${inspect(types[type])}" is invalid ` +
                     `for option "${option}"`
           });
       });

diff --git a/test/parallel/test-http2-respond-file-errors.js b/test/parallel/test-http2-respond-file-errors.js
@@ -6,6 +6,7 @@ if (!common.hasCrypto)
 const fixtures = require('../common/fixtures');
 const assert = require('assert');
 const http2 = require('http2');
+const { inspect } = require('util');
 
 const optionsWithTypeError = {
   offset: 'number',
@@ -45,7 +46,7 @@ server.on('stream', common.mustCall((stream) => {
         {
           name: 'TypeError',
           code: 'ERR_INVALID_OPT_VALUE',
-          message: `The value "${String(types[type])}" is invalid ` +
+          message: `The value "${inspect(types[type])}" is invalid ` +
                    `for option "${option}"`
         }
       );

diff --git a/test/parallel/test-http2-respond-file-fd-errors.js b/test/parallel/test-http2-respond-file-fd-errors.js
@@ -7,6 +7,7 @@ const fixtures = require('../common/fixtures');
 const assert = require('assert');
 const http2 = require('http2');
 const fs = require('fs');
+const { inspect } = require('util');
 
 const optionsWithTypeError = {
   offset: 'number',
@@ -65,7 +66,7 @@ server.on('stream', common.mustCall((stream) => {
         {
           name: 'TypeError',
           code: 'ERR_INVALID_OPT_VALUE',
-          message: `The value "${String(types[type])}" is invalid ` +
+          message: `The value "${inspect(types[type])}" is invalid ` +
                    `for option "${option}"`
         }
       );

diff --git a/test/parallel/test-performanceobserver.js b/test/parallel/test-performanceobserver.js
@@ -58,7 +58,7 @@ assert.strictEqual(counts[NODE_PERFORMANCE_ENTRY_TYPE_FUNCTION], 0);
                   {
                     code: 'ERR_INVALID_OPT_VALUE',
                     name: 'TypeError',
-                    message: `The value "${i}" is invalid ` +
+                    message: `The value "${inspect(i)}" is invalid ` +
                                    'for option "entryTypes"'
                   });
   });

diff --git a/test/parallel/test-streams-highwatermark.js b/test/parallel/test-streams-highwatermark.js
@@ -3,6 +3,7 @@ const common = require('../common');
 
 const assert = require('assert');
 const stream = require('stream');
+const { inspect } = require('util');
 
 {
   // This test ensures that the stream implementation correctly handles values
@@ -20,14 +21,16 @@ const stream = require('stream');
   assert.strictEqual(writable._writableState.highWaterMark, ovfl);
 
   for (const invalidHwm of [true, false, '5', {}, -5, NaN]) {
+    const expected = typeof invalidHwm === 'string' ?
+      invalidHwm : inspect(invalidHwm);
     for (const type of [stream.Readable, stream.Writable]) {
       assert.throws(() => {
         type({ highWaterMark: invalidHwm });
       }, {
         name: 'TypeError',
         code: 'ERR_INVALID_OPT_VALUE',
         message:
-          `The value "${invalidHwm}" is invalid for option "highWaterMark"`
+          `The value "${expected}" is invalid for option "highWaterMark"`
       });
     }
   }