-
Notifications
You must be signed in to change notification settings - Fork 2.3k
try to reconect on some exceptions #1594
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
base: master
Are you sure you want to change the base?
Conversation
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.
Pull Request Overview
This PR modifies the WebSocket reconnection logic to handle additional exceptions as recoverable instead of fatal, allowing the connection to automatically retry rather than completely failing.
- Adds
ConnectionClosedOK
exception handling for graceful connection closures - Moves
BinanceWebsocketUnableToConnect
from fatal to recoverable exception category
@@ -15,9 +15,9 @@ | |||
pass | |||
|
|||
try: | |||
from websockets.exceptions import ConnectionClosedError # type: ignore | |||
from websockets.exceptions import ConnectionClosedError, ConnectionClosedOK # type: ignore |
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.
The PR title contains a spelling error: 'reconect' should be 'reconnect'.
Copilot uses AI. Check for mistakes.
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.
Hi @cogisum.
Thanks for the PR!
I believe it's correct to break the loop when throwing BinanceWebsocketUnableToConnect as this means the loop has tried to reconnect several times unsuccesfully.
As for the ConnectionClosedOK exception, I believe it's also correct to break the loop as the connection was closed cleanly.
Could you explain a bit more the issue you are trying to solve?
Hi @pcriadoperez , the PR is for |
Give it a chance to reconnect on some non-fatal exceptions.