Skip to content
Closed
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions doc/api/net.md
Original file line number Diff line number Diff line change
Expand Up @@ -400,6 +400,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
Expand Down
2 changes: 1 addition & 1 deletion lib/_tls_legacy.js
Original file line number Diff line number Diff line change
Expand Up @@ -477,7 +477,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';
Expand Down
10 changes: 5 additions & 5 deletions lib/_tls_wrap.js
Original file line number Diff line number Diff line change
Expand Up @@ -272,7 +272,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;
Expand Down Expand Up @@ -466,9 +466,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');
});
}
Expand All @@ -480,7 +480,7 @@ TLSSocket.prototype._init = function(socket, wrap) {
});
} else {
assert(!socket);
this._connecting = true;
this.connecting = true;
}
};

Expand Down Expand Up @@ -581,7 +581,7 @@ TLSSocket.prototype._finishInit = function() {
};

TLSSocket.prototype._start = function() {
if (this._connecting) {
if (this.connecting) {
this.once('connect', function() {
this._start();
});
Expand Down
35 changes: 21 additions & 14 deletions lib/net.js
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,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;
Expand Down Expand Up @@ -202,7 +202,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);
}
Expand Down Expand Up @@ -367,9 +367,16 @@ Socket.prototype.address = function() {
};


Object.defineProperty(Socket.prototype, '_connecting', {
get: function() {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should there be a deprecation message here?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ChALkeR Can you check how many people are using ._connecting in the wild?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@evanlucas it should be, will do it in a follow-up PR, because it won't be backported.

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';
Expand Down Expand Up @@ -397,7 +404,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) {
Expand Down Expand Up @@ -430,7 +437,7 @@ function maybeDestroy(socket) {
if (!socket.readable &&
!socket.writable &&
!socket.destroyed &&
!socket._connecting &&
!socket.connecting &&
!socket._writableState.length) {
socket.destroy();
}
Expand Down Expand Up @@ -465,7 +472,7 @@ Socket.prototype._destroy = function(exception, cb) {
return;
}

this._connecting = false;
this.connecting = false;

this.readable = this.writable = false;

Expand Down Expand Up @@ -648,7 +655,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() {
Expand Down Expand Up @@ -803,7 +810,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;

Expand Down Expand Up @@ -913,7 +920,7 @@ Socket.prototype.connect = function(options, cb) {

this._unrefTimer();

this._connecting = true;
this.connecting = true;
this.writable = true;

if (pipe) {
Expand Down Expand Up @@ -952,7 +959,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;
Expand Down Expand Up @@ -980,7 +987,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
Expand Down Expand Up @@ -1048,8 +1055,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) {
Expand All @@ -1065,7 +1072,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;
Expand Down
2 changes: 1 addition & 1 deletion test/parallel/test-net-connect-buffer.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand Down
21 changes: 21 additions & 0 deletions test/parallel/test-net-socket-connecting.js
Original file line number Diff line number Diff line change
@@ -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.equal(client.connecting, false);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

strictEqual would be better I guess.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ack.


// Legacy getter
assert.equal(client._connecting, false);
client.end();
})
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did this pass linter? I think semicolon is missing

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It doesn't, sorry! I added test after doing lint, so it didn't show it.

assert.equal(client.connecting, true);

// Legacy getter
assert.equal(client._connecting, true);
});