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: nodejs/node#14782 Refs: nodejs/node#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>
ayojs · Oct 15, 2017 · 01a7b1c · 01a7b1c
1 parent d0b540f
commit 01a7b1c
Show file tree

Hide file tree

Showing 15 changed files with 168 additions and 125 deletions.
diff --git 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).
 
+<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
 

diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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(() => {});