socket.connecting is false before calling the connect listener

[szmarczak]

• Version: v11.6.0
• Platform: Ubuntu 18.10
• Subsystem: net

According to the docs:

socket.connecting
[...] Will be set to true before emitting 'connect' event and/or calling socket.connect(options[, connectListener])'s callback.

I don't think socket.connecting should be false before emitting the connect event.

Demo: https://runkit.com/szmarczak/5c2e555cc7b0160012352b15

It breaks for example http-timer.

[sam-github]

You didn't say whether you felt the docs were wrong or the behaviour was wrong, but reading behind the lines, I think you found the docs unclear.

Does #25333 help?

[szmarczak]

You didn't say whether you felt the docs were wrong or the behaviour was wrong

I've said that indirectly. Anyway, I'm saying it now: I feel the behavior is wrong.

Does #25333 help?

👍, but I still don't know if the described behavior above is right or it's just a bug.

[sam-github]

I don't understand why you think its a bug.

You provided code, but what is it about the code that you think is wrong? What do you think it should do differently?

[szmarczak]

You provided code, but what is it about the code that you think is wrong? What do you think it should do differently?

I think it should do differently because the connection hasn't been fully estabilished yet.
The connect event occurs when it has been fully estabilished. Until that socket.connecting should be true.

[szmarczak]

Or maybe there's another way to check if the socket has been estabilished?

[sam-github]

You don't want to know when the TCP connection is established, it seems you want to know when the TLS handshake has completed. .connecting does not, as you see, tell you that.

Maybe tls.TLSSocket should override the meaning of the connect event, and .connecting property, so that the socket isn't considered connected until the TLS handshake has completed.... but that isn't how it was done, and it would be a really big and backwards incompatible change to change them now.

The callback function to tls.connect() is attached to the secureConnect event (unlike the cb function to net.connect(), which is attached to connect).

Generally, waiting for events is preferred to polling properties for their state, but on the client side you could look at https://nodejs.org/api/tls.html#tls_tlssocket_authorized, it will be set to true just before secureConnect is emitted.

[szmarczak]

You don't want to know when the TCP connection is established, it seems you want to know when the TLS handshake has completed. .connecting does not, as you see, tell you that.

Nope (maybe?) 😄 I want to see if the TCP connection has been established: for example, http-timer needs this - it works with the http module too (where TCP socket is used, not TLS) ;)

You don't want to know when the TCP connection is established, it seems you want to know when the TLS handshake has completed. .connecting does not, as you see, tell you that.

Good to know ^_^

[sam-github]

I want to see if the TCP connection has been established

Then you can use the 'connect' event and .connecting property, and not use the callback to tls.connect(). If you want code that works for tls or net, change

[net,tls].connect(.., callback);

to

[net,tls].connect(...).once('connect', callback)

[szmarczak]

You don't get it. This issue is not solved yet.

not use the callback to tls.connect()

It's no difference if I add the connect listener through net.connect() or socket.on('connect, ...). Not sure what did you mean here.

Then you can use the 'connect' event and .connecting property

I have no idea how many times I've told you that socket.connecting becomes false before the connect event. It's not fully connected. See the demo.

Could you reopen this issue? Otherwise I'll make a new one.

[sam-github]

.connecting is supposed to become false before the connect event is emitted, as documented, and as implemented:

node/lib/net.js

Lines 1057 to 1066 in 9dfbc39

	self.connecting = false;
	self._sockname = null;
	if (status === 0) {
	self.readable = readable;
	if (!self._writableState.ended)
	self.writable = writable;
	self._unrefTimer();
	self.emit('connect');

Your example code shows that .connecting becomes false before the secureConnect event, as it should do. This is unrelated to your assertion (since it doesn't use the connect event).

Please provide example code, with a clear description of what about that code's behaviour you believe to be a bug.

[szmarczak]

Thanks for your interest, I really appreciate it! 🙌

Here's the example: https://runkit.com/szmarczak/5c2fc4a004d48e001292057c

On the finish event socket.connecting is false though the connect event hasn't been emitted yet. How to prove that it hasn't been fully connected at that point? Is it even possible? (without setting additional connect event - there may be a case when it has been already emitted for example)

[sam-github]

@Trott I think this should be moved to nodejs/help

@szmarczak

On the finish event socket.connecting is false though the connect event hasn't been emitted yet.

Sorry, while I am not an expert in the HTTP sub-system, this doesn't surprise me. finish is triggered by your call to .end(), see https://nodejs.org/api/stream.html#stream_event_finish

You are probably seeing the finish event before the underlying socket has had .connect() called on it.

Also, there is connection pooling by the agent with request. Its possible that the socket was already connected, and is being reused, in which case an http.request() will not even cause a .connect() call to be made on a socket.

I'd suggest asking in nodejs/help, describing what you are trying to accomplish, and perhaps someone can help you find a way.

[szmarczak]

Hmm. I assumed that tls.connect(..., listener) stands for the connect event. It stands for the secureConnect event. secureConnect is not connect, so I was wrong from the beginning.

Sorry for disturbing you guys. Feel free to move this to the help repo.