Skip to content

More robust handling of Connection#close. #12

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 8 commits into from
Mar 7, 2023
Merged

Conversation

ioquatix
Copy link
Member

@ioquatix ioquatix commented Mar 7, 2023

The WebSocket RFC states:

If an endpoint receives a Close frame and did not previously send a
   Close frame, the endpoint MUST send a Close frame in response.  (When
   sending a Close frame in response, the endpoint typically echos the
   status code it received.)  It SHOULD do so as soon as practical.  An
   endpoint MAY delay sending a Close frame until its current message is
   sent (for instance, if the majority of a fragmented message is
   already sent, an endpoint MAY send the remaining fragments before
   sending a Close frame).  However, there is no guarantee that the
   endpoint that has already sent a Close frame will continue to process
   data.

   After both sending and receiving a Close message, an endpoint
   considers the WebSocket connection closed and MUST close the
   underlying TCP connection.  The server MUST close the underlying TCP
   connection immediately; the client SHOULD wait for the server to
   close the connection but MAY close the connection at any time after
   sending and receiving a Close message, e.g., if it has not received a
   TCP Close from the server in a reasonable time period.

   If a client and server both send a Close message at the same time,
   both endpoints will have sent and received a Close message and should
   consider the WebSocket connection closed and close the underlying TCP
   connection.

This is not handled correctly in the current code, as we mostly don't care about waiting for a close message. This PR makes ignores failures to write the close message.

socketry/async-websocket#50

Types of Changes

  • Bug fix.

Contribution

@ioquatix ioquatix force-pushed the close-ignore-errors branch from 309c35b to 1f43012 Compare March 7, 2023 11:46
@ioquatix ioquatix merged commit e53c542 into main Mar 7, 2023
@ioquatix ioquatix deleted the close-ignore-errors branch March 7, 2023 20:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant