Implement complete close handshake #160
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
@nhooyr one more case where this is important: we had problems with Swift WebSocket libraries which disconnect with Abnormal closure status code if closing handshake was not properly done and sending closing frame from client get a TCP RST on balancer as connection have been already closed by server. |
Closed
Sounds good. Glad to hear this will fix that. |
I changed my mind after #103 as browsers include a wasClean event to indicate whether the connection was closed cleanly. From my tests, if a server using this library prior to this commit initiates the close handshake, wasClean will be false for the browser as the connection was closed before it could respond with a close frame. Thus, I believe it's necessary to fully implement the close handshake. @stephenyama You'll enjoy this.
nhooyr
force-pushed
the
close
branch
3 times, most recently
from
0c77bfe to
4fc87a5
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
I changed my mind after #103 as browsers include a wasClean field in the close event to indicate
whether the connection was closed cleanly. From my tests, if a server using
this library prior to this commit initiates the close handshake, wasClean
will be false for the browser as the connection was closed before it could
respond with a close frame. Thus, I believe it's necessary to fully implement
the close handshake.
@stephenyama You'll enjoy this.
Closes #161