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

support refreshing connection config during reconnect #66

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

joshk
Copy link

@joshk joshk commented Apr 25, 2024

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
Copy link
Collaborator

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
Copy link
Author

joshk commented Apr 30, 2024

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)

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.

2 participants