search

NFIBrokerage / slipstream

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

Change to reconnect/1 API #48

Closed cadebward closed 2 years ago

cadebward commented 2 years ago

Would you be open to changing the API of reconnect/1 to also accept the same options as connect/2?

Use Case

If you are working with an authenticated websocket server and your token expires (403), you'll want to refresh the token and then reconnect with a new query param. If reconnect/1 also took the same options as connect/2, you could handle the token refresh in response to a 403 and rebuild the uri inside of handle_disconnect.

I am open to helping out here, but I wanted to check first if the changes made sense. What do you think? Or is there a better way to handle this scenario?

Thanks

the-mikedavis commented 2 years ago

So, really the only difference between reconnect/1 and connect/2 is that reconnect/1 attempts to connect with some delay which backs off. For the situation where the connection failed because of an invalid request, I think it makes sense to reconnect immediately with new configuration (via connect/2). reconnect/1 is geared towards transient disconnects like 503s.

On the other hand, a reconnect/2 that takes a new configuration as an optional second argument would fit nicely in the API. I suppose it just depends whether you would want that reconnection to have the retry-backoff behavior or not. What do you think?

cadebward commented 2 years ago

You are absolutely right, immediately connecting due to an invalid request is correct. It did not cross my mind that connect/2 could be reused inside of a handle_disconnect. Perhaps I could contribute an example around token auth and refreshing tokens when they expire.