NFIBrokerage / slipstream

A slick WebSocket client for Phoenix Channels
https://hex.pm/packages/slipstream
Apache License 2.0
155 stars 18 forks source link

support refreshing connection config during `reconnect` #66

Open joshk opened 5 months ago

joshk commented 5 months ago

This adds a callback to update the connection config when reconnect is called.

Although there is an example on how to capture 403 connection errors and fresh a uri token, this means you don't get to use the reconnect_after_msec logic built into reconnect.

While there are ways to use reconnect by updating channel_config, you run into an issue where the connection configuration is updated before the Process.send_after in reconnect, which can have a side effect of the connection configuration expiring if the reconnect delay is too long.

the-mikedavis commented 4 months ago

I'm unsure about this. reconnect/1 is intended for the connection being closed due to transient failures like if the server is restarting or unavailable briefly. If you need to change configuration, for example the token refreshing example in the docs, you shouldn't need retry-backoff for reconnecting. In cases where you do want to wait before reconnecting I believe you can do that with Process.send_after/4 and calling connect from within Slipstream:handle_info/2

joshk commented 4 months ago

This PR was driven because of how nerves_hub_link requires an auth token to be refreshed if the token being used is older than the 'max-age' set by the server, which is currently set to 1 minute in nerves_hub_web. Our default retry back-off can exceed this as we add some random jitter so that devices in the field don't all try to reconnect simultaneously.

If the server restarts or is briefly unavailable, using the retry back-off built into Slipstream and not having to replicate this logic ourselves is preferable, as this is primarily focused on mitigating against a thundering heard.

This API would allow us to simplify our code and use a cleaner API (I'm biased of course)