Skip to content

Commit

Permalink
lib: remove ERR_INVALID_OPT_VALUE and ERR_INVALID_OPT_VALUE_ENCODING
Browse files Browse the repository at this point in the history
This will be a start to generalize all argument validation
errors. As currently we throw ARG/OPT, OUT_OF_RANGE, and other more
specific errors.
The OPT errors didn't bring much to the errors as it's just another
variant of ARG error which is sometimes more confusing (some of our code
used OPT errors to denote just argument validation errors presumably
because of similarity of OPT to 'option' and not 'options-object')
and they don't specify the name of the options object where the invalid
value is located. Much better approach would be to just specify path
to the invalid value in the name of the value as it is done in this PR
(i.e. 'options.format', 'options.publicKey.type' etc)

Also since this decreases a variety of errors we have it'd be easier to
reuse validation code across the codebase.

Refs: #31251
Refs: #34070 (comment)
Signed-off-by: Denys Otrishko <shishugi@gmail.com>

PR-URL: #34682
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
  • Loading branch information
lundibundi committed Sep 11, 2020
1 parent 8a8ca4b commit c66e647
Show file tree
Hide file tree
Showing 63 changed files with 315 additions and 325 deletions.
21 changes: 17 additions & 4 deletions doc/api/buffer.md
Original file line number Diff line number Diff line change
Expand Up @@ -288,6 +288,10 @@ It can be constructed in a variety of ways.
<!-- YAML
added: v5.10.0
changes:
- version: REPLACEME
pr-url: https://github.com/nodejs/node/pull/34682
description: Throw ERR_INVALID_ARG_VALUE instead of ERR_INVALID_OPT_VALUE
for invalid input arguments.
- version: v10.0.0
pr-url: https://github.com/nodejs/node/pull/18129
description: Attempting to fill a non-zero length buffer with a zero length
Expand Down Expand Up @@ -319,7 +323,7 @@ console.log(buf);
```

If `size` is larger than
[`buffer.constants.MAX_LENGTH`][] or smaller than 0, [`ERR_INVALID_OPT_VALUE`][]
[`buffer.constants.MAX_LENGTH`][] or smaller than 0, [`ERR_INVALID_ARG_VALUE`][]
is thrown.

If `fill` is specified, the allocated `Buffer` will be initialized by calling
Expand Down Expand Up @@ -353,6 +357,10 @@ A `TypeError` will be thrown if `size` is not a number.
<!-- YAML
added: v5.10.0
changes:
- version: REPLACEME
pr-url: https://github.com/nodejs/node/pull/34682
description: Throw ERR_INVALID_ARG_VALUE instead of ERR_INVALID_OPT_VALUE
for invalid input arguments.
- version: v7.0.0
pr-url: https://github.com/nodejs/node/pull/7079
description: Passing a negative `size` will now throw an error.
Expand All @@ -361,7 +369,7 @@ changes:
* `size` {integer} The desired length of the new `Buffer`.

Allocates a new `Buffer` of `size` bytes. If `size` is larger than
[`buffer.constants.MAX_LENGTH`][] or smaller than 0, [`ERR_INVALID_OPT_VALUE`][]
[`buffer.constants.MAX_LENGTH`][] or smaller than 0, [`ERR_INVALID_ARG_VALUE`][]
is thrown.

The underlying memory for `Buffer` instances created in this way is *not
Expand Down Expand Up @@ -401,12 +409,17 @@ additional performance that [`Buffer.allocUnsafe()`][] provides.
### Static method: `Buffer.allocUnsafeSlow(size)`
<!-- YAML
added: v5.12.0
changes:
- version: REPLACEME
pr-url: https://github.com/nodejs/node/pull/34682
description: Throw ERR_INVALID_ARG_VALUE instead of ERR_INVALID_OPT_VALUE
for invalid input arguments.
-->

* `size` {integer} The desired length of the new `Buffer`.

Allocates a new `Buffer` of `size` bytes. If `size` is larger than
[`buffer.constants.MAX_LENGTH`][] or smaller than 0, [`ERR_INVALID_OPT_VALUE`][]
[`buffer.constants.MAX_LENGTH`][] or smaller than 0, [`ERR_INVALID_ARG_VALUE`][]
is thrown. A zero-length `Buffer` is created if `size` is 0.

The underlying memory for `Buffer` instances created in this way is *not
Expand Down Expand Up @@ -3274,7 +3287,7 @@ introducing security vulnerabilities into an application.
[`Buffer.poolSize`]: #buffer_class_property_buffer_poolsize
[`DataView`]: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/DataView
[`ERR_INVALID_BUFFER_SIZE`]: errors.html#ERR_INVALID_BUFFER_SIZE
[`ERR_INVALID_OPT_VALUE`]: errors.html#ERR_INVALID_OPT_VALUE
[`ERR_INVALID_ARG_VALUE`]: errors.html#ERR_INVALID_ARG_VALUE
[`ERR_OUT_OF_RANGE`]: errors.html#ERR_OUT_OF_RANGE
[`JSON.stringify()`]: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/JSON/stringify
[`SharedArrayBuffer`]: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/SharedArrayBuffer
Expand Down
28 changes: 18 additions & 10 deletions doc/api/errors.md
Original file line number Diff line number Diff line change
Expand Up @@ -1378,16 +1378,6 @@ An IP address is not valid.
The imported module string is an invalid URL, package name, or package subpath
specifier.

<a id="ERR_INVALID_OPT_VALUE"></a>
### `ERR_INVALID_OPT_VALUE`

An invalid or unexpected value was passed in an options object.

<a id="ERR_INVALID_OPT_VALUE_ENCODING"></a>
### `ERR_INVALID_OPT_VALUE_ENCODING`

An invalid or unknown file encoding was passed.

<a id="ERR_INVALID_PACKAGE_CONFIG"></a>
### `ERR_INVALID_PACKAGE_CONFIG`

Expand Down Expand Up @@ -2391,6 +2381,24 @@ Used when an invalid character is found in an HTTP response status message
-->
A given index was out of the accepted range (e.g. negative offsets).

<a id="ERR_INVALID_OPT_VALUE"></a>
### `ERR_INVALID_OPT_VALUE`
<!-- YAML
added: v8.0.0
removed: REPLACEME
-->

An invalid or unexpected value was passed in an options object.

<a id="ERR_INVALID_OPT_VALUE_ENCODING"></a>
### `ERR_INVALID_OPT_VALUE_ENCODING`
<!-- YAML
added: v9.0.0
removed: REPLACEME
-->

An invalid or unknown file encoding was passed.

<a id="ERR_MISSING_MESSAGE_PORT_IN_TRANSFER_LIST"></a>
### `ERR_MISSING_MESSAGE_PORT_IN_TRANSFER_LIST`
<!-- YAML
Expand Down
2 changes: 1 addition & 1 deletion lib/_http_agent.js
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@ function Agent(options) {
this.maxTotalSockets = this.options.maxTotalSockets;
this.totalSocketCount = 0;

validateOneOf(this.scheduling, 'scheduling', ['fifo', 'lifo'], true);
validateOneOf(this.scheduling, 'scheduling', ['fifo', 'lifo']);

if (this.maxTotalSockets !== undefined) {
validateNumber(this.maxTotalSockets, 'maxTotalSockets');
Expand Down
10 changes: 5 additions & 5 deletions lib/_tls_common.js
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ const tls = require('tls');
const {
ERR_CRYPTO_CUSTOM_ENGINE_NOT_SUPPORTED,
ERR_INVALID_ARG_TYPE,
ERR_INVALID_OPT_VALUE,
ERR_INVALID_ARG_VALUE,
ERR_TLS_INVALID_PROTOCOL_VERSION,
ERR_TLS_PROTOCOL_VERSION_CONFLICT,
} = require('internal/errors').codes;
Expand Down Expand Up @@ -157,7 +157,7 @@ exports.createSecureContext = function createSecureContext(options) {
}

if (sigalgs === '') {
throw new ERR_INVALID_OPT_VALUE('sigalgs', sigalgs);
throw new ERR_INVALID_ARG_VALUE('options.sigalgs', sigalgs);
}

c.context.setSigalgs(sigalgs);
Expand All @@ -167,12 +167,12 @@ exports.createSecureContext = function createSecureContext(options) {
if (privateKeyIdentifier !== undefined) {
if (privateKeyEngine === undefined) {
// Engine is required when privateKeyIdentifier is present
throw new ERR_INVALID_OPT_VALUE('privateKeyEngine',
throw new ERR_INVALID_ARG_VALUE('options.privateKeyEngine',
privateKeyEngine);
}
if (key) {
// Both data key and engine key can't be set at the same time
throw new ERR_INVALID_OPT_VALUE('privateKeyIdentifier',
throw new ERR_INVALID_ARG_VALUE('options.privateKeyIdentifier',
privateKeyIdentifier);
}

Expand Down Expand Up @@ -210,7 +210,7 @@ exports.createSecureContext = function createSecureContext(options) {
if (cipherSuites === '' && cipherList === '') {
// Specifying empty cipher suites for both TLS1.2 and TLS1.3 is invalid, its
// not possible to handshake with no suites.
throw new ERR_INVALID_OPT_VALUE('ciphers', ciphers);
throw new ERR_INVALID_ARG_VALUE('options.ciphers', ciphers);
}

c.context.setCipherSuites(cipherSuites);
Expand Down
3 changes: 1 addition & 2 deletions lib/buffer.js
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,6 @@ const {
ERR_INVALID_ARG_TYPE,
ERR_INVALID_ARG_VALUE,
ERR_INVALID_BUFFER_SIZE,
ERR_INVALID_OPT_VALUE,
ERR_OUT_OF_RANGE,
ERR_UNKNOWN_ENCODING
},
Expand Down Expand Up @@ -342,7 +341,7 @@ const assertSize = hideStackFrames((size) => {
throw new ERR_INVALID_ARG_TYPE('size', 'number', size);
}
if (!(size >= 0 && size <= kMaxLength)) {
throw new ERR_INVALID_OPT_VALUE.RangeError('size', size);
throw new ERR_INVALID_ARG_VALUE.RangeError('size', size);
}
});

Expand Down
8 changes: 4 additions & 4 deletions lib/dns.js
Original file line number Diff line number Diff line change
Expand Up @@ -42,8 +42,8 @@ const {
} = require('internal/dns/utils');
const {
ERR_INVALID_ARG_TYPE,
ERR_INVALID_ARG_VALUE,
ERR_INVALID_CALLBACK,
ERR_INVALID_OPT_VALUE,
ERR_MISSING_ARGS,
} = errors.codes;
const {
Expand Down Expand Up @@ -115,7 +115,7 @@ function lookup(hostname, options, callback) {
family = options >>> 0;
}

validateOneOf(family, 'family', [0, 4, 6], true);
validateOneOf(family, 'family', [0, 4, 6]);

if (!hostname) {
emitInvalidHostnameWarning(hostname);
Expand Down Expand Up @@ -171,7 +171,7 @@ function lookupService(address, port, callback) {
throw new ERR_MISSING_ARGS('address', 'port', 'callback');

if (isIP(address) === 0)
throw new ERR_INVALID_OPT_VALUE('address', address);
throw new ERR_INVALID_ARG_VALUE('address', address);

validatePort(port);

Expand Down Expand Up @@ -262,7 +262,7 @@ function resolve(hostname, rrtype, callback) {
if (typeof resolver === 'function') {
return resolver.call(this, hostname, callback);
}
throw new ERR_INVALID_OPT_VALUE('rrtype', rrtype);
throw new ERR_INVALID_ARG_VALUE('rrtype', rrtype);
}

function defaultResolverSetServers(servers) {
Expand Down
10 changes: 5 additions & 5 deletions lib/internal/child_process.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,8 @@ const {
errnoException,
codes: {
ERR_INVALID_ARG_TYPE,
ERR_INVALID_ARG_VALUE,
ERR_INVALID_HANDLE_TYPE,
ERR_INVALID_OPT_VALUE,
ERR_INVALID_SYNC_FORK_INPUT,
ERR_IPC_CHANNEL_CLOSED,
ERR_IPC_DISCONNECTED,
Expand Down Expand Up @@ -226,7 +226,7 @@ function stdioStringToArray(stdio, channel) {
case 'pipe': options.push(stdio, stdio, stdio); break;
case 'inherit': options.push(0, 1, 2); break;
default:
throw new ERR_INVALID_OPT_VALUE('stdio', stdio);
throw new ERR_INVALID_ARG_VALUE('stdio', stdio);
}

if (channel) options.push(channel);
Expand Down Expand Up @@ -347,7 +347,7 @@ ChildProcess.prototype.spawn = function(options) {


validateOneOf(options.serialization, 'options.serialization',
[undefined, 'json', 'advanced'], true);
[undefined, 'json', 'advanced']);
const serialization = options.serialization || 'json';

if (ipc !== undefined) {
Expand Down Expand Up @@ -934,7 +934,7 @@ function getValidStdio(stdio, sync) {
if (typeof stdio === 'string') {
stdio = stdioStringToArray(stdio);
} else if (!ArrayIsArray(stdio)) {
throw new ERR_INVALID_OPT_VALUE('stdio', stdio);
throw new ERR_INVALID_ARG_VALUE('stdio', stdio);
}

// At least 3 stdio will be created
Expand Down Expand Up @@ -1019,7 +1019,7 @@ function getValidStdio(stdio, sync) {
} else {
// Cleanup
cleanup();
throw new ERR_INVALID_OPT_VALUE('stdio', stdio);
throw new ERR_INVALID_ARG_VALUE('stdio', stdio);
}

return acc;
Expand Down
4 changes: 2 additions & 2 deletions lib/internal/crypto/cipher.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ const {
const {
ERR_CRYPTO_INVALID_STATE,
ERR_INVALID_ARG_TYPE,
ERR_INVALID_OPT_VALUE
ERR_INVALID_ARG_VALUE
} = require('internal/errors').codes;
const { validateEncoding, validateString } = require('internal/validators');

Expand Down Expand Up @@ -87,7 +87,7 @@ function getUIntOption(options, key) {
let value;
if (options && (value = options[key]) != null) {
if (value >>> 0 !== value)
throw new ERR_INVALID_OPT_VALUE(key, value);
throw new ERR_INVALID_ARG_VALUE(`options.${key}`, value);
return value;
}
return -1;
Expand Down
6 changes: 3 additions & 3 deletions lib/internal/crypto/diffiehellman.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ const {
ERR_CRYPTO_INCOMPATIBLE_KEY,
ERR_CRYPTO_INVALID_KEY_OBJECT_TYPE,
ERR_INVALID_ARG_TYPE,
ERR_INVALID_OPT_VALUE
ERR_INVALID_ARG_VALUE,
} = require('internal/errors').codes;
const {
validateString,
Expand Down Expand Up @@ -258,10 +258,10 @@ function diffieHellman(options) {

const { privateKey, publicKey } = options;
if (!(privateKey instanceof KeyObject))
throw new ERR_INVALID_OPT_VALUE('privateKey', privateKey);
throw new ERR_INVALID_ARG_VALUE('options.privateKey', privateKey);

if (!(publicKey instanceof KeyObject))
throw new ERR_INVALID_OPT_VALUE('publicKey', publicKey);
throw new ERR_INVALID_ARG_VALUE('options.publicKey', publicKey);

if (privateKey.type !== 'private')
throw new ERR_CRYPTO_INVALID_KEY_OBJECT_TYPE(privateKey.type, 'private');
Expand Down
Loading

0 comments on commit c66e647

Please sign in to comment.