Skip to content

Commit

Permalink
errors: migrate dns to use internal/errors
Browse files Browse the repository at this point in the history
PR-URL: #14212
Refs: #11273
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Khaidi Chu <i@2333.moe>
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
  • Loading branch information
starkwang authored and refack committed Jul 24, 2017
1 parent 3566195 commit 9cb390d
Show file tree
Hide file tree
Showing 8 changed files with 147 additions and 56 deletions.
8 changes: 8 additions & 0 deletions doc/api/errors.md
Original file line number Diff line number Diff line change
Expand Up @@ -596,6 +596,10 @@ Used when `Console` is instantiated without `stdout` stream or when `stdout` or

Used when the native call from `process.cpuUsage` cannot be processed properly.

<a id="ERR_DNS_SET_SERVERS_FAILED"></a>

Used when `c-ares` failed to set the DNS server.

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

Expand Down Expand Up @@ -685,6 +689,10 @@ Used when an attempt is made to send an unsupported "handle" over an IPC
communication channel to a child process. See [`child.send()`] and
[`process.send()`] for more information.

<a id="ERR_INVALID_IP_ADDRESS"></a>

Used when an IP address is not valid.

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

Expand Down
33 changes: 18 additions & 15 deletions lib/dns.js
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ const cares = process.binding('cares_wrap');
const uv = process.binding('uv');
const internalNet = require('internal/net');
const { customPromisifyArgs } = require('internal/util');
const errors = require('internal/errors');

const GetAddrInfoReqWrap = cares.GetAddrInfoReqWrap;
const GetNameInfoReqWrap = cares.GetNameInfoReqWrap;
Expand Down Expand Up @@ -126,13 +127,13 @@ 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', 'falsey'], hostname);
} 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 errors.TypeError('ERR_INVALID_CALLBACK');
} else if (options !== null && typeof options === 'object') {
hints = options.hints >>> 0;
family = options.family >>> 0;
Expand All @@ -142,14 +143,14 @@ function lookup(hostname, options, callback) {
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');
throw new errors.TypeError('ERR_INVALID_OPT_VALUE', 'hints', 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.TypeError('ERR_INVALID_OPT_VALUE', 'family', family);

if (!hostname) {
if (all) {
Expand Down Expand Up @@ -200,16 +201,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.TypeError('ERR_MISSING_ARGS', 'host', 'port', 'callback');

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

if (!isLegalPort(port))
throw new TypeError(`"port" should be >= 0 and < 65536, got "${port}"`);
throw new errors.RangeError('ERR_SOCKET_BAD_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 @@ -250,9 +251,10 @@ 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', name);
} else if (typeof callback !== 'function') {
throw new Error('"callback" argument must be a function');
throw new errors.TypeError('ERR_INVALID_CALLBACK');
}

var req = new QueryReqWrap();
Expand Down Expand Up @@ -290,13 +292,14 @@ function resolve(hostname, rrtype, callback) {
resolver = resolveMap.A;
callback = rrtype;
} else {
throw new TypeError('"rrtype" argument must be a string');
throw new errors.TypeError('ERR_INVALID_ARG_TYPE', 'rrtype',
'string', rrtype);
}

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

Expand Down Expand Up @@ -344,7 +347,7 @@ function setServers(servers) {
return newSet.push([ipVersion, s, parseInt(p)]);
}

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

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

Expand Down
3 changes: 3 additions & 0 deletions lib/internal/errors.js
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,8 @@ E('ERR_CHILD_CLOSED_BEFORE_REPLY', 'Child closed before reply received');
E('ERR_CONSOLE_WRITABLE_STREAM',
'Console expects a writable stream instance for %s');
E('ERR_CPU_USAGE', 'Unable to obtain cpu usage %s');
E('ERR_DNS_SET_SERVERS_FAILED', (err, servers) =>
`c-ares failed to set servers: "${err}" [${servers}]`);
E('ERR_FALSY_VALUE_REJECTION', 'Promise was rejected with falsy value');
E('ERR_HTTP_HEADERS_SENT',
'Cannot render headers after they are sent to the client');
Expand All @@ -129,6 +131,7 @@ E('ERR_INVALID_FILE_URL_HOST',
'File URL host must be "localhost" or empty on %s');
E('ERR_INVALID_FILE_URL_PATH', 'File URL path %s');
E('ERR_INVALID_HANDLE_TYPE', 'This handle type cannot be sent');
E('ERR_INVALID_IP_ADDRESS', 'Invalid IP address: %s');
E('ERR_INVALID_OPT_VALUE',
(name, value) => {
return `The value "${String(value)}" is invalid for option "${name}"`;
Expand Down
20 changes: 15 additions & 5 deletions test/parallel/test-c-ares.js
Original file line number Diff line number Diff line change
Expand Up @@ -46,11 +46,21 @@ dns.lookup('::1', common.mustCall((error, result, addressType) => {
assert.strictEqual(6, addressType);
}));

// Try calling resolve with an unsupported type.
assert.throws(() => dns.resolve('www.google.com', 'HI'), /Unknown type/);

// Try calling resolve with an unsupported type that's an object key
assert.throws(() => dns.resolve('www.google.com', 'toString'), /Unknown type/);
[
// Try calling resolve with an unsupported type.
'HI',
// Try calling resolve with an unsupported type that's an object key
'toString'
].forEach((val) => {
common.expectsError(
() => dns.resolve('www.google.com', val),
{
code: 'ERR_INVALID_OPT_VALUE',
type: TypeError,
message: `The value "${val}" is invalid for option "rrtype"`
}
);
});

// Windows doesn't usually have an entry for localhost 127.0.0.1 in
// C:\Windows\System32\drivers\etc\hosts
Expand Down
28 changes: 23 additions & 5 deletions test/parallel/test-dns-lookup.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,31 +9,49 @@ cares.getaddrinfo = () => process.binding('uv').UV_ENOENT;

assert.throws(() => {
dns.lookup(1, {});
}, /^TypeError: Invalid arguments: hostname must be a string or falsey$/);
}, common.expectsError({
code: 'ERR_INVALID_ARG_TYPE',
type: TypeError,
message: /^The "hostname" argument must be one of type string or falsey/
}));

assert.throws(() => {
dns.lookup(false, 'cb');
}, /^TypeError: Invalid arguments: callback must be passed$/);
}, common.expectsError({
code: 'ERR_INVALID_CALLBACK',
type: TypeError
}));

assert.throws(() => {
dns.lookup(false, 'options', 'cb');
}, /^TypeError: Invalid arguments: callback must be passed$/);
}, common.expectsError({
code: 'ERR_INVALID_CALLBACK',
type: TypeError
}));

