Skip to content
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

Adjust client reconnect #81

Merged
merged 2 commits into from
Sep 27, 2023
Merged

Adjust client reconnect #81

merged 2 commits into from
Sep 27, 2023

Conversation

UkoeHB
Copy link
Collaborator

@UkoeHB UkoeHB commented Sep 26, 2023

Problem

  • Reconnect attempt is delayed #78
  • The client actor will continue trying to reconnect even if the client itself dies.
  • Messages sent to the client while reconnecting will pile up in to_socket_receiver. Normally, websocket messages that fail to send because the websocket is not connected are simply discarded. Note that there is actually a race condition inside the client actor between self.to_socket_receiver.recv() and self.socket.stream.recv() where an incoming user message will reach self.socket.send() when the socket is disconnected even though you want the stream to capture disconnects. That means in the current code it is possible for client messages to be dropped.
  • reconnect_interval is an Option but is never None.

Solution

  • Move reconnect delay until after reconnect attempt so the first attempt is immediate.
  • Kill the reconnect loop if the client is dead.
  • Discard incoming user messages while trying to reconnect.
  • Update reconnect_interval so it's not an Option.

Future work

  • Currently there is no way to report to the user that a message failed to send. I don't know the best way to solve this. (EDIT: solved by Track message status #83)
  • A message can fail to send for multiple reasons. If the reason is 'connection closed', ignore it (the stream will output a close frame eventually). If the error is IO/TLS error, then use on_disconnect and maybe try to reconnect. All other errors should cause the client actor to shut down.

… the client itself is dead instead of hanging
@UkoeHB UkoeHB merged commit 97c81c8 into gbaranski:master Sep 27, 2023
3 checks passed
@UkoeHB UkoeHB deleted the fix_reconnect branch September 27, 2023 19:53
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