-
Notifications
You must be signed in to change notification settings - Fork 140
Reconnect fix #201
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
Reconnect fix #201
Conversation
|
Fixes #199 |
|
does this mean there will not be any reconnect attempts anymore after this fix? |
|
No, this just correctly resets the reconnection counter so that the reconnection logic works again a second time. Before, if the connection failed and six reconnection attempts were exceeded, and the user then decided to call connect again, it would retry once, and then fail. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good, but I think this needs a test for the new behaviour.
29f4064 to
b32079e
Compare
|
I added a test that fails at Line 402 in b32079e
Without the fix in this PR. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
Description of the pull request
This PR resets the reconnectAttempts int to zero when the state machine transitions to disconnected. This means that the reconnect logic will trigger whenever .connect() is called again.
Why is the change necessary?
It's weird that the reconnection logic only works until it's exhausted, and then never again.