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

fix: Reconnect failure #295

Merged
merged 1 commit into from
Jun 20, 2020
Merged

fix: Reconnect failure #295

merged 1 commit into from
Jun 20, 2020

Conversation

aragaer
Copy link
Contributor

@aragaer aragaer commented Mar 14, 2020

The "notified on disconnect" flag prevents double disconnects but once
it is set it is not being reset. So it does prevent double disconnect
event on first disconnect but on second disconnect it doesn't fire
"disconnected" event at all. This fix resets the "notified on
disconnect" flag before attempting new connection.

closes issue #294

@aragaer
Copy link
Contributor Author

aragaer commented Mar 14, 2020

This patch works with localhost connection but it seems it actually performs double reconnect when I'm using my custom transport. In this case I get the following:

  • heartbeat missed
  • receiver loop ends
  • notify("receiver_loop_completed") is called
    • "notified on disconnect" is set
    • on_disconnected method is called
    • reconnect happens
    • "notified on disconnect" is reset
  • "notified on disconnect" now is false again, so notify("disconnected") is called again

Looks like the better solution would be to have something like this instead:

logging.info("Receiver loop ended")
self.notify("receiver_loop_completed")
if notify_disconnected and not self.__notified_on_disconnect:
    self.notify("disconnected")
self.__notified_on_disconnect = False

The "notified on disconnect" flag prevents double disconnects but once
it is set it is not being reset. So it does prevent double disconnect
event on first disconnect but on second disconnect it doesn't fire
"disconnected" event at all. This fix resets the "notified on
disconnect" after skipping the "disconnected" event.

closes issue jasonrbriggs#294
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.

2 participants