Implement close handshake

[stephenyama]

The package does not implement the closing handshake as required by the RFC. The package closes the network connection after sending the close message. Any pending or future reads return immediately with an error derived from the close message. The application cannot read until a close message is sent by the peer or a timeout.

I didn't realize that this is a problem until recently. I incorrectly assumed that a CloseError represents a received close message.

[nhooyr]

I know the RFC says it's required but it's not clear to me why and I couldn't find any information supporting its claim that TCP's closing handshake isn't always reliable. In fact, I found several blog posts criticizing the RFC for effectively reimplementing the TCP close handshake. Will link them if I can find it again.

Pretty much every protocol in existence relies on TCP's close being reliable.

The RFC also claims that the TCP handshake being closed by the server is better than the client as it enables the client to reuse ports better but the client has practically 65k ports. I don't see this being an issue in practice, nor is it an issue with any other protocol.

Thus, I don't think the WebSocket close handshake adds anything.

Yea, CloseError represents the close reason and status of the connection which can either be from a received or sent close frame.

This decision should definitely be documented better.

[nhooyr]

The application cannot read until a close message is sent by the peer or a timeout.

What do you mean by that? The application can't read as the connection is closed.

[nhooyr]

Argh, there is one use case I didn't think of.

An application might want to send a bunch of messages to its peer, then close the connection with normal closure.

Right now, as the connection is closed immediately, there is no guarantee that the peer received all of the application's messages. Whereas if the close handshake was implemented and the status code was echoed by the peer, then it is guaranteed that the peer received all the application's messages.

There may be applications that rely on this guarantee so I will go ahead and implement this. It's very easy.

[nhooyr]

Actually there is no guarantee. Applications need to implement such a guarantee by themselves with an extra message because the close frame from the peer is not necessarily the echoed close frame, even if the status codes match. See https://tools.ietf.org/html/rfc6455#section-7.1.5

I'm again against implementing this.

Would it help your use case if *CloseError was only returned on received frames and for sent frames it was instead just the string?

[nhooyr]

Nvm, I'm wrong. The peer would just guarantee it never sends StatusNormalClosure unless the application code sends it first which would make it ok.

[stephenyama]

You figured out the use case, but I'll add this since you mention TCP: In the TCP protocol, you can tell the peer that no more data is coming while continuing to read data from the peer. The Go standard library exposes this functionality through the socket CloseWrite method.

The application can't read as the connection is closed.

That strikes at the core of this issue. The package fully closes the connection when it should not.

Would it help your use case if *CloseError was only returned on received frames and for sent frames it was instead just the string?

That will make the issue more obvious, but it does not address the issue.

[nhooyr]

The application cannot read until a close message is sent by the peer or a timeout.

To be clear, I didn't understand what you meant by that part. How would a message sent by the peer or a timeout allow the application to read again?

[nhooyr]

So the use case I discussed doesn't necessarily work in the general case. E.g. if you're processing messages concurrently from the read loop like in javascript or with this library, you'd respond with a close frame before necessarily fully processing/validating the peer's messages at which point the close frame means nothing.

I think you're better served with an application level message that acts as CloseWrite.

[nhooyr]

What I will do is document this better and make sure CloseError is only accessible via xerrors.As for received close frames.