assert.throws(() => {
dns.lookup(false, {
hints: 100,
family: 0,
all: false
}, common.mustNotCall());
}, /^TypeError: Invalid argument: hints must use valid flags$/);
}, common.expectsError({
code: 'ERR_INVALID_OPT_VALUE',
type: TypeError,
message: 'The value "100" is invalid for option "hints"'
}));

assert.throws(() => {
dns.lookup(false, {
hints: 0,
family: 20,
all: false
}, common.mustNotCall());
}, /^TypeError: Invalid argument: family must be 4 or 6$/);
}, common.expectsError({
code: 'ERR_INVALID_OPT_VALUE',
type: TypeError,
message: 'The value "20" is invalid for option "family"'
}));

assert.doesNotThrow(() => {
dns.lookup(false, {
Expand Down
13 changes: 10 additions & 3 deletions test/parallel/test-dns-regress-7070.js
Original file line number Diff line number Diff line change
Expand Up @@ -20,12 +20,19 @@
// USE OR OTHER DEALINGS IN THE SOFTWARE.

'use strict';
require('../common');
const common = require('../common');
const assert = require('assert');
const dns = require('dns');

// Should not raise assertion error. Issue #7070
assert.throws(() => dns.resolveNs([]), // bad name
/^Error: "name" argument must be a string$/);
common.expectsError({
code: 'ERR_INVALID_ARG_TYPE',
type: TypeError,
message: /^The "name" argument must be of type string/
}));
assert.throws(() => dns.resolveNs(''), // bad callback
/^Error: "callback" argument must be a function$/);
common.expectsError({
code: 'ERR_INVALID_CALLBACK',
type: TypeError
}));
90 changes: 65 additions & 25 deletions test/parallel/test-dns.js
Original file line number Diff line number Diff line change
Expand Up @@ -68,10 +68,16 @@ const goog = [
];
assert.doesNotThrow(() => dns.setServers(goog));
assert.deepStrictEqual(dns.getServers(), goog);
assert.throws(() => dns.setServers(['foobar']),
/^Error: IP address is not properly formatted: foobar$/);
assert.throws(() => dns.setServers(['127.0.0.1:va']),
/^Error: IP address is not properly formatted: 127\.0\.0\.1:va$/);
assert.throws(() => dns.setServers(['foobar']), common.expectsError({
code: 'ERR_INVALID_IP_ADDRESS',
type: Error,
message: 'Invalid IP address: foobar'
}));
assert.throws(() => dns.setServers(['127.0.0.1:va']), common.expectsError({
code: 'ERR_INVALID_IP_ADDRESS',
type: Error,
message: 'Invalid IP address: 127.0.0.1:va'
}));
assert.deepStrictEqual(dns.getServers(), goog);

const goog6 = [
Expand Down Expand Up @@ -105,12 +111,20 @@ assert.deepStrictEqual(dns.getServers(), []);

assert.throws(() => {
dns.resolve('example.com', [], common.mustNotCall());
}, /^TypeError: "rrtype" argument must be a string$/);
}, common.expectsError({
code: 'ERR_INVALID_ARG_TYPE',
type: TypeError,
message: 'The "rrtype" argument must be of type string. ' +
'Received type object'
}));

// dns.lookup should accept only falsey and string values
{
const errorReg =
/^TypeError: Invalid arguments: hostname must be a string or falsey$/;
const errorReg = common.expectsError({
code: 'ERR_INVALID_ARG_TYPE',
type: TypeError,
message: /^The "hostname" argument must be one of type string or falsey/
}, 5);

assert.throws(() => dns.lookup({}, common.mustNotCall()), errorReg);

Expand Down Expand Up @@ -156,13 +170,21 @@ assert.throws(() => {
assert.throws(() => {
dns.lookup('nodejs.org', { hints: (dns.V4MAPPED | dns.ADDRCONFIG) + 1 },
common.mustNotCall());
}, /^TypeError: Invalid argument: hints must use valid flags$/);

assert.throws(() => dns.lookup('nodejs.org'),
/^TypeError: Invalid arguments: callback must be passed$/);

assert.throws(() => dns.lookup('nodejs.org', 4),
/^TypeError: Invalid arguments: callback must be passed$/);
}, common.expectsError({
code: 'ERR_INVALID_OPT_VALUE',
type: TypeError,
message: /The value "\d+" is invalid for option "hints"/
}));

assert.throws(() => dns.lookup('nodejs.org'), common.expectsError({
code: 'ERR_INVALID_CALLBACK',
type: TypeError
}));

assert.throws(() => dns.lookup('nodejs.org', 4), common.expectsError({
code: 'ERR_INVALID_CALLBACK',
type: TypeError
}));

assert.doesNotThrow(() => dns.lookup('', { family: 4, hints: 0 },
common.mustCall()));
Expand All @@ -183,25 +205,43 @@ assert.doesNotThrow(() => {
}, common.mustCall());
});

assert.throws(() => dns.lookupService('0.0.0.0'),
/^Error: Invalid arguments$/);

assert.throws(() => dns.lookupService('fasdfdsaf', 0, common.mustNotCall()),
/^TypeError: "host" argument needs to be a valid IP address$/);
assert.throws(() => dns.lookupService('0.0.0.0'), common.expectsError({
code: 'ERR_MISSING_ARGS',
type: TypeError,
message: 'The "host", "port", and "callback" arguments must be specified'
}));

const invalidHost = 'fasdfdsaf';
assert.throws(() => {
dns.lookupService(invalidHost, 0, common.mustNotCall());
}, common.expectsError({
code: 'ERR_INVALID_OPT_VALUE',
type: TypeError,
message: `The value "${invalidHost}" is invalid for option "host"`
}));

const badPortMsg = common.expectsError({
code: 'ERR_SOCKET_BAD_PORT',
type: RangeError,
message: 'Port should be > 0 and < 65536'
}, 4);
assert.throws(() => dns.lookupService('0.0.0.0', null, common.mustNotCall()),
/^TypeError: "port" should be >= 0 and < 65536, got "null"$/);
badPortMsg);

assert.throws(
() => dns.lookupService('0.0.0.0', undefined, common.mustNotCall()),
/^TypeError: "port" should be >= 0 and < 65536, got "undefined"$/
badPortMsg
);

assert.throws(() => dns.lookupService('0.0.0.0', 65538, common.mustNotCall()),
/^TypeError: "port" should be >= 0 and < 65536, got "65538"$/);
badPortMsg);

assert.throws(() => dns.lookupService('0.0.0.0', 'test', common.mustNotCall()),
/^TypeError: "port" should be >= 0 and < 65536, got "test"$/);
badPortMsg);

assert.throws(() => dns.lookupService('0.0.0.0', 80, null),
/^TypeError: "callback" argument must be a function$/);
assert.throws(() => {
dns.lookupService('0.0.0.0', 80, null);
}, common.expectsError({
code: 'ERR_INVALID_CALLBACK',
type: TypeError
}));
Loading

0 comments on commit 9cb390d

Please sign in to comment.