From 7f55349079705a9e1d43024ed45c1351907f3d8e Mon Sep 17 00:00:00 2001 From: matzavinos Date: Sat, 12 Aug 2017 00:02:15 +0300 Subject: [PATCH] net: convert to using internal/errors 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: https://github.com/nodejs/node/pull/14782 Refs: https://github.com/nodejs/node/issues/11273 Reviewed-By: Refael Ackermann Reviewed-By: James M Snell Reviewed-By: Ruben Bridgewater Reviewed-By: Michael Dawson Reviewed-By: Joyee Cheung --- doc/api/errors.md | 5 ++ lib/dgram.js | 2 +- lib/dns.js | 2 +- lib/internal/errors.js | 3 +- lib/net.js | 42 ++++++++---- test/parallel/test-dns.js | 35 +++++----- test/parallel/test-internal-errors.js | 8 +++ .../parallel/test-net-connect-options-port.js | 65 ++++++++++--------- test/parallel/test-net-localerror.js | 25 ++++--- test/parallel/test-net-options-lookup.js | 9 +-- .../test-net-server-listen-options.js | 51 +++++++-------- test/parallel/test-net-server-options.js | 13 +++- test/parallel/test-net-socket-write-error.js | 10 +-- test/parallel/test-regress-GH-5727.js | 16 +++-- test/parallel/test-tls-lookup.js | 7 +- 15 files changed, 168 insertions(+), 125 deletions(-) diff --git a/doc/api/errors.md b/doc/api/errors.md index 13cde98283..b6514b2d2e 100644 --- a/doc/api/errors.md +++ b/doc/api/errors.md @@ -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). + +### ERR_INVALID_FD_TYPE + +Used when a file descriptor ('fd') type is not valid. + ### ERR_INVALID_FILE_URL_HOST diff --git a/lib/dgram.js b/lib/dgram.js index f33e872e64..38b182e3e7 100644 --- a/lib/dgram.js +++ b/lib/dgram.js @@ -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. diff --git a/lib/dns.js b/lib/dns.js index e536f1b4f9..581310b132 100644 --- a/lib/dns.js +++ b/lib/dns.js @@ -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'); diff --git a/lib/internal/errors.js b/lib/internal/errors.js index 7c5c8a1f0a..d760aa8425 100755 --- a/lib/internal/errors.js +++ b/lib/internal/errors.js @@ -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'); @@ -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', diff --git a/lib/net.js b/lib/net.js index a78266336c..a86edb13ef 100644 --- a/lib/net.js +++ b/lib/net.js @@ -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); } @@ -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); }; @@ -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; @@ -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, @@ -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; @@ -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 @@ -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) { diff --git a/test/parallel/test-dns.js b/test/parallel/test-dns.js index ba905d7490..02bb5f7176 100644 --- a/test/parallel/test-dns.js +++ b/test/parallel/test-dns.js @@ -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); diff --git a/test/parallel/test-internal-errors.js b/test/parallel/test-internal-errors.js index 93757ae66c..80edc25df1 100644 --- a/test/parallel/test-internal-errors.js +++ b/test/parallel/test-internal-errors.js @@ -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'); @@ -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( diff --git a/test/parallel/test-net-connect-options-port.js b/test/parallel/test-net-connect-options-port.js index 9e4d5991fd..975022be8a 100644 --- a/test/parallel/test-net-connect-options-port.js +++ b/test/parallel/test-net-connect-options-port.js @@ -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 @@ -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'})`); } } diff --git a/test/parallel/test-net-localerror.js b/test/parallel/test-net-localerror.js index 845983843d..91a7b38c3e 100644 --- a/test/parallel/test-net-localerror.js +++ b/test/parallel/test-net-localerror.js @@ -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); diff --git a/test/parallel/test-net-options-lookup.js b/test/parallel/test-net-options-lookup.js index f0e8b34c80..337a071a19 100644 --- a/test/parallel/test-net-options-lookup.js +++ b/test/parallel/test-net-options-lookup.js @@ -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. @@ -17,7 +15,10 @@ function connectThrows(input) { assert.throws(() => { net.connect(opts); - }, expectedError); + }, common.expectsError({ + code: 'ERR_INVALID_ARG_TYPE', + type: TypeError + })); } connectDoesNotThrow(() => {}); diff --git a/test/parallel/test-net-server-listen-options.js b/test/parallel/test-net-server-listen-options.js index 9f44a5bd3b..bc28b57ac1 100644 --- a/test/parallel/test-net-server-listen-options.js +++ b/test/parallel/test-net-server-listen-options.js @@ -2,21 +2,9 @@ const common = require('../common'); const assert = require('assert'); const net = require('net'); -const util = require('util'); function close() { this.close(); } -function listenError(literals, ...values) { - let result = literals[0]; - for (const [i, value] of values.entries()) { - const str = util.inspect(value); - // Need to escape special characters. - result += str.replace(/[\\^$.*+?()[\]{}|=!<>:-]/g, '\\$&'); - result += literals[i + 1]; - } - return new RegExp(`Error: Invalid listen argument: ${result}`); -} - { // Test listen() net.createServer().listen().on('listening', common.mustCall(close)); @@ -36,7 +24,13 @@ const listenOnPort = [ ]; { - const portError = /^RangeError: "port" argument must be >= 0 and < 65536$/; + const assertPort = () => { + return common.expectsError({ + code: 'ERR_SOCKET_BAD_PORT', + type: RangeError + }); + }; + for (const listen of listenOnPort) { // Arbitrary unused ports listen('0', common.mustCall(close)); @@ -44,37 +38,36 @@ const listenOnPort = [ listen(undefined, common.mustCall(close)); listen(null, common.mustCall(close)); // Test invalid ports - assert.throws(() => listen(-1, common.mustNotCall()), portError); - assert.throws(() => listen(NaN, common.mustNotCall()), portError); - assert.throws(() => listen(123.456, common.mustNotCall()), portError); - assert.throws(() => listen(65536, common.mustNotCall()), portError); - assert.throws(() => listen(1 / 0, common.mustNotCall()), portError); - assert.throws(() => listen(-1 / 0, common.mustNotCall()), portError); + assert.throws(() => listen(-1, common.mustNotCall()), assertPort()); + assert.throws(() => listen(NaN, common.mustNotCall()), assertPort()); + assert.throws(() => listen(123.456, common.mustNotCall()), assertPort()); + assert.throws(() => listen(65536, common.mustNotCall()), assertPort()); + assert.throws(() => listen(1 / 0, common.mustNotCall()), assertPort()); + assert.throws(() => listen(-1 / 0, common.mustNotCall()), assertPort()); } // In listen(options, cb), port takes precedence over path assert.throws(() => { net.createServer().listen({ port: -1, path: common.PIPE }, common.mustNotCall()); - }, portError); + }, assertPort()); } { - function shouldFailToListen(options, optionsInMessage) { - // Plain arguments get normalized into an object before - // formatted in message, options objects don't. - if (arguments.length === 1) { - optionsInMessage = options; - } + function shouldFailToListen(options) { const block = () => { net.createServer().listen(options, common.mustNotCall()); }; - assert.throws(block, listenError`${optionsInMessage}`, - `expect listen(${util.inspect(options)}) to throw`); + assert.throws(block, + common.expectsError({ + code: 'ERR_INVALID_OPT_VALUE', + type: Error, + message: /^The value "{.*}" is invalid for option "options"$/ + })); } shouldFailToListen(false, { port: false }); shouldFailToListen({ port: false }); - shouldFailToListen(true, { port: true }); + shouldFailToListen(true); shouldFailToListen({ port: true }); // Invalid fd as listen(handle) shouldFailToListen({ fd: -1 }); diff --git a/test/parallel/test-net-server-options.js b/test/parallel/test-net-server-options.js index be7e40e49b..6b120821b0 100644 --- a/test/parallel/test-net-server-options.js +++ b/test/parallel/test-net-server-options.js @@ -1,9 +1,16 @@ 'use strict'; -require('../common'); +const common = require('../common'); const assert = require('assert'); const net = require('net'); assert.throws(function() { net.createServer('path'); }, - /^TypeError: options must be an object$/); + common.expectsError({ + code: 'ERR_INVALID_ARG_TYPE', + type: TypeError + })); + assert.throws(function() { net.createServer(0); }, - /^TypeError: options must be an object$/); + common.expectsError({ + code: 'ERR_INVALID_ARG_TYPE', + type: TypeError + })); diff --git a/test/parallel/test-net-socket-write-error.js b/test/parallel/test-net-socket-write-error.js index 2af04d3109..0a2dd967df 100644 --- a/test/parallel/test-net-socket-write-error.js +++ b/test/parallel/test-net-socket-write-error.js @@ -1,6 +1,6 @@ 'use strict'; -require('../common'); +const common = require('../common'); const assert = require('assert'); const net = require('net'); @@ -8,9 +8,11 @@ const server = net.createServer().listen(0, connectToServer); function connectToServer() { const client = net.createConnection(this.address().port, () => { - assert.throws(() => { - client.write(1337); - }, /Invalid data, chunk must be a string or buffer, not number/); + assert.throws(() => client.write(1337), + common.expectsError({ + code: 'ERR_INVALID_ARG_TYPE', + type: TypeError + })); client.end(); }) diff --git a/test/parallel/test-regress-GH-5727.js b/test/parallel/test-regress-GH-5727.js index 7cedd831b2..053719c6fa 100644 --- a/test/parallel/test-regress-GH-5727.js +++ b/test/parallel/test-regress-GH-5727.js @@ -4,7 +4,6 @@ const assert = require('assert'); const net = require('net'); const invalidPort = -1 >>> 0; -const errorMessage = /"port" argument must be >= 0 and < 65536/; net.Server().listen(0, function() { const address = this.address(); @@ -17,14 +16,23 @@ net.Server().listen(0, function() { // The first argument is a configuration object assert.throws(() => { net.Server().listen({ port: invalidPort }, common.mustNotCall()); -}, errorMessage); +}, common.expectsError({ + code: 'ERR_SOCKET_BAD_PORT', + type: RangeError +})); // The first argument is the port, no IP given. assert.throws(() => { net.Server().listen(invalidPort, common.mustNotCall()); -}, errorMessage); +}, common.expectsError({ + code: 'ERR_SOCKET_BAD_PORT', + type: RangeError +})); // The first argument is the port, the second an IP. assert.throws(() => { net.Server().listen(invalidPort, '0.0.0.0', common.mustNotCall()); -}, errorMessage); +}, common.expectsError({ + code: 'ERR_SOCKET_BAD_PORT', + type: RangeError +})); diff --git a/test/parallel/test-tls-lookup.js b/test/parallel/test-tls-lookup.js index 4656d3e5cc..fddbb20ee0 100644 --- a/test/parallel/test-tls-lookup.js +++ b/test/parallel/test-tls-lookup.js @@ -6,8 +6,6 @@ if (!common.hasCrypto) const assert = require('assert'); const tls = require('tls'); -const expectedError = /^TypeError: "lookup" option should be a function$/; - ['foobar', 1, {}, []].forEach(function connectThrows(input) { const opts = { host: 'localhost', @@ -17,7 +15,10 @@ const expectedError = /^TypeError: "lookup" option should be a function$/; assert.throws(function() { tls.connect(opts); - }, expectedError); + }, common.expectsError({ + code: 'ERR_INVALID_ARG_TYPE', + type: TypeError + })); }); connectDoesNotThrow(common.mustCall(() => {}));