Start the client WebSocket closing timeout after sending the close frame

[vietj]

Motivation:

When closing a client WebSocket, the client sends a close frame and expects the server to respond with a close frame and then close the connection.

Some servers can misbehave by ignoring the close frame and this let the client deal with the socket.

The client has a closing timeout which handle the case where the server responds with a close frame but does not actually close the connection.

The timeout can be extended to handle the case where the server does not send a close frame.

Changes:

Start the closing timeout after sending successfully the close frame instead of upon reception of the server close frame.

[emattheis]

@vietj this code resolves #5592 but introduces a small concern: it is not possible to determine if the connection was closed normally within the close handler since the close status is set to the status of the initial frame in the closing handshake. I suppose this is an entirely separate issue, but thought I would point it out. Perhaps we should avoid firing the close handler if a close frame was not received?

[vietj]

@vietj this code resolves #5592 but introduces a small concern: it is not possible to determine if the connection was closed normally within the close handler since the close status is set to the status of the initial frame in the closing handshake. I suppose this is an entirely separate issue, but thought I would point it out. Perhaps we should avoid firing the close handler if a close frame was not received?

@vietj this code resolves #5592 but introduces a small concern: it is not possible to determine if the connection was closed normally within the close handler since the close status is set to the status of the initial frame in the closing handshake. I suppose this is an entirely separate issue, but thought I would point it out. Perhaps we should avoid firing the close handler if a close frame was not received?

that's a good point, we can use the status code 1006 to indicate that : Abnormal Closure

[vietj]

@emattheis I think actually it does not apply to this case because the client initiates the close operation and provides a status code. The server should respond with a close frame that is an echo of the client sent close frame so the status should be the same value as provided by the client that initiated the closure.

[emattheis]

that's a good point, we can use the status code 1006 to indicate that : Abnormal Closure

This gets tricky, though, because the sent and received close frames should always match. We don't want to misrepresent what went over the wire. Per the Vert.x API the close status and reason should be null, when no close frame has been received from the remote endpoint. I think the deeper issue here is that the status and reason are sent to the value from the local endpoint if initiated locally.

[vietj]

@emattheis the main difference with the normal case is that vertx end handler will not be called and instead the exception handler will be

[vietj]

@emattheis what you are saying kind of makes sense too, but changing that would be breaking with users expecting a status code not null

[vietj]

I think the only case then is to use status code 1006 instead which signals this case instead

[vietj]

@emattheis I updated the code with the usage of 1006 (https://datatracker.ietf.org/doc/html/rfc6455#section-7.1.5)

[emattheis]

@emattheis I updated the code with the usage of 1006 (https://datatracker.ietf.org/doc/html/rfc6455#section-7.1.5)

Personally, I think this is more confusing, because we have no way to tell if that was actually the closure code set by the client or the server or Vert.x. At least in the current release we know the code was set (explicitly or implicitly) by calling WebSocket::close, or was received from the remote endpoint.

On the other hand, I guess if I really want to know if the close frame was received from the remote endpoint I can register an exception handler and look for the closed exception, so it won't matter if the status code is changed by Vert.x.