Need to be able to update the original query_parameters

[boydm]

Hi Sasa,

The scenario is this:

• I've connected to the server with a token that was passed to GenSocketClient during the init function.
• Time passes and the token is now expired. I can get a new one before the expiration, but don't know how to update the original query_params with it.
• Something happens on the network to cause a disconnection. When I try to reconnect, it uses the old token, which will fail.

How can I update the query_params I passed in during init? Is this going to need a new function?

Thank you

[sasa1977]

Unfortunately, this is currently not officially supported.

There are two options here:

1. Introduce a helper function which can be used to modify the query_params field of the internal behaviour state.
2. Require the client to always return the query params when connecting. Currently this is mandated only in init/1, but not in other callbacks.

Option 1 is an easy fix, but I think that option 2 is a cleaner solution, although it would lead to a breaking change and required us to bump the major version.

If we go for option 2, I think we should also remove url from the internal state. This would mean that in every :connect response, a client would have to return the url and query params. This would allow the client to vary either of them (or both) when reconnecting.

Thoughts?

[boydm]

Number 2 - always declaring the url and params - is clearly the better solution. It also feels more "functional" in nature with less behind-the-scenes magic in state.

When my client makes the authentication api call, the server has the opportunity to tell it to go to a new url as well as giving a new token. This was very important in past services I've built in order to migrate users to a new cluster for either functionality or load reasons...

There is a way we could do this without breaking change (but I don't like it). We could make a new, optional, return value for the other callbacks. something like {:connect, url, params, state} without removing the existing one. Then match against it to do the right thing. Isn't as clean as it would still require keeping the url around in private state, but it wouldn't introduce a breaking change.

Personally, I think I would go for the breaking solution. Migrating the url and the query_params are super important for long-term production services and might be worth bumping the major version. Never storing the url/params in state increases transparency and makes it clear how to go about controlling the service overall.

The optional version isn't bad either tho. What do you think?

[sasa1977]

Yeah I think optional version would be a good immediate solution, mainly because I'm considering some other changes to the interface, and I don't want to introduce breaking changes very frequently. So I'd say let's support {:connect, url, params, state} in addition to {:connect, state}, and I'll think some more about the interface for 3.x.

[boydm]

Cool. I can work on the update later tonight unless it's easy for you to do. Just let me know if you are working on it so we don't both do the same thing...

[sasa1977]

I won't be able to work on this immediately, so feel free to take it :-)

[boydm]

Just submitted a proposed fix in a pull-request.

[boydm]

Fixed in PR #30, which was just merged.