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

[DEV-11398] Remove workaround for invalid token in PhoenixSocket.connect() #6

Conversation

miguelcmedeiros
Copy link

Remove the workaround for handling an invalid token.

With the old implementation, the client will return null as the token in case it can't determine one (e.g. not authentication, token expired, etc.) and then PhoenixSocket.connect() expects this situation and inspects the token in the socket connection parameters. This package should not care about this implementation detail.

Instead, the client should throw an exception when it fails to get a valid token and PhoenixSocket.connect() should handle that exception when retrieving the (dynamic) parameters from the socket options.

@miguelcmedeiros miguelcmedeiros requested a review from kpsroka July 26, 2024 11:29
return;
} else {
throw PhoenixException();
} // else
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you can drop the comment now :D

expect(phoenixSocket.isConnected, isFalse);

// first retry after ~10 seconds
async.elapse(const Duration(seconds: 11));
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Time doesn't flow by itself in fakeAsync, you should be able to use exact durations from reconnectDelays. (also below)

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The delay used is coming from the list of reconnect delays passed in options plus a random duration between 0 and 1 second. That's why I went with custom reconnect delays where the 0-1 random second does not get in the way.

@miguelcmedeiros miguelcmedeiros merged commit dcfe22d into master Jul 26, 2024
1 check failed
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