Skip to content

Commit

Permalink
errors: improve ERR_INVALID_OPT_VALUE error
Browse files Browse the repository at this point in the history
* 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>
  • Loading branch information
lundibundi authored and MylesBorins committed Aug 17, 2020
1 parent 88d4686 commit cbf53ec
Show file tree
Hide file tree
Showing 9 changed files with 45 additions and 26 deletions.
4 changes: 2 additions & 2 deletions lib/internal/child_process.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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;
Expand Down
11 changes: 7 additions & 4 deletions lib/internal/errors.js
Original file line number Diff line number Diff line change
Expand Up @@ -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) => {
Expand Down
3 changes: 1 addition & 2 deletions lib/net.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;
});
Expand Down Expand Up @@ -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) {
Expand Down
37 changes: 24 additions & 13 deletions test/parallel/test-crypto-keygen.js
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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: {
Expand All @@ -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"'
});
}
Expand Down Expand Up @@ -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: {
Expand All @@ -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"'
});
}
Expand All @@ -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"'
});
}
Expand Down Expand Up @@ -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"'
});
}
Expand All @@ -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"'
});
}
Expand Down Expand Up @@ -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"'
});
}
Expand Down Expand Up @@ -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"`
});
}

Expand Down Expand Up @@ -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', {
Expand All @@ -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"`
}
);
}
Expand Down
3 changes: 2 additions & 1 deletion test/parallel/test-http2-client-request-options-errors.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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}"`
});
});
Expand Down
3 changes: 2 additions & 1 deletion test/parallel/test-http2-respond-file-errors.js
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand Down Expand Up @@ -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}"`
}
);
Expand Down
3 changes: 2 additions & 1 deletion test/parallel/test-http2-respond-file-fd-errors.js
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand Down Expand Up @@ -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}"`
}
);
Expand Down
2 changes: 1 addition & 1 deletion test/parallel/test-performanceobserver.js
Original file line number Diff line number Diff line change
Expand Up @@ -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"'
});
});
Expand Down
5 changes: 4 additions & 1 deletion test/parallel/test-streams-highwatermark.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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"`
});
}
}
Expand Down

0 comments on commit cbf53ec

Please sign in to comment.