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 all commits
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
82 changes: 71 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,58 @@ 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.

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

The `'ERR_SETTING_SERVERS'` error code is used generically to identify that
an error occured setting the DNS servers


[`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 +621,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?

45 changes: 25 additions & 20 deletions lib/dns.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ const util = require('util');
const cares = process.binding('cares_wrap');
const uv = process.binding('uv');
const internalNet = require('internal/net');
const errors = require('internal/errors');

const GetAddrInfoReqWrap = cares.GetAddrInfoReqWrap;
const GetNameInfoReqWrap = cares.GetNameInfoReqWrap;
Expand All @@ -18,8 +19,8 @@ function errnoException(err, syscall, hostname) {
// FIXME(bnoordhuis) Remove this backwards compatibility nonsense and pass
// the true error to the user. ENOTFOUND is not even a proper POSIX error!
if (err === uv.UV_EAI_MEMORY ||
err === uv.UV_EAI_NODATA ||
err === uv.UV_EAI_NONAME) {
err === uv.UV_EAI_NODATA ||
err === uv.UV_EAI_NONAME) {
Copy link
Contributor

Choose a reason for hiding this comment

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

This should be reverted, the conditionals should be lined up.

err = 'ENOTFOUND';
}
var ex = null;
Expand Down Expand Up @@ -106,30 +107,32 @@ 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']);
} else if (typeof options === 'function') {
callback = options;
family = 0;
} else if (typeof callback !== 'function') {
throw new TypeError('Invalid arguments: callback must be passed');
// throw new TypeError('Invalid arguments: callback must be passed');
Copy link
Contributor

Choose a reason for hiding this comment

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

This needs to be removed.

throw new errors.TypeError('ERR_INVALID_CALLBACK');
} else if (options !== null && typeof options === 'object') {
hints = options.hints >>> 0;
family = options.family >>> 0;
all = options.all === true;

if (hints !== 0 &&
hints !== cares.AI_ADDRCONFIG &&
hints !== cares.AI_V4MAPPED &&
hints !== (cares.AI_ADDRCONFIG | cares.AI_V4MAPPED)) {
throw new TypeError('Invalid argument: hints must use valid flags');
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 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',
'family', ['4', '6']);

callback = makeAsync(callback);

Expand Down Expand Up @@ -180,16 +183,16 @@ 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_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);

if (!isLegalPort(port))
throw new TypeError(`"port" should be >= 0 and < 65536, got "${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;
callback = makeAsync(callback);
Expand Down Expand Up @@ -230,9 +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.TypeError('ERR_INVALID_CALLBACK');
}

callback = makeAsync(callback);
Expand Down Expand Up @@ -272,13 +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', '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');
}
}

Expand Down Expand Up @@ -313,7 +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);
});

const errorNumber = cares.setServers(newSet);
Expand All @@ -323,7 +328,7 @@ 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_SETTINGS_SERVERS', err, servers);
}
}

Expand Down
59 changes: 59 additions & 0 deletions lib/internal/errors.js
Original file line number Diff line number Diff line change
Expand Up @@ -85,4 +85,63 @@ 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_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}`);
E('ERR_SETTING_SERVERS',
(err, servers) => `c-ares failed to set servers: "${err}" [${servers}]`);
// Add new errors from here...

// 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?


function invalidArgType(name, expected, actual) {
const assert = lazyAssert();
assert(name, 'name is required');
assert(expected, 'expected is required');
var msg = `The "${name}" argument must be `;
if (Array.isArray(expected)) {
var len = expected.length;
expected = expected.map((i) => String(i));
if (len > 1) {
msg += `one of type ${expected.slice(0, len - 1).join(', ')}, or ` +
expected[len - 1];
} else {
msg += `type ${expected[0]}`;
}
} else {
msg += `type ${String(expected)}`;
}
if (arguments.length >= 3) {
msg += `. Received type ${actual !== null ? typeof actual : 'null'}`;
}
return msg;
}

function invalidArgValue(name, expected, actual) {
const assert = lazyAssert();
assert(name, 'name is required');
assert(expected, 'expected is required');
var msg = `The "${name}" argument must be `;
if (Array.isArray(expected)) {
var len = expected.length;
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];
} else {
msg += `${expected[0]}`;
}
} else {
msg += `${String(expected)}`;
}
if (arguments.length >= 3) {
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