TCP socket won't call end() callback if wasn't connected

[Antonius-S]

Version

v18.15.0

Platform

Win 8.1 x64

Subsystem

net

What steps will reproduce the bug?

'use strict';

const net = require('net');
const stream = require('stream');

function check(stm)
{
  stm.on('finish', () => console.log('finish'));
  stm.on('close', () => console.log('close'));
  stm.end(() => console.log('ended'));
}

// nothing is printed
check(new net.Socket());
// 'finish' and 'ended' is printed
// check(new stream.Writable());

How often does it reproduce? Is there a required condition?

always

What is the expected behavior? Why is that the expected behavior?

Expect at least 'ended' to show

What do you see instead?

nothing printed

Additional information

All streams seem to always call the callback from end() but non-connected sockets do not thus violating stream.Writable API.

Why it is needed: I use universal function to close stream gracefully and when it's done call destroy. With this behavior the function stucks at non-connected sockets. Currently I seem to have to check stream.writable and only call end when it's true.

[Antonius-S]

Well, base Writable works as expected while Socket does not. So Socket violates its parent's behavior and thus makes impossible some flows dealing with generic streams.

[bnoordhuis]

So, I don't think the behavior is too surprising.

You're comparing net.Socket to a stream.Writable but it's an instance of stream.Duplex - i.e., it's bidrectional, as @jazelly points out.

What's more, the stream-y part of the socket is never activated; no data goes in or out. It would be more surprising IMO if the stream events did fire.

[Antonius-S]

Writable works as it is a unidirectional stream. It closes when you end one stream. But socket is bidirectional. Ending a socket only ends the writable part. It's still readable. This has been implied from the documentation on Socket.end().

You're comparing net.Socket to a stream.Writable but it's an instance of stream.Duplex - i.e., it's bidrectional, as @jazelly points out.

But stream.Duplex does produce 'finish' and 'ended' output!
I'm not insisting on events here, but I think if it is allowed to run a method with callback, it must be called always. Or just let it throw error.

[bnoordhuis]

The documentation for socket.end() says this:

Half-closes the socket. i.e., it sends a FIN packet. It is possible the server will still send some data.

And it has this to say on the callback argument:

callback {Function} Optional callback for when the socket is finished.

But the socket never finishes (because it is never started) so I feel it's unsurprising that the callback isn't called. For the same reason it's unsurprising no 'close' or 'finish' events are emitted.

That's my opinion. I'll keep this issue open for a few days and see if other maintainers feel differently.

[ronag]

Based on a very quick look at this I think the callback should be called. I'll try to find time to dig into this.

[jazelly]

What happens here is the prefinish() check in Writable, which will not destroy a pending socket until it's connected.

See here.

The pending getter returns true in this case, as it simply regards any socket that is connecting or not handling data as pending, and defers the final call.

IMO it's better to differentiate the cases of pending, one is user has attempted to connnect while waiting for the connection, the other is this, constructed but never started the attempt.

[ronag]

@jazelly That sounds like a correct analysis. PR welcome!