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

Reconnect to BitMex if the websocket disconnects #1014

Merged
merged 1 commit into from
Jul 27, 2023
Merged

Conversation

Restioson
Copy link
Contributor

@Restioson Restioson commented Jul 26, 2023

Adds a timeout to the websocket to bitmex in order to provoke a reconnect if the websocket becomes unresponsive. Fixes #919.

@Restioson Restioson requested a review from holzeis July 26, 2023 10:53
@Restioson
Copy link
Contributor Author

Restioson commented Jul 26, 2023

I'm wondering why this piece of code would not already work to reconnect to bitmex? I just found this now.

@Restioson
Copy link
Contributor Author

Restioson commented Jul 26, 2023

Ah, even prolonged physical disconnection of my PC's ethernet cable (5min) does not provoke this code to execute. It seems like there is no timeout in place from the client side at all, and sends do not fail either, because we throw away the result of the ping. The way I see it, we can either propagate the error upward, add a timeout, or both.

EDIT: it looks like the send doesn't fail... strange. I guess we need to have a timeout then.

@Restioson Restioson marked this pull request as ready for review July 26, 2023 16:27
@Restioson
Copy link
Contributor Author

Restioson commented Jul 26, 2023

New fix works!

@bonomat I've also requested your review here as well because, like mentioned in #919, it's possible that the scheduled restart could be removed with this fix in (at least from a testing setup first to make sure it works...)

This is to prevent the websocket from getting stuck if disconnected, as pings will still return Ok(()) even if the link layer fails
Copy link
Contributor

@bonomat bonomat left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's get this in. Thank you.

@bonomat
Copy link
Contributor

bonomat commented Jul 27, 2023

bors r+

@bonomat
Copy link
Contributor

bonomat commented Jul 27, 2023

bors r-

@bors
Copy link
Contributor

bors bot commented Jul 27, 2023

Canceled.

bors bot added a commit that referenced this pull request Jul 27, 2023
1014: Reconnect to BitMex if the websocket disconnects r=bonomat a=Restioson

Adds a timeout to the websocket to bitmex in order to provoke a reconnect if the websocket becomes unresponsive. Fixes #919.

Co-authored-by: Restioson <[email protected]>
@bonomat bonomat merged commit 849f986 into main Jul 27, 2023
7 checks passed
@bonomat bonomat deleted the reconnect-to-bitmex branch July 27, 2023 08:48
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.

The maker does not reconnect after losing the WebSocket connection to BitMEX
2 participants