net: introduce Socket#connecting property

indutny · Myles Borins · commit 0eef0981f6ac · 2016-07-11T13:00:01.000-07:00
There is no official way to figure out if the socket that you have on hand is still connecting to the remote host. Introduce `Socket#connecting`, which is essentially an unprefixed `_connecting` property that we already had. PR-URL: #6404 Reviewed-By: Evan Lucas <evanlucas@me.com> Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
diff --git a/doc/api/net.md b/doc/api/net.md
@@ -392,6 +392,12 @@ The `connectListener` parameter will be added as a listener for the
 As [`socket.connect(options\[, connectListener\])`][`socket.connect(options, connectListener)`],
 with options either as either `{port: port, host: host}` or `{path: path}`.
 
+### socket.connecting
+
+If `true` - [`socket.connect(options\[, connectListener\])`][] was called and
+haven't yet finished. Will be set to `false` before emitting `connect` event
+and/or calling [`socket.connect(options\[, connectListener\])`][]'s callback.
+
 ### socket.destroy()
 
 Ensures that no more I/O activity happens on this socket. Only necessary in
diff --git a/lib/_tls_legacy.js b/lib/_tls_legacy.js
@@ -480,7 +480,7 @@ CryptoStream.prototype._done = function() {
 // readyState is deprecated. Don't use it.
 Object.defineProperty(CryptoStream.prototype, 'readyState', {
   get: function() {
-    if (this._connecting) {
+    if (this.connecting) {
       return 'opening';
     } else if (this.readable && this.writable) {
       return 'open';
diff --git a/lib/_tls_wrap.js b/lib/_tls_wrap.js
@@ -284,7 +284,7 @@ function TLSSocket(socket, options) {
 
   this._init(socket, wrap);
 
-  // Make sure to setup all required properties like: `_connecting` before
+  // Make sure to setup all required properties like: `connecting` before
   // starting the flow of the data
   this.readable = true;
   this.writable = true;
@@ -472,9 +472,9 @@ TLSSocket.prototype._init = function(socket, wrap) {
     this._parent = socket;
 
     // To prevent assertion in afterConnect() and properly kick off readStart
-    this._connecting = socket._connecting || !socket._handle;
+    this.connecting = socket.connecting || !socket._handle;
     socket.once('connect', function() {
-      self._connecting = false;
+      self.connecting = false;
       self.emit('connect');
     });
   }
@@ -486,7 +486,7 @@ TLSSocket.prototype._init = function(socket, wrap) {
     });
   } else {
     assert(!socket);
-    this._connecting = true;
+    this.connecting = true;
   }
 };
 
@@ -583,7 +583,7 @@ TLSSocket.prototype._finishInit = function() {
 };
 
 TLSSocket.prototype._start = function() {
-  if (this._connecting) {
+  if (this.connecting) {
     this.once('connect', function() {
       this._start();
     });
diff --git a/lib/net.js b/lib/net.js
@@ -118,7 +118,7 @@ const BYTES_READ = Symbol('bytesRead');
 function Socket(options) {
   if (!(this instanceof Socket)) return new Socket(options);
 
-  this._connecting = false;
+  this.connecting = false;
   this._hadError = false;
   this._handle = null;
   this._parent = null;
@@ -201,7 +201,7 @@ Socket.prototype._unrefTimer = function unrefTimer() {
 // so that only the writable side will be cleaned up.
 function onSocketFinish() {
   // If still connecting - defer handling 'finish' until 'connect' will happen
-  if (this._connecting) {
+  if (this.connecting) {
     debug('osF: not yet connected');
     return this.once('connect', onSocketFinish);
   }
@@ -366,9 +366,16 @@ Socket.prototype.address = function() {
 };
 
 
+Object.defineProperty(Socket.prototype, '_connecting', {
+  get: function() {
+    return this.connecting;
+  }
+});
+
+
 Object.defineProperty(Socket.prototype, 'readyState', {
   get: function() {
-    if (this._connecting) {
+    if (this.connecting) {
       return 'opening';
     } else if (this.readable && this.writable) {
       return 'open';
@@ -396,7 +403,7 @@ Object.defineProperty(Socket.prototype, 'bufferSize', {
 Socket.prototype._read = function(n) {
   debug('_read');
 
-  if (this._connecting || !this._handle) {
+  if (this.connecting || !this._handle) {
     debug('_read wait for connection');
     this.once('connect', () => this._read(n));
   } else if (!this._handle.reading) {
@@ -429,7 +436,7 @@ function maybeDestroy(socket) {
   if (!socket.readable &&
       !socket.writable &&
       !socket.destroyed &&
-      !socket._connecting &&
+      !socket.connecting &&
       !socket._writableState.length) {
     socket.destroy();
   }
@@ -466,7 +473,7 @@ Socket.prototype._destroy = function(exception, cb) {
     return;
   }
 
-  self._connecting = false;
+  this.connecting = false;
 
   this.readable = this.writable = false;
 
@@ -647,7 +654,7 @@ Socket.prototype._writeGeneric = function(writev, data, encoding, cb) {
   // If we are still connecting, then buffer this for later.
   // The Writable logic will buffer up any more writes while
   // waiting for this one to be done.
-  if (this._connecting) {
+  if (this.connecting) {
     this._pendingData = data;
     this._pendingEncoding = encoding;
     this.once('connect', function() {
@@ -802,7 +809,7 @@ function connect(self, address, port, addressType, localAddress, localPort) {
   // TODO return promise from Socket.prototype.connect which
   // wraps _connectReq.
 
-  assert.ok(self._connecting);
+  assert.ok(self.connecting);
 
   var err;
 
@@ -912,7 +919,7 @@ Socket.prototype.connect = function(options, cb) {
 
   this._unrefTimer();
 
-  this._connecting = true;
+  this.connecting = true;
   this.writable = true;
 
   if (pipe) {
@@ -949,7 +956,7 @@ function lookupAndConnect(self, options) {
   var addressType = exports.isIP(host);
   if (addressType) {
     process.nextTick(function() {
-      if (self._connecting)
+      if (self.connecting)
         connect(self, host, port, addressType, localAddress, localPort);
     });
     return;
@@ -984,7 +991,7 @@ function lookupAndConnect(self, options) {
     // It's possible we were destroyed while looking this up.
     // XXX it would be great if we could cancel the promise returned by
     // the look up.
-    if (!self._connecting) return;
+    if (!self.connecting) return;
 
     if (err) {
       // net.createConnection() creates a net.Socket object and
@@ -1052,8 +1059,8 @@ function afterConnect(status, handle, req, readable, writable) {
 
   debug('afterConnect');
 
-  assert.ok(self._connecting);
-  self._connecting = false;
+  assert.ok(self.connecting);
+  self.connecting = false;
   self._sockname = null;
 
   if (status == 0) {
@@ -1069,7 +1076,7 @@ function afterConnect(status, handle, req, readable, writable) {
       self.read(0);
 
   } else {
-    self._connecting = false;
+    self.connecting = false;
     var details;
     if (req.localAddress && req.localPort) {
       details = req.localAddress + ':' + req.localPort;
diff --git a/test/parallel/test-net-connect-buffer.js b/test/parallel/test-net-connect-buffer.js
@@ -40,7 +40,7 @@ tcp.listen(common.PORT, function() {
     connectHappened = true;
   });
 
-  console.log('_connecting = ' + socket._connecting);
+  console.log('connecting = ' + socket.connecting);
 
   assert.equal('opening', socket.readyState);
 
diff --git a/test/parallel/test-net-socket-connecting.js b/test/parallel/test-net-socket-connecting.js
@@ -0,0 +1,21 @@
+'use strict';
+const common = require('../common');
+const assert = require('assert');
+const net = require('net');
+
+const server = net.createServer((conn) => {
+  conn.end();
+  server.close();
+}).listen(common.PORT, () => {
+  const client = net.connect(common.PORT, () => {
+    assert.strictEqual(client.connecting, false);
+
+    // Legacy getter
+    assert.strictEqual(client._connecting, false);
+    client.end();
+  });
+  assert.strictEqual(client.connecting, true);
+
+  // Legacy getter
+  assert.strictEqual(client._connecting, true);
+});
