Sitelet https://github.com/nodejs/node/issues/47322
Skip to content

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

Description

@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.

Activity

  1. added
    netIssues and PRs related to the net subsystem.
    on Mar 30, 2023
  2. jazelly commented on Mar 30, 2023

    @jazelly
  3. Antonius-S commented on Mar 31, 2023

    @Antonius-S
    Author

    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.

  4. jazelly commented on Mar 31, 2023

    @jazelly
  5. bnoordhuis commented on Mar 31, 2023

    @bnoordhuis
    Member

    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.

  6. Antonius-S commented on Mar 31, 2023

    @Antonius-S
    Author

    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.

  7. bnoordhuis commented on Mar 31, 2023

    @bnoordhuis
    Member

    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.

  8. ronag commented on Apr 1, 2023

    @ronag
    Member

    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.

  9. jazelly commented on Apr 3, 2023

    @jazelly
    Member

    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.

  10. ronag commented on Apr 3, 2023

    @ronag
    Member

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    netIssues and PRs related to the net subsystem.

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions