Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

dns, net, internal/net: migrate to internals/error #11738

Closed
wants to merge 3 commits into from
Closed
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
76 changes: 65 additions & 11 deletions doc/api/errors.md
Original file line number Diff line number Diff line change
Expand Up @@ -243,7 +243,7 @@ new MyError().stack;

### Error.stackTraceLimit

* {Number}
* {number}

The `Error.stackTraceLimit` property specifies the number of stack frames
collected by a stack trace (whether generated by `new Error().stack` or
Expand All @@ -255,9 +255,16 @@ will affect any stack trace captured *after* the value has been changed.
If set to a non-number value, or set to a negative number, stack traces will
not capture any frames.

### error.message
#### error.code
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is this being duplicated? error.code already exists below.


* {string}

The `error.code` property is a string label that identifies the kind of error.
See [Node.js Error Codes][] for details about specific codes.

* {String}
#### error.message

* {string}

The `error.message` property is the string description of the error as set by calling `new Error(message)`.
The `message` passed to the constructor will also appear in the first line of
Expand All @@ -273,7 +280,7 @@ console.error(err.message);

### error.stack

* {String}
* {string}

The `error.stack` property is a string describing the point in the code at which
the `Error` was instantiated.
Expand Down Expand Up @@ -449,14 +456,14 @@ added properties.

#### error.code

* {String}
* {string}

The `error.code` property is a string representing the error code, which is always
`E` followed by a sequence of capital letters.

#### error.errno

* {String | Number}
* {string | number}

The `error.errno` property is a number or a string.
The number is a **negative** value which corresponds to the error code defined in
Expand All @@ -466,27 +473,27 @@ In case of a string, it is the same as `error.code`.

#### error.syscall

* {String}
* {string}

The `error.syscall` property is a string describing the [syscall][] that failed.

#### error.path

* {String}
* {string}

When present (e.g. in `fs` or `child_process`), the `error.path` property is a string
containing a relevant invalid pathname.

#### error.address

* {String}
* {string}

When present (e.g. in `net` or `dgram`), the `error.address` property is a string
describing the address to which the connection failed.

#### error.port

* {Number}
* {number}
Copy link
Contributor

@mscdex mscdex Mar 8, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These kinds of changes should be in a separate commit.


When present (e.g. in `net` or `dgram`), the `error.port` property is a number representing
the connection's port that is not available.
Expand Down Expand Up @@ -550,6 +557,52 @@ found [here][online].
encountered by [`http`][] or [`net`][] -- often a sign that a `socket.end()`
was not properly called.

<a id="nodejs-error-codes"></a>
## Node.js Error Codes

<a id="ERR_INVALID_ARGS"></a>
### ERR_INVALID_ARGS

The `'ERR_INVALID_ARGS'` error code is used generically to identify that
one or more arguments were passed wrong to a Node.js API.

<a id="ERR_INVALID_ARG_TYPE"></a>
### ERR_INVALID_ARG_TYPE

The `'ERR_INVALID_ARG_TYPE'` error code is used generically to identify that
an argument of the wrong type has been passed to a Node.js API.

<a id="ERR_INVALID_ARG_VALUE"></a>
### ERR_INVALID_ARG_VALUE

The `'ERR_INVALID_ARG_VALUE'` error code is used generically to identify that
an argument containing the wrong value was passed to a Node.js API.

<a id="ERR_INVALID_CALLBACK"></a>
### ERR_INVALID_CALLBACK

The `'ERR_INVALID_CALLBACK'` error code is used generically to identify that
a callback function is required and has not been provided to a Node.js API.

<a id="ERR_INVALID_FLAG"></a>
### ERR_INVALID_FLAG

The `'ERR_INVALID_FLAG'` error code is used generically to identify that
a invalid flag was passed as an argument to a Node.js API.

<a id="ERR_INVALID_IP"></a>
### ERR_INVALID_IP

The `'ERR_INVALID_IP'` error code is used generically to identify that
a invalid value was passed as an IP to a Node.js API.

<a id="ERR_INVALID_PORT"></a>
### ERR_INVALID_PORT

The `'ERR_INVALID_PORT'` error code is used generically to identify that
a invalid value was passed as a PORT to a Node.js API.


[`fs.readdir`]: fs.html#fs_fs_readdir_path_options_callback
[`fs.readFileSync`]: fs.html#fs_fs_readfilesync_file_options
[`fs.unlink`]: fs.html#fs_fs_unlink_path_callback
Expand All @@ -562,9 +615,10 @@ found [here][online].
[domains]: domain.html
[event emitter-based]: events.html#events_class_eventemitter
[file descriptors]: https://en.wikipedia.org/wiki/File_descriptor
[Node.js Error Codes]: #nodejs-error-codes
[online]: http://man7.org/linux/man-pages/man3/errno.3.html
[stream-based]: stream.html
[syscall]: http://man7.org/linux/man-pages/man2/syscall.2.html
[try-catch]: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Statements/try...catch
[V8's stack trace API]: https://github.com/v8/v8/wiki/Stack-Trace-API
[vm]: vm.html
[vm]: vm.html
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's this change?

40 changes: 16 additions & 24 deletions lib/dns.js
Original file line number Diff line number Diff line change
Expand Up @@ -107,8 +107,8 @@ function lookup(hostname, options, callback) {

// Parse arguments
if (hostname && typeof hostname !== 'string') {
// throw new TypeError('Invalid arguments: ' + 'hostname must be a string or falsey');
throw new errors.TypeError('ERR_INVALID_ARG_TYPE', 'hostname', ['string', 'buffer']);
throw new errors.TypeError('ERR_INVALID_ARG_TYPE',
'hostname', ['string', 'buffer']);
} else if (typeof options === 'function') {
callback = options;
family = 0;
Expand All @@ -124,16 +124,15 @@ function lookup(hostname, options, callback) {
hints !== cares.AI_ADDRCONFIG &&
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ditto about reverting the indentation changes.

hints !== cares.AI_V4MAPPED &&
hints !== (cares.AI_ADDRCONFIG | cares.AI_V4MAPPED)) {
// throw new TypeError('Invalid argument: hints must use valid flags');
throw new errors.Error('ERR_INVALID_FLAGS', 'options.hints');
throw new errors.Error('ERR_INVALID_FLAG', 'hints');
}
} else {
family = options >>> 0;
}

if (family !== 0 && family !== 4 && family !== 6)
// throw new TypeError('Invalid argument: family must be 4 or 6');
throw new errors.Error('ERR_INVALID_ARG_VALUE', 'options.family', ['4', '6']);
throw new errors.Error('ERR_INVALID_ARG_VALUE',
'family', ['4', '6']);

callback = makeAsync(callback);

Expand Down Expand Up @@ -184,19 +183,15 @@ function onlookupservice(err, host, service) {
// lookupService(address, port, callback)
function lookupService(host, port, callback) {
if (arguments.length !== 3)
// throw new Error('Invalid arguments');
throw new errors.Error('ERR_INSUFFICIENT_ARGS');
throw new errors.Error('ERR_INVALID_ARGS');

if (isIP(host) === 0)
// throw new TypeError('"host" argument needs to be a valid IP address');
throw new errors.Error('ERR_INVALID_IP', 'host', host)
throw new errors.Error('ERR_INVALID_IP', 'host', host);

if (!isLegalPort(port))
// throw new TypeError(`"port" should be >= 0 and < 65536, got "${port}"`);
throw new errors.Error('ERR_INVALID_PORT', 'port', port);
throw new errors.RangeError('ERR_INVALID_PORT', 'port', port);

if (typeof callback !== 'function')
// throw new TypeError('"callback" argument must be a function');
throw new errors.TypeError('ERR_INVALID_CALLBACK');

port = +port;
Expand Down Expand Up @@ -238,11 +233,9 @@ function resolver(bindingName) {
}

if (typeof name !== 'string') {
// throw new Error('"name" argument must be a string');
throw new errors.TypeError('ERR_INVALID_ARG_TYPE', 'name', 'string');
} else if (typeof callback !== 'function') {
// throw new Error('"callback" argument must be a function');
throw new errors.Error('ERR_INVALID_CALLBACK');
throw new errors.TypeError('ERR_INVALID_CALLBACK');
}

callback = makeAsync(callback);
Expand Down Expand Up @@ -282,15 +275,15 @@ function resolve(hostname, type_, callback_) {
resolver = resolveMap.A;
callback = type_;
} else {
// throw new Error('"type" argument must be a string');
throw new errors.TypeError('ERR_INVALID_ARG_TYPE', 'type', 'string');
throw new errors.TypeError('ERR_INVALID_ARG_TYPE',
'type', ['string', 'function']);
}

if (typeof resolver === 'function') {
return resolver(hostname, callback);
} else {
// throw new Error(`Unknown type "${type_}"`);
throw new errors.TypeError('ERR_INVALID_ARG_TYPE', 'resolver', 'function');
throw new errors.TypeError('ERR_INVALID_ARG_TYPE',
'resolver', 'function');
}
}

Expand Down Expand Up @@ -325,8 +318,7 @@ function setServers(servers) {
if (ipVersion !== 0)
return newSet.push([ipVersion, s]);

// throw new Error(`IP address is not properly formatted: ${serv}`);
throw new errors.Error('ERR_INVALID_IP', 'serv', serv)
throw new errors.Error('ERR_INVALID_IP', 'serv', serv);
});

const errorNumber = cares.setServers(newSet);
Expand All @@ -336,8 +328,8 @@ function setServers(servers) {
cares.setServers(orig.join(','));

var err = cares.strerror(errorNumber);
// throw new Error(`c-ares failed to set servers: "${err}" [${servers}]`);
throw new errors.Error('ERR_ASSERTION', `c-ares failed to set servers: "${err}" [${servers}]`);
throw new errors.Error('ERR_ASSERTION',
`c-ares failed to set servers: "${err}" [${servers}]`);
}
}

Expand Down
22 changes: 10 additions & 12 deletions lib/internal/errors.js
Original file line number Diff line number Diff line change
Expand Up @@ -85,18 +85,16 @@ module.exports = exports = {
//
// Note: Please try to keep these in alphabetical order
E('ERR_ASSERTION', (msg) => msg);
E('ERR_INVALID_ARGS', 'invalid arguments');
E('ERR_INVALID_ARG_TYPE', invalidArgType);
E('ERR_INVALID_ARG_VALUE', invalidArgValue);
E('ERR_INVALID_CALLBACK', 'callback must be a function');
E('ERR_STDERR_CLOSE', 'process.stderr cannot be closed');
E('ERR_STDOUT_CLOSE', 'process.stdout cannot be closed');
E('ERR_UNKNOWN_STDIN_TYPE', 'Unknown stdin file type');
E('ERR_UNKNOWN_STREAM_TYPE', 'Unknown stream file type');
E('ERR_INVALID_FLAG', (arg) => `${arg} must use valid flags`);
E('ERR_INVALID_IP',
(arg, ip) => `"${arg}" argument must be a valid IP address, got ${ip}`);
E('ERR_INVALID_PORT',
(arg, port) => `"${arg}" argument must be >= 0 and < 65536, got ${port}`);
// Add new errors from here...
E('ERR_INVALID_ARG_VALUE', invalidArgValue);
E('ERR_INVALID_FLAGS', arg => `${arg} must use valid flags`);
E('ERR_INSUFFICIENT_ARGS', 'insufficient arguments were passed to function');
E('ERR_INVALID_PORT', (arg, port) => `"${arg}" argument must be >= 0 and < 65536, got ${port}`);
E('ERR_INVALID_IP', (arg, ip) => `"${arg}" argument must be a valid IP address, got ${ip}`);

// Errors from 111294, port error from 11302
Copy link
Contributor

@mscdex mscdex Mar 8, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think 111294 here should be 11294 instead? Also it's a little bit vague about what this comment is referring to, perhaps it can just be removed?


Expand All @@ -110,7 +108,7 @@ function invalidArgType(name, expected, actual) {
expected = expected.map((i) => String(i));
if (len > 1) {
msg += `one of type ${expected.slice(0, len - 1).join(', ')}, or ` +
expected[len - 1];
expected[len - 1];
} else {
msg += `type ${expected[0]}`;
}
Expand All @@ -133,7 +131,7 @@ function invalidArgValue(name, expected, actual) {
expected = expected.map((i) => String(i));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Avoid these functional Array methods for performance.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ditto about Array methods.

if (len > 1) {
msg += `${expected.slice(0, len - 1).join(', ')}, or ` +
expected[len - 1];
expected[len - 1];
} else {
msg += `${expected[0]}`;
}
Expand All @@ -144,4 +142,4 @@ function invalidArgValue(name, expected, actual) {
msg += `. Received ${actual !== null ? typeof actual : 'null'}`;
}
return msg;
}
}
4 changes: 3 additions & 1 deletion lib/internal/net.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
'use strict';

const errors = require('internal/errors');

// Check that the port number is not NaN when coerced to a number,
// is an integer and that it falls within the legal range of port numbers.
function isLegalPort(port) {
Expand All @@ -12,7 +14,7 @@ function isLegalPort(port) {

function assertPort(port) {
if (typeof port !== 'undefined' && !isLegalPort(port))
throw new RangeError('"port" argument must be >= 0 and < 65536');
throw new errors.RangeError('ERR_INVALID_PORT', 'port', port);
}

module.exports = {
Expand Down
Loading