net: make Socket.connect() consistently asynchronous

lib/net.js

@@ -769,34 +769,18 @@ function connect(self, address, port, addressType, localAddress, localPort) {
   assert.ok(self._connecting);
 
   var err;
-  if (localAddress || localPort) {
-    if (localAddress && !exports.isIP(localAddress))
-      err = new TypeError(
-          'localAddress should be a valid IP: ' + localAddress);
-
-    if (localPort && !util.isNumber(localPort))
-      err = new TypeError('localPort should be a number: ' + localPort);
 
+  if (localAddress || localPort) {
     var bind;
 
-    switch (addressType) {
-      case 4:
-        if (!localAddress)
-          localAddress = '0.0.0.0';
-        bind = self._handle.bind;
-        break;
-      case 6:
-        if (!localAddress)
-          localAddress = '::';
-        bind = self._handle.bind6;
-        break;
-      default:
-        err = new TypeError('Invalid addressType: ' + addressType);
-        break;
-    }
-
-    if (err) {
-      self._destroy(err);
+    if (addressType === 4) {
+      localAddress = localAddress || '0.0.0.0';
+      bind = self._handle.bind;
+    } else if (addressType === 6) {
+      localAddress = localAddress || '::';
+      bind = self._handle.bind6;
+    } else {
+      self._destroy(new TypeError('Invalid addressType: ' + addressType));
       return;
     }
 
@@ -814,16 +798,11 @@ function connect(self, address, port, addressType, localAddress, localPort) {
   }
 
   var req = { oncomplete: afterConnect };
-  if (addressType === 6 || addressType === 4) {
-    port = port | 0;
-    if (port <= 0 || port > 65535)
-      throw new RangeError('Port should be > 0 and < 65536');
 
-    if (addressType === 6) {
-      err = self._handle.connect6(req, address, port);
-    } else if (addressType === 4) {
-      err = self._handle.connect(req, address, port);
-    }
+  if (addressType === 4) {
+    err = self._handle.connect(req, address, port);
+  } else if (addressType === 6) {
+    err = self._handle.connect6(req, address, port);
   } else {
     err = self._handle.connect(req, address, afterConnect);
   }
@@ -879,19 +858,26 @@ Socket.prototype.connect = function(options, cb) {
   if (pipe) {
     connect(self, options.path);
 
-  } else if (!options.host) {
-    debug('connect: missing host');
-    self._host = '127.0.0.1';
-    connect(self, self._host, options.port, 4);
-
   } else {
     var dns = require('dns');
-    var host = options.host;
+    var host = options.host || 'localhost';
+    var port = options.port | 0;
+    var localAddress = options.localAddress;
+    var localPort = options.localPort;
     var dnsopts = {
       family: options.family,
       hints: 0
     };
 
+    if (localAddress && !exports.isIP(localAddress))
+      throw new TypeError('localAddress must be a valid IP: ' + localAddress);
+
+    if (localPort && !util.isNumber(localPort))
+      throw new TypeError('localPort should be a number: ' + localPort);
+
+    if (port <= 0 || port > 65535)
+      throw new RangeError('port should be > 0 and < 65536: ' + port);
+
     if (dnsopts.family !== 4 && dnsopts.family !== 6)
       dnsopts.hints = dns.ADDRCONFIG | dns.V4MAPPED;
 
@@ -917,19 +903,12 @@ Socket.prototype.connect = function(options, cb) {
         });
       } else {
         timers._unrefActive(self);
-
-        addressType = addressType || 4;
-
-        // node_net.cc handles null host names graciously but user land
-        // expects remoteAddress to have a meaningful value
-        ip = ip || (addressType === 4 ? '127.0.0.1' : '0:0:0:0:0:0:0:1');
-
         connect(self,
                 ip,
-                options.port,
+                port,
                 addressType,
-                options.localAddress,
-                options.localPort);
+                localAddress,
+                localPort);
       }
     });
   }

test/simple/test-net-localerror.js

@@ -23,39 +23,30 @@ var common = require('../common');
 var assert = require('assert');
 var net = require('net');
 
-var server = net.createServer(function(socket) {
-  assert.ok(false, 'no clients should connect');
-}).listen(common.PORT).on('listening', function() {
-  server.unref();
+  connect({
+    host: 'localhost',
+    port: common.PORT,
+    localPort: 'foobar',
+  }, 'localPort should be a number: foobar');
 
-  function test1(next) {
-    connect({
-      host: '127.0.0.1',
-      port: common.PORT,
-      localPort: 'foobar',
-    },
-    'localPort should be a number: foobar',
-    next);
-  }
+  connect({
+    host: 'localhost',
+    port: common.PORT,
+    localAddress: 'foobar',
+  }, 'localAddress should be a valid IP: foobar');
 
-  function test2(next) {
-    connect({
-      host: '127.0.0.1',
-      port: common.PORT,
-      localAddress: 'foobar',
-    },
-    'localAddress should be a valid IP: foobar',
-    next)
-  }
+  connect({
+    host: 'localhost',
+    port: 65536
+  }, 'port should be > 0 and < 65536: 65536');
 
-  test1(test2);
-})
+  connect({
+    host: 'localhost',
+    port: 0
+  }, 'port should be > 0 and < 65536: 0');
 
-function connect(opts, msg, cb) {
-  var client = net.connect(opts).on('connect', function() {
-    assert.ok(false, 'we should never connect');
-  }).on('error', function(err) {
-    assert.strictEqual(err.message, msg);
-    if (cb) cb();
-  });
+function connect(opts, msg) {
+  assert.throws(function() {
+    var client = net.connect(opts);
+  }, msg);
 }

test/simple/test-process-active-wraps.js

@@ -41,9 +41,16 @@ var handles = [];
 })();
 
 (function() {
+  function onlookup() {
+    setImmediate(function() {
+      assert.equal(process._getActiveRequests().length, 0);
+    });
+  };
+
   expect(1, 0);
   var conn = net.createConnection(common.PORT);
-  conn.on('error', function() { /* ignore */ });
+  conn.on('lookup', onlookup); 
+  conn.on('error', function() { assert(false); });
   expect(2, 1);
   conn.destroy();
   expect(2, 1); // client handle doesn't shut down until next tick
@@ -52,10 +59,15 @@ var handles = [];
 
 (function() {
   var n = 0;
+
   handles.forEach(function(handle) {
     handle.once('close', onclose);
   });
   function onclose() {
-    if (++n === handles.length) setImmediate(expect, 0, 0);
+    if (++n === handles.length) {
+      setImmediate(function() {
+        assert.equal(process._getActiveHandles().length, 0);
+      });
+    }
   }
 })();