From cbf53ecaf2b6f16efd814c1b718ac8e4f8771858 Mon Sep 17 00:00:00 2001 From: Denys Otrishko Date: Fri, 7 Aug 2020 20:31:20 +0300 Subject: [PATCH] errors: improve ERR_INVALID_OPT_VALUE error * use util.inspect for value presentation * allow to optionally specify error reason PR-URL: https://github.com/nodejs/node/pull/34671 Reviewed-By: James M Snell Reviewed-By: Rich Trott Reviewed-By: Mary Marchini --- lib/internal/child_process.js | 4 +- lib/internal/errors.js | 11 ++++-- lib/net.js | 3 +- test/parallel/test-crypto-keygen.js | 37 ++++++++++++------- ...est-http2-client-request-options-errors.js | 3 +- .../test-http2-respond-file-errors.js | 3 +- .../test-http2-respond-file-fd-errors.js | 3 +- test/parallel/test-performanceobserver.js | 2 +- test/parallel/test-streams-highwatermark.js | 5 ++- 9 files changed, 45 insertions(+), 26 deletions(-) diff --git a/lib/internal/child_process.js b/lib/internal/child_process.js index 85ce63a9660f86..1295950f5d51ae 100644 --- 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 index c8d681e5356387..de5bf7dc7ca1f4 100644 --- a/lib/internal/errors.js +++ b/lib/internal/errors.js @@ -1106,10 +1106,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 index f1dcff22463c54..aebe9418b62c85 100644 --- 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; }); @@ -1488,7 +1487,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 index 1f059c469419fc..384f4fa68a2d02 100644 --- 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 index 365381cc5ab9d0..557b585d2e7200 100644 --- 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 index 1f2b227f1f501d..1b0a538554c1d0 100644 --- 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 index 750f762a53eb37..f06253aefe6307 100644 --- 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 index 3b2f94411f3df3..f1c6fce970a84f 100644 --- 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 index cd13f2af695cfd..2228c12f1421f5 100644 --- 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,6 +21,8 @@ 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 }); @@ -27,7 +30,7 @@ const stream = require('stream'); name: 'TypeError', code: 'ERR_INVALID_OPT_VALUE', message: - `The value "${invalidHwm}" is invalid for option "highWaterMark"` + `The value "${expected}" is invalid for option "highWaterMark"` }); } }