Skip to content

Commit

Permalink
net: convert to using internal/errors
Browse files Browse the repository at this point in the history
Covert lib/net.js over to using lib/internal/errors.js

- Replace thrown errors in lib/net.js
  with errors from lib/internal/errors.
  The ERR_INVALID_OPT_VALUE error have been used
  in the Server.prototype.listen() method
- Update tests according to the above modifications

PR-URL: #14782
Refs: #11273
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
  • Loading branch information
matzavinos authored and joyeecheung committed Oct 15, 2017
1 parent 2b76b5d commit 7f55349
Show file tree
Hide file tree
Showing 15 changed files with 168 additions and 125 deletions.
5 changes: 5 additions & 0 deletions doc/api/errors.md
Original file line number Diff line number Diff line change
Expand Up @@ -940,6 +940,11 @@ Used when `hostname` can not be parsed from a provided URL.

Used when a file descriptor ('fd') is not valid (e.g. it has a negative value).

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

Used when a file descriptor ('fd') type is not valid.

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

Expand Down
2 changes: 1 addition & 1 deletion lib/dgram.js
Original file line number Diff line number Diff line change
Expand Up @@ -417,7 +417,7 @@ Socket.prototype.send = function(buffer,

port = port >>> 0;
if (port === 0 || port > 65535)
throw new errors.RangeError('ERR_SOCKET_BAD_PORT');
throw new errors.RangeError('ERR_SOCKET_BAD_PORT', port);

// Normalize callback so it's either a function or undefined but not anything
// else.
Expand Down
2 changes: 1 addition & 1 deletion lib/dns.js
Original file line number Diff line number Diff line change
Expand Up @@ -216,7 +216,7 @@ function lookupService(host, port, callback) {
throw new errors.TypeError('ERR_INVALID_OPT_VALUE', 'host', host);

if (!isLegalPort(port))
throw new errors.RangeError('ERR_SOCKET_BAD_PORT');
throw new errors.RangeError('ERR_SOCKET_BAD_PORT', port);

if (typeof callback !== 'function')
throw new errors.TypeError('ERR_INVALID_CALLBACK');
Expand Down
3 changes: 2 additions & 1 deletion lib/internal/errors.js
Original file line number Diff line number Diff line change
Expand Up @@ -254,6 +254,7 @@ E('ERR_INVALID_CURSOR_POS',
'Cannot set cursor row without setting its column');
E('ERR_INVALID_DOMAIN_NAME', 'Unable to determine the domain name');
E('ERR_INVALID_FD', '"fd" must be a positive integer: %s');
E('ERR_INVALID_FD_TYPE', 'Unsupported fd type: %s');
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');
Expand Down Expand Up @@ -301,7 +302,7 @@ E('ERR_SERVER_ALREADY_LISTEN',
'Listen method has been called more than once without closing.');
E('ERR_SOCKET_ALREADY_BOUND', 'Socket is already bound');
E('ERR_SOCKET_BAD_BUFFER_SIZE', 'Buffer size must be a positive integer');
E('ERR_SOCKET_BAD_PORT', 'Port should be > 0 and < 65536');
E('ERR_SOCKET_BAD_PORT', 'Port should be > 0 and < 65536. Received %s.');
E('ERR_SOCKET_BAD_TYPE',
'Bad socket type specified. Valid types are: udp4, udp6');
E('ERR_SOCKET_BUFFER_SIZE',
Expand Down
42 changes: 28 additions & 14 deletions lib/net.js
Original file line number Diff line number Diff line change
Expand Up @@ -61,10 +61,10 @@ const normalizedArgsSymbol = internalNet.normalizedArgsSymbol;
function noop() {}

function createHandle(fd) {
var type = TTYWrap.guessHandleType(fd);
const type = TTYWrap.guessHandleType(fd);
if (type === 'PIPE') return new Pipe();
if (type === 'TCP') return new TCP();
throw new TypeError('Unsupported fd type: ' + type);
throw new errors.TypeError('ERR_INVALID_FD_TYPE', type);
}


Expand Down Expand Up @@ -695,8 +695,10 @@ protoGetter('localPort', function localPort() {

Socket.prototype.write = function(chunk, encoding, cb) {
if (typeof chunk !== 'string' && !(chunk instanceof Buffer)) {
throw new TypeError(
'Invalid data, chunk must be a string or buffer, not ' + typeof chunk);
throw new errors.TypeError('ERR_INVALID_ARG_TYPE',
'chunk',
['string', 'Buffer'],
chunk);
}
return stream.Duplex.prototype.write.apply(this, arguments);
};
Expand Down Expand Up @@ -1035,21 +1037,25 @@ function lookupAndConnect(self, options) {
var localPort = options.localPort;

if (localAddress && !cares.isIP(localAddress)) {
throw new TypeError('"localAddress" option must be a valid IP: ' +
localAddress);
throw new errors.TypeError('ERR_INVALID_IP_ADDRESS', localAddress);
}

if (localPort && typeof localPort !== 'number') {
throw new TypeError('"localPort" option should be a number: ' + localPort);
throw new errors.TypeError('ERR_INVALID_ARG_TYPE',
'options.localPort',
'number',
localPort);
}

if (typeof port !== 'undefined') {
if (typeof port !== 'number' && typeof port !== 'string') {
throw new TypeError('"port" option should be a number or string: ' +
port);
throw new errors.TypeError('ERR_INVALID_ARG_TYPE',
'options.port',
['number', 'string'],
port);
}
if (!isLegalPort(port)) {
throw new RangeError('"port" option should be >= 0 and < 65536: ' + port);
throw new errors.RangeError('ERR_SOCKET_BAD_PORT', port);
}
}
port |= 0;
Expand All @@ -1065,7 +1071,10 @@ function lookupAndConnect(self, options) {
}

if (options.lookup && typeof options.lookup !== 'function')
throw new TypeError('"lookup" option should be a function');
throw new errors.TypeError('ERR_INVALID_ARG_TYPE',
'options.lookup',
'function',
options.lookup);

var dnsopts = {
family: options.family,
Expand Down Expand Up @@ -1207,7 +1216,10 @@ function Server(options, connectionListener) {
this.on('connection', connectionListener);
}
} else {
throw new TypeError('options must be an object');
throw new errors.TypeError('ERR_INVALID_ARG_TYPE',
'options',
'object',
options);
}

this._connections = 0;
Expand Down Expand Up @@ -1465,7 +1477,7 @@ Server.prototype.listen = function(...args) {
var backlog;
if (typeof options.port === 'number' || typeof options.port === 'string') {
if (!isLegalPort(options.port)) {
throw new RangeError('"port" argument must be >= 0 and < 65536');
throw new errors.RangeError('ERR_SOCKET_BAD_PORT', options.port);
}
backlog = options.backlog || backlogFromArgs;
// start TCP server listening on host:port
Expand All @@ -1490,7 +1502,9 @@ Server.prototype.listen = function(...args) {
return this;
}

throw new Error('Invalid listen argument: ' + util.inspect(options));
throw new errors.Error('ERR_INVALID_OPT_VALUE',
'options',
util.inspect(options));
};

function lookupAndListen(self, port, address, backlog, exclusive) {
Expand Down
35 changes: 17 additions & 18 deletions test/parallel/test-dns.js
Original file line number Diff line number Diff line change
Expand Up @@ -220,24 +220,23 @@ assert.throws(() => {
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()),
badPortMsg);

assert.throws(
() => dns.lookupService('0.0.0.0', undefined, common.mustNotCall()),
badPortMsg
);

assert.throws(() => dns.lookupService('0.0.0.0', 65538, common.mustNotCall()),
badPortMsg);

assert.throws(() => dns.lookupService('0.0.0.0', 'test', common.mustNotCall()),
badPortMsg);
const portErr = (port) => {
common.expectsError(
() => {
dns.lookupService('0.0.0.0', port, common.mustNotCall());
},
{
code: 'ERR_SOCKET_BAD_PORT',
message:
`Port should be > 0 and < 65536. Received ${port}.`,
type: RangeError
}
);
};
portErr(null);
portErr(undefined);
portErr(65538);
portErr('test');

assert.throws(() => {
dns.lookupService('0.0.0.0', 80, null);
Expand Down
8 changes: 8 additions & 0 deletions test/parallel/test-internal-errors.js
Original file line number Diff line number Diff line change
Expand Up @@ -216,6 +216,10 @@ assert.strictEqual(
errors.message('ERR_INVALID_ARG_TYPE', [['a', 'b', 'c'], 'not d']),
'The "a", "b", "c" arguments must not be of type d');

// Test ERR_INVALID_FD_TYPE
assert.strictEqual(errors.message('ERR_INVALID_FD_TYPE', ['a']),
'Unsupported fd type: a');

// Test ERR_INVALID_URL_SCHEME
assert.strictEqual(errors.message('ERR_INVALID_URL_SCHEME', ['file']),
'The URL must be of scheme file');
Expand Down Expand Up @@ -246,6 +250,10 @@ assert.throws(
message: /^At least one arg needs to be specified$/
}));

// Test ERR_SOCKET_BAD_PORT
assert.strictEqual(
errors.message('ERR_SOCKET_BAD_PORT', [0]),
'Port should be > 0 and < 65536. Received 0.');

// Test ERR_TLS_CERT_ALTNAME_INVALID
assert.strictEqual(
Expand Down
65 changes: 35 additions & 30 deletions test/parallel/test-net-connect-options-port.js
Original file line number Diff line number Diff line change
Expand Up @@ -27,35 +27,33 @@ const net = require('net');

// Test wrong type of ports
{
function portTypeError(opt) {
const prefix = '"port" option should be a number or string: ';
const cleaned = opt.replace(/[\\^$.*+?()[\]{}|=!<>:-]/g, '\\$&');
return new RegExp(`^TypeError: ${prefix}${cleaned}$`);
}

syncFailToConnect(true, portTypeError('true'));
syncFailToConnect(false, portTypeError('false'));
syncFailToConnect([], portTypeError(''), true);
syncFailToConnect({}, portTypeError('[object Object]'), true);
syncFailToConnect(null, portTypeError('null'));
const portTypeError = common.expectsError({
code: 'ERR_INVALID_ARG_TYPE',
type: TypeError
}, 96);

syncFailToConnect(true, portTypeError);
syncFailToConnect(false, portTypeError);
syncFailToConnect([], portTypeError, true);
syncFailToConnect({}, portTypeError, true);
syncFailToConnect(null, portTypeError);
}

// Test out of range ports
{
function portRangeError(opt) {
const prefix = '"port" option should be >= 0 and < 65536: ';
const cleaned = opt.replace(/[\\^$.*+?()[\]{}|=!<>:-]/g, '\\$&');
return new RegExp(`^RangeError: ${prefix}${cleaned}$`);
}

syncFailToConnect('', portRangeError(''));
syncFailToConnect(' ', portRangeError(' '));
syncFailToConnect('0x', portRangeError('0x'), true);
syncFailToConnect('-0x1', portRangeError('-0x1'), true);
syncFailToConnect(NaN, portRangeError('NaN'));
syncFailToConnect(Infinity, portRangeError('Infinity'));
syncFailToConnect(-1, portRangeError('-1'));
syncFailToConnect(65536, portRangeError('65536'));
const portRangeError = common.expectsError({
code: 'ERR_SOCKET_BAD_PORT',
type: RangeError
}, 168);

syncFailToConnect('', portRangeError);
syncFailToConnect(' ', portRangeError);
syncFailToConnect('0x', portRangeError, true);
syncFailToConnect('-0x1', portRangeError, true);
syncFailToConnect(NaN, portRangeError);
syncFailToConnect(Infinity, portRangeError);
syncFailToConnect(-1, portRangeError);
syncFailToConnect(65536, portRangeError);
}

// Test invalid hints
Expand Down Expand Up @@ -129,33 +127,40 @@ function doConnect(args, getCb) {
];
}

function syncFailToConnect(port, regexp, optOnly) {
function syncFailToConnect(port, assertErr, optOnly) {
if (!optOnly) {
// connect(port, cb) and connect(port)
const portArgBlocks = doConnect([port], () => common.mustNotCall());
for (const block of portArgBlocks) {
assert.throws(block, regexp, `${block.name}(${port})`);
assert.throws(block,
assertErr,
`${block.name}(${port})`);
}

// connect(port, host, cb) and connect(port, host)
const portHostArgBlocks = doConnect([port, 'localhost'],
() => common.mustNotCall());
for (const block of portHostArgBlocks) {
assert.throws(block, regexp, `${block.name}(${port}, 'localhost')`);
assert.throws(block,
assertErr,
`${block.name}(${port}, 'localhost')`);
}
}
// connect({port}, cb) and connect({port})
const portOptBlocks = doConnect([{ port }],
() => common.mustNotCall());
for (const block of portOptBlocks) {
assert.throws(block, regexp, `${block.name}({port: ${port}})`);
assert.throws(block,
assertErr,
`${block.name}({port: ${port}})`);
}

// connect({port, host}, cb) and connect({port, host})
const portHostOptBlocks = doConnect([{ port: port, host: 'localhost' }],
() => common.mustNotCall());
for (const block of portHostOptBlocks) {
assert.throws(block, regexp,
assert.throws(block,
assertErr,
`${block.name}({port: ${port}, host: 'localhost'})`);
}
}
Expand Down
25 changes: 12 additions & 13 deletions test/parallel/test-net-localerror.js
Original file line number Diff line number Diff line change
Expand Up @@ -20,25 +20,24 @@
// USE OR OTHER DEALINGS IN THE SOFTWARE.

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

// Using port 0 as localPort / localAddress is already invalid.
const connect = (opts, code, type) => {
common.expectsError(
() => net.connect(opts),
{ code, type }
);
};

connect({
host: 'localhost',
port: 0,
localPort: 'foobar',
}, /^TypeError: "localPort" option should be a number: foobar$/);
localAddress: 'foobar',
}, 'ERR_INVALID_IP_ADDRESS', TypeError);

connect({
host: 'localhost',
port: 0,
localAddress: 'foobar',
}, /^TypeError: "localAddress" option must be a valid IP: foobar$/);

function connect(opts, msg) {
assert.throws(() => {
net.connect(opts);
}, msg);
}
localPort: 'foobar',
}, 'ERR_INVALID_ARG_TYPE', TypeError);
9 changes: 5 additions & 4 deletions test/parallel/test-net-options-lookup.js
Original file line number Diff line number Diff line change
@@ -1,10 +1,8 @@
'use strict';
require('../common');
const common = require('../common');
const assert = require('assert');
const net = require('net');

const expectedError = /^TypeError: "lookup" option should be a function$/;

['foobar', 1, {}, []].forEach((input) => connectThrows(input));

// Using port 0 as lookup is emitted before connecting.
Expand All @@ -17,7 +15,10 @@ function connectThrows(input) {

assert.throws(() => {
net.connect(opts);
}, expectedError);
}, common.expectsError({
code: 'ERR_INVALID_ARG_TYPE',
type: TypeError
}));
}

connectDoesNotThrow(() => {});
Expand Down
Loading

0 comments on commit 7f55349

Please sign in to comment.