From 0cc1ea669b505be6082858577528d992b451b60f Mon Sep 17 00:00:00 2001
From: matzavinos <matzavinos@workable.com>
Date: Sat, 12 Aug 2017 00:02:15 +0300
Subject: [PATCH 01/10] 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(() => {}));

From 00b8f490457a165db4fd2de85c4d8afcf64f19c1 Mon Sep 17 00:00:00 2001
From: matzavinos <matzavinos@workable.com>
Date: Sun, 20 Aug 2017 20:12:36 +0300
Subject: [PATCH 02/10] [fixup] lib/net: update tests

- refactor the portTypeError and portRangeError assertions,
  by passing the expected call count to common.expectsError in
  test-net-connect-options-port.js
  (Each syncFailToConnect() call will call 4 or 2 times the
  doConnect() method, wich returns an array of size 6,
  so the call counts are 96 and 168 for portTypeError and
  portRangeError respectively)
- refactor the connect() method in test-net-localerror.js,
  by removing the reduntant assert
  and use the common.expectsError() assertion
- move require('../common') to the top in test-net-server-options.js
---
 .../parallel/test-net-connect-options-port.js | 28 ++++++++-----------
 test/parallel/test-net-localerror.js          | 11 ++++----
 test/parallel/test-net-server-options.js      |  2 +-
 3 files changed, 18 insertions(+), 23 deletions(-)

diff --git a/test/parallel/test-net-connect-options-port.js b/test/parallel/test-net-connect-options-port.js
index 11277be251c93e..975022be8aa88c 100644
--- a/test/parallel/test-net-connect-options-port.js
+++ b/test/parallel/test-net-connect-options-port.js
@@ -27,12 +27,10 @@ const net = require('net');
 
 // Test wrong type of ports
 {
-  function portTypeError() {
-    return common.expectsError({
-      code: 'ERR_INVALID_ARG_TYPE',
-      type: TypeError
-    });
-  }
+  const portTypeError = common.expectsError({
+    code: 'ERR_INVALID_ARG_TYPE',
+    type: TypeError
+  }, 96);
 
   syncFailToConnect(true, portTypeError);
   syncFailToConnect(false, portTypeError);
@@ -43,12 +41,10 @@ const net = require('net');
 
 // Test out of range ports
 {
-  function portRangeError() {
-    return common.expectsError({
-      code: 'ERR_SOCKET_BAD_PORT',
-      type: RangeError
-    });
-  }
+  const portRangeError = common.expectsError({
+    code: 'ERR_SOCKET_BAD_PORT',
+    type: RangeError
+  }, 168);
 
   syncFailToConnect('', portRangeError);
   syncFailToConnect(' ', portRangeError);
@@ -137,7 +133,7 @@ function syncFailToConnect(port, assertErr, optOnly) {
     const portArgBlocks = doConnect([port], () => common.mustNotCall());
     for (const block of portArgBlocks) {
       assert.throws(block,
-                    assertErr(),
+                    assertErr,
                     `${block.name}(${port})`);
     }
 
@@ -146,7 +142,7 @@ function syncFailToConnect(port, assertErr, optOnly) {
                                         () => common.mustNotCall());
     for (const block of portHostArgBlocks) {
       assert.throws(block,
-                    assertErr(),
+                    assertErr,
                     `${block.name}(${port}, 'localhost')`);
     }
   }
@@ -155,7 +151,7 @@ function syncFailToConnect(port, assertErr, optOnly) {
                                   () => common.mustNotCall());
   for (const block of portOptBlocks) {
     assert.throws(block,
-                  assertErr(),
+                  assertErr,
                   `${block.name}({port: ${port}})`);
   }
 
@@ -164,7 +160,7 @@ function syncFailToConnect(port, assertErr, optOnly) {
                                       () => common.mustNotCall());
   for (const block of portHostOptBlocks) {
     assert.throws(block,
-                  assertErr(),
+                  assertErr,
                   `${block.name}({port: ${port}, host: 'localhost'})`);
   }
 }
diff --git a/test/parallel/test-net-localerror.js b/test/parallel/test-net-localerror.js
index 657f8c3f6606f8..70aadcfee13fcf 100644
--- a/test/parallel/test-net-localerror.js
+++ b/test/parallel/test-net-localerror.js
@@ -21,9 +21,13 @@
 
 'use strict';
 const common = require('../common');
-const assert = require('assert');
 const net = require('net');
 
+const connect = (opts, code, type) => common.expectsError(
+  () => net.connect(opts),
+  { code, type }
+);
+
 connect({
   host: 'localhost',
   port: 0,
@@ -35,8 +39,3 @@ connect({
   port: 0,
   localPort: 'foobar',
 }, 'ERR_INVALID_ARG_TYPE', TypeError);
-
-function connect(opts, code, type) {
-  assert.throws(() => net.connect(opts),
-                common.expectsError({ code: code, type: type }));
-}
diff --git a/test/parallel/test-net-server-options.js b/test/parallel/test-net-server-options.js
index b0424bdc32ecb2..6b120821b0d3ac 100644
--- a/test/parallel/test-net-server-options.js
+++ b/test/parallel/test-net-server-options.js
@@ -1,6 +1,6 @@
 'use strict';
-const assert = require('assert');
 const common = require('../common');
+const assert = require('assert');
 const net = require('net');
 
 assert.throws(function() { net.createServer('path'); },

From 16d8519fc818248f40ba8563b5ac4945096a7d90 Mon Sep 17 00:00:00 2001
From: matzavinos <matzavinos@workable.com>
Date: Sun, 20 Aug 2017 21:34:10 +0300
Subject: [PATCH 03/10] [fixup] lib/net: refac ERR_INVALID_ARG_TYPE err

- refactor the valid types argument of ERR_INVALID_ARG_TYPE errors,
  to be a string instead of an array when the number of valid types
  equals 1, in lib/net.js
---
 lib/net.js | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/lib/net.js b/lib/net.js
index e9676b1a59d598..62006c0248ee15 100644
--- a/lib/net.js
+++ b/lib/net.js
@@ -1043,14 +1043,14 @@ function lookupAndConnect(self, options) {
   if (localPort && typeof localPort !== 'number') {
     throw new errors.TypeError('ERR_INVALID_ARG_TYPE',
                                'options.localPort',
-                               ['number'],
+                               'number',
                                localPort);
   }
 
   if (typeof port !== 'undefined') {
     if (typeof port !== 'number' && typeof port !== 'string') {
       throw new errors.TypeError('ERR_INVALID_ARG_TYPE',
-                                 'otions.port',
+                                 'options.port',
                                  ['number', 'string'],
                                  port);
     }
@@ -1073,7 +1073,7 @@ function lookupAndConnect(self, options) {
   if (options.lookup && typeof options.lookup !== 'function')
     throw new errors.TypeError('ERR_INVALID_ARG_TYPE',
                                'options.lookup',
-                               ['function'],
+                               'function',
                                options.lookup);
 
   var dnsopts = {
@@ -1218,7 +1218,7 @@ function Server(options, connectionListener) {
   } else {
     throw new errors.TypeError('ERR_INVALID_ARG_TYPE',
                                'options',
-                               ['object'],
+                               'object',
                                options);
   }
 

From 90eb1a5d2d081dd2c8a3a563454b374ed8865101 Mon Sep 17 00:00:00 2001
From: matzavinos <matzavinos@workable.com>
Date: Mon, 21 Aug 2017 12:07:16 +0300
Subject: [PATCH 04/10] [fixup] lib/net: update  Server.listen() err msg

replace "listen options" with "options"
---
 lib/net.js                                      | 2 +-
 test/parallel/test-net-server-listen-options.js | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/lib/net.js b/lib/net.js
index 62006c0248ee15..93c3deadee0c8b 100644
--- a/lib/net.js
+++ b/lib/net.js
@@ -1503,7 +1503,7 @@ Server.prototype.listen = function(...args) {
   }
 
   throw new errors.TypeError('ERR_INVALID_OPT_VALUE',
-                             'listen options',
+                             'options',
                              util.inspect(options));
 };
 
diff --git a/test/parallel/test-net-server-listen-options.js b/test/parallel/test-net-server-listen-options.js
index daccea3c556d52..f34131d3cb95c3 100644
--- a/test/parallel/test-net-server-listen-options.js
+++ b/test/parallel/test-net-server-listen-options.js
@@ -61,7 +61,7 @@ const listenOnPort = [
                   common.expectsError({
                     code: 'ERR_INVALID_OPT_VALUE',
                     type: TypeError,
-                    message: /^The value "{.*}" is invalid for option "listen options"$/
+                    message: /^The value "{.*}" is invalid for option "options"$/
                   }));
   }
 

From 056dfcc0ed72f34e70b1165c452e4c92885cea16 Mon Sep 17 00:00:00 2001
From: matzavinos <matzavinos@workable.com>
Date: Fri, 25 Aug 2017 01:43:26 +0300
Subject: [PATCH 05/10] [fixup] lib/net.js update createHandle() err

- Create a new error 'ERR_INVALID_FD_TYPE'
  in lib/internal/errors.js,
  following the corresponding discussion at Ref: #14782.
  This error should be thrown
  when a file descriptor type is not valid.
- add test for the new err in test-internal-errors.js
- update the error doc in errors.md
- use the new err in createHandle() in lib/net.js
- Add brackets {} in arrow function in test-net-localerror.js
  to match the typical source code's format.
---
 doc/api/errors.md                     |  5 +++++
 lib/internal/errors.js                |  1 +
 lib/net.js                            |  2 +-
 test/parallel/test-internal-errors.js |  4 ++++
 test/parallel/test-net-localerror.js  | 10 ++++++----
 5 files changed, 17 insertions(+), 5 deletions(-)

diff --git a/doc/api/errors.md b/doc/api/errors.md
index ba81f2f75eee5b..34ea06ebc3db1b 100644
--- a/doc/api/errors.md
+++ b/doc/api/errors.md
@@ -918,6 +918,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/internal/errors.js b/lib/internal/errors.js
index d81a070a110a65..fc94849fb3194d 100755
--- a/lib/internal/errors.js
+++ b/lib/internal/errors.js
@@ -219,6 +219,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');
diff --git a/lib/net.js b/lib/net.js
index 93c3deadee0c8b..d167b110f51bf5 100644
--- a/lib/net.js
+++ b/lib/net.js
@@ -64,7 +64,7 @@ function createHandle(fd) {
   const type = TTYWrap.guessHandleType(fd);
   if (type === 'PIPE') return new Pipe();
   if (type === 'TCP') return new TCP();
-  throw new errors.Error('ERR_INVALID_HANDLE_TYPE');
+  throw new errors.Error('ERR_INVALID_FD_TYPE', type);
 }
 
 
diff --git a/test/parallel/test-internal-errors.js b/test/parallel/test-internal-errors.js
index 1c16823e7a1389..78a2185133578b 100644
--- a/test/parallel/test-internal-errors.js
+++ b/test/parallel/test-internal-errors.js
@@ -207,6 +207,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');
diff --git a/test/parallel/test-net-localerror.js b/test/parallel/test-net-localerror.js
index 70aadcfee13fcf..a327c35092ab18 100644
--- a/test/parallel/test-net-localerror.js
+++ b/test/parallel/test-net-localerror.js
@@ -23,10 +23,12 @@
 const common = require('../common');
 const net = require('net');
 
-const connect = (opts, code, type) => common.expectsError(
-  () => net.connect(opts),
-  { code, type }
-);
+const connect = (opts, code, type) => {
+  common.expectsError(
+    () => net.connect(opts),
+    { code, type }
+  );
+};
 
 connect({
   host: 'localhost',

From 7373ee21d8e7bf398c97dda423258e9b6d42f5c0 Mon Sep 17 00:00:00 2001
From: matzavinos <matzavinos@workable.com>
Date: Fri, 25 Aug 2017 02:57:34 +0300
Subject: [PATCH 06/10] [fixup] lib/net: ERR_INVALID_FD_TYPE add test

- add missing test for the 'ERR_INVALID_FD_TYPE' err that is thrown
  from the net.Socket constructor
- correct the err type from Error to TypeError
---
 lib/net.js                                |  2 +-
 test/parallel/test-net-socket-fd-error.js | 16 ++++++++++++++++
 2 files changed, 17 insertions(+), 1 deletion(-)
 create mode 100644 test/parallel/test-net-socket-fd-error.js

diff --git a/lib/net.js b/lib/net.js
index d167b110f51bf5..b20018623cc242 100644
--- a/lib/net.js
+++ b/lib/net.js
@@ -64,7 +64,7 @@ function createHandle(fd) {
   const type = TTYWrap.guessHandleType(fd);
   if (type === 'PIPE') return new Pipe();
   if (type === 'TCP') return new TCP();
-  throw new errors.Error('ERR_INVALID_FD_TYPE', type);
+  throw new errors.TypeError('ERR_INVALID_FD_TYPE', type);
 }
 
 
diff --git a/test/parallel/test-net-socket-fd-error.js b/test/parallel/test-net-socket-fd-error.js
new file mode 100644
index 00000000000000..71c4b4b04a0d50
--- /dev/null
+++ b/test/parallel/test-net-socket-fd-error.js
@@ -0,0 +1,16 @@
+'use strict';
+const common = require('../common');
+const net = require('net');
+
+const block = () => {
+  new net.Socket({
+    fd: 'invalid'
+  });
+};
+
+const err = {
+  code: 'ERR_INVALID_FD_TYPE',
+  type: TypeError
+};
+
+common.expectsError(block, err);

From 787682b9cfe17f3b586b35cbe283ea41cd1df803 Mon Sep 17 00:00:00 2001
From: matzavinos <matzavinos@workable.com>
Date: Fri, 25 Aug 2017 13:01:41 +0300
Subject: [PATCH 07/10] [fixup] lib/net: correct err types

- correct 'ERR_INVALID_IP_ADDRESS' type to TypeError err in net.js
- correct 'ERR_SOCKET_BAD_PORT' type to RangeError in net.js
- correct 'ERR_INVALID_OPT_VALUE' type to Error in net.js
- pass the port value to the the 'ERR_SOCKET_BAD_PORT' err in net.js
---
 lib/net.js                                      | 12 ++++++------
 test/parallel/test-net-localerror.js            |  2 +-
 test/parallel/test-net-server-listen-options.js |  4 ++--
 3 files changed, 9 insertions(+), 9 deletions(-)

diff --git a/lib/net.js b/lib/net.js
index b20018623cc242..480cb9ddf44b78 100644
--- a/lib/net.js
+++ b/lib/net.js
@@ -1037,7 +1037,7 @@ function lookupAndConnect(self, options) {
   var localPort = options.localPort;
 
   if (localAddress && !cares.isIP(localAddress)) {
-    throw new errors.Error('ERR_INVALID_IP_ADDRESS', localAddress);
+    throw new errors.TypeError('ERR_INVALID_IP_ADDRESS', localAddress);
   }
 
   if (localPort && typeof localPort !== 'number') {
@@ -1055,7 +1055,7 @@ function lookupAndConnect(self, options) {
                                  port);
     }
     if (!isLegalPort(port)) {
-      throw new errors.RangeError('ERR_SOCKET_BAD_PORT');
+      throw new errors.RangeError('ERR_SOCKET_BAD_PORT', port);
     }
   }
   port |= 0;
@@ -1477,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 errors.RangeError('ERR_SOCKET_BAD_PORT');
+      throw new errors.RangeError('ERR_SOCKET_BAD_PORT', options.port);
     }
     backlog = options.backlog || backlogFromArgs;
     // start TCP server listening on host:port
@@ -1502,9 +1502,9 @@ Server.prototype.listen = function(...args) {
     return this;
   }
 
-  throw new errors.TypeError('ERR_INVALID_OPT_VALUE',
-                             'options',
-                             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-net-localerror.js b/test/parallel/test-net-localerror.js
index a327c35092ab18..91a7b38c3ef661 100644
--- a/test/parallel/test-net-localerror.js
+++ b/test/parallel/test-net-localerror.js
@@ -34,7 +34,7 @@ connect({
   host: 'localhost',
   port: 0,
   localAddress: 'foobar',
-}, 'ERR_INVALID_IP_ADDRESS', Error);
+}, 'ERR_INVALID_IP_ADDRESS', TypeError);
 
 connect({
   host: 'localhost',
diff --git a/test/parallel/test-net-server-listen-options.js b/test/parallel/test-net-server-listen-options.js
index f34131d3cb95c3..bc28b57ac12854 100644
--- a/test/parallel/test-net-server-listen-options.js
+++ b/test/parallel/test-net-server-listen-options.js
@@ -27,7 +27,7 @@ const listenOnPort = [
   const assertPort = () => {
     return common.expectsError({
       code: 'ERR_SOCKET_BAD_PORT',
-      type: Error
+      type: RangeError
     });
   };
 
@@ -60,7 +60,7 @@ const listenOnPort = [
     assert.throws(block,
                   common.expectsError({
                     code: 'ERR_INVALID_OPT_VALUE',
-                    type: TypeError,
+                    type: Error,
                     message: /^The value "{.*}" is invalid for option "options"$/
                   }));
   }

From 74d6b0bdc794cbed412ec992499a4dda03b3c4e7 Mon Sep 17 00:00:00 2001
From: matzavinos <matzavinos@workable.com>
Date: Tue, 29 Aug 2017 11:31:26 +0300
Subject: [PATCH 08/10] [fixup] lib/net: ERR_SOCKET_BAD_PORT handle port

- Modify the ERR_SOCKET_BAD_PORT definition in internal/errors.js
  to include the specified port in the err msg, if the port
  arg is passed to the err constructor.
- Add tests for ERR_SOCKET_BAD_PORT msg in test-internal-errors.js
---
 lib/internal/errors.js                | 8 +++++++-
 test/parallel/test-internal-errors.js | 7 +++++++
 2 files changed, 14 insertions(+), 1 deletion(-)

diff --git a/lib/internal/errors.js b/lib/internal/errors.js
index fc94849fb3194d..eb0de294d20f0a 100755
--- a/lib/internal/errors.js
+++ b/lib/internal/errors.js
@@ -267,7 +267,13 @@ 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) => {
+  let portMsg = '';
+  if (typeof port !== 'undefined' && port !== null) {
+    portMsg = `:${port}`;
+  }
+  return `Port${portMsg} should be > 0 and < 65536`;
+});
 E('ERR_SOCKET_BAD_TYPE',
   'Bad socket type specified. Valid types are: udp4, udp6');
 E('ERR_SOCKET_BUFFER_SIZE',
diff --git a/test/parallel/test-internal-errors.js b/test/parallel/test-internal-errors.js
index 78a2185133578b..e835ddba817e9e 100644
--- a/test/parallel/test-internal-errors.js
+++ b/test/parallel/test-internal-errors.js
@@ -241,6 +241,13 @@ assert.throws(
     message: /^At least one arg needs to be specified$/
   }));
 
+// Test ERR_SOCKET_BAD_PORT
+assert.strictEqual(errors.message('ERR_SOCKET_BAD_PORT', []),
+                   'Port should be > 0 and < 65536');
+assert.strictEqual(errors.message('ERR_SOCKET_BAD_PORT', ['0']),
+                   'Port:0 should be > 0 and < 65536');
+assert.strictEqual(errors.message('ERR_SOCKET_BAD_PORT', [0]),
+                   'Port:0 should be > 0 and < 65536');
 
 // Test ERR_TLS_CERT_ALTNAME_INVALID
 assert.strictEqual(

From 5a4e9828280223c30b63b7a80b32b840416d14f4 Mon Sep 17 00:00:00 2001
From: matzavinos <matzavinos@workable.com>
Date: Fri, 1 Sep 2017 13:33:20 +0300
Subject: [PATCH 09/10] [fixup] lib/net: simplify err ERR_SOCKET_BAD_PORT

- simplify the ERR_SOCKET_BAD_PORT error definition in errors.js
  by replacing the cb arg with the string:
  'Port should be > 0 and < 65536. Received %s.'
- update the ERR_SOCKET_BAD_PORT test in test-internal-errors.js
- update the ERR_SOCKET_BAD_PORT constructor call in
  lib/dgram.js and lib/dns to include the actual port
  as the second arg
- update the already existing ERR_SOCKET_BAD_PORT message
  assertion in test-dns.js, to include the actual port.
---
 lib/dgram.js                          |  2 +-
 lib/dns.js                            |  2 +-
 lib/internal/errors.js                |  8 +-----
 test/parallel/test-dns.js             | 35 +++++++++++++--------------
 test/parallel/test-internal-errors.js |  9 +++----
 5 files changed, 23 insertions(+), 33 deletions(-)

diff --git a/lib/dgram.js b/lib/dgram.js
index 85e5f909c02f2b..d8fb8288fb6252 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 e536f1b4f942de..581310b1322120 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 eb0de294d20f0a..8f9a960c49f478 100755
--- a/lib/internal/errors.js
+++ b/lib/internal/errors.js
@@ -267,13 +267,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) => {
-  let portMsg = '';
-  if (typeof port !== 'undefined' && port !== null) {
-    portMsg = `:${port}`;
-  }
-  return `Port${portMsg} 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/test/parallel/test-dns.js b/test/parallel/test-dns.js
index ba905d7490d1e9..02bb5f717687f7 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 e835ddba817e9e..dea91bd688b3aa 100644
--- a/test/parallel/test-internal-errors.js
+++ b/test/parallel/test-internal-errors.js
@@ -242,12 +242,9 @@ assert.throws(
   }));
 
 // Test ERR_SOCKET_BAD_PORT
-assert.strictEqual(errors.message('ERR_SOCKET_BAD_PORT', []),
-                   'Port should be > 0 and < 65536');
-assert.strictEqual(errors.message('ERR_SOCKET_BAD_PORT', ['0']),
-                   'Port:0 should be > 0 and < 65536');
-assert.strictEqual(errors.message('ERR_SOCKET_BAD_PORT', [0]),
-                   'Port:0 should be > 0 and < 65536');
+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(

From 30a1ca0601eb0c620a732a50db9388a232516e28 Mon Sep 17 00:00:00 2001
From: matzavinos <matzavinos@workable.com>
Date: Wed, 27 Sep 2017 22:17:11 +0300
Subject: [PATCH 10/10] [fixup] lib/net: remove failing test

removed the
test/parallel/test-net-socket-fd-error.js

this test was added in the current PR
but is not consistent. As the used
fd mock value does not consistenly
resolve to a fd type not equal to 'PIPE'
and not equal to 'TCP'
---
 test/parallel/test-net-socket-fd-error.js | 16 ----------------
 1 file changed, 16 deletions(-)
 delete mode 100644 test/parallel/test-net-socket-fd-error.js

diff --git a/test/parallel/test-net-socket-fd-error.js b/test/parallel/test-net-socket-fd-error.js
deleted file mode 100644
index 71c4b4b04a0d50..00000000000000
--- a/test/parallel/test-net-socket-fd-error.js
+++ /dev/null
@@ -1,16 +0,0 @@
-'use strict';
-const common = require('../common');
-const net = require('net');
-
-const block = () => {
-  new net.Socket({
-    fd: 'invalid'
-  });
-};
-
-const err = {
-  code: 'ERR_INVALID_FD_TYPE',
-  type: TypeError
-};
-
-common.expectsError(block, err);