From 0cc1ea669b505be6082858577528d992b451b60f Mon Sep 17 00:00:00 2001 From: matzavinos Date: Sat, 12 Aug 2017 00:02:15 +0300 Subject: [PATCH] lib/net: Convert to using internal/errors Covert lib/net.js over to using lib/internal/errors.js Ref: #11273 I have not addressed the cases that use errnoException(), for reasons described in GH-12926 - 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 after a discussion in Ref: #14782 - Update tests according to the above modifications --- lib/net.js | 42 ++++++++----- .../parallel/test-net-connect-options-port.js | 61 +++++++++++-------- test/parallel/test-net-localerror.js | 18 +++--- 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 ++- 9 files changed, 130 insertions(+), 97 deletions(-) diff --git a/lib/net.js b/lib/net.js index 9c426102d6a47e..e9676b1a59d598 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.Error('ERR_INVALID_HANDLE_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.Error('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', + 'otions.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 |= 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'); } 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.TypeError('ERR_INVALID_OPT_VALUE', + 'listen options', + util.inspect(options)); }; function lookupAndListen(self, port, address, backlog, exclusive) { diff --git a/test/parallel/test-net-connect-options-port.js b/test/parallel/test-net-connect-options-port.js index 9e4d5991fd62dd..11277be251c93e 100644 --- a/test/parallel/test-net-connect-options-port.js +++ b/test/parallel/test-net-connect-options-port.js @@ -27,35 +27,37 @@ 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}$`); + function portTypeError() { + return common.expectsError({ + code: 'ERR_INVALID_ARG_TYPE', + type: TypeError + }); } - syncFailToConnect(true, portTypeError('true')); - syncFailToConnect(false, portTypeError('false')); - syncFailToConnect([], portTypeError(''), true); - syncFailToConnect({}, portTypeError('[object Object]'), true); - syncFailToConnect(null, portTypeError('null')); + 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}$`); + function portRangeError() { + return common.expectsError({ + code: 'ERR_SOCKET_BAD_PORT', + type: RangeError + }); } - 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')); + 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 +131,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 845983843d8a55..657f8c3f6606f8 100644 --- a/test/parallel/test-net-localerror.js +++ b/test/parallel/test-net-localerror.js @@ -20,25 +20,23 @@ // USE OR OTHER DEALINGS IN THE SOFTWARE. 'use strict'; -require('../common'); +const common = require('../common'); const assert = require('assert'); const net = require('net'); -// Using port 0 as localPort / localAddress is already invalid. connect({ host: 'localhost', port: 0, - localPort: 'foobar', -}, /^TypeError: "localPort" option should be a number: foobar$/); + localAddress: 'foobar', +}, 'ERR_INVALID_IP_ADDRESS', Error); connect({ host: 'localhost', port: 0, - localAddress: 'foobar', -}, /^TypeError: "localAddress" option must be a valid IP: foobar$/); + localPort: 'foobar', +}, 'ERR_INVALID_ARG_TYPE', TypeError); -function connect(opts, msg) { - assert.throws(() => { - net.connect(opts); - }, msg); +function connect(opts, code, type) { + assert.throws(() => net.connect(opts), + common.expectsError({ code: code, type: type })); } diff --git a/test/parallel/test-net-options-lookup.js b/test/parallel/test-net-options-lookup.js index f0e8b34c807df0..337a071a19b80b 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 9f44a5bd3b7a2a..daccea3c556d52 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: Error + }); + }; + 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: TypeError, + message: /^The value "{.*}" is invalid for option "listen 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 be7e40e49bf16f..b0424bdc32ecb2 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 assert = require('assert'); +const common = require('../common'); 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 2af04d310903be..0a2dd967df6a2a 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 7cedd831b2f783..053719c6facf76 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 4656d3e5cc6a1e..fddbb20ee00f36 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(() => {}));