bitfinexcom / bitfinex-api-py

Apache License 2.0
200 stars 125 forks source link

Failed to reauth and resub after reconnection #39

Closed ph4z closed 4 years ago

ph4z commented 4 years ago

Issue type

Brief description

When running an authenticated websocket client, in case of deconnection, client is able to reconnect but not to re-authenticate and resubscribe to preivously subscribed channels.

Steps to reproduce

python bfxapi/examples/ws/wallet_balance.py & netstat -lataupen | grep 443 # Look for the remote api ip address ip route add blackhole ipaddress/32 # Add a blakhole route to api After few seconds client will connect to a new ip address but not reauthenticate and resubscribe

Additional Notes:
ph4z commented 4 years ago

It seems like the issue rely in the GenericWebsocket class, during reconnect attemps, the "s_connect" method is called and then "on_message" but "on_message" is just a pass function.

I don't really understand if it is the expected behaviour or not, but could anyone give me some insights or code snippets in order to implement a better way to reconnect ? Thanks for your help.

JacobPlaster commented 4 years ago

Hi @ph4z thanks for opening the issue. The logic that handles the resubscribe event is here:

https://github.com/bitfinexcom/bitfinex-api-py/blob/master/bfxapi/websockets/bfx_websocket.py#L461

The idea is that - on_open will be called when the socket is reconnected which will then go on to re-subscribe any existing channels. I think the issue may be with this line:

if self.API_KEY and self.API_SECRET and self.get_authenticated_socket() == None:
    await self._ws_authenticate_socket(socket_id)

Since its a resubscribe and we have previously had an authenticated socket, this line will be skipped meaning no sockets will be authenticated. I will add a fix to this when I get the chance (hopefully this week).

ph4z commented 4 years ago

Thanks for your reply @JacobPlaster it helped me a lot. I have created a PR for fixing this: #40

JacobPlaster commented 4 years ago

Ahh amazing! I'll take a look now

JacobPlaster commented 4 years ago

Merged, thanks very much for the help