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

conditionally restart connection after refetch or reauth #26

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

Conversation

erquhart
Copy link

@erquhart erquhart commented Jan 23, 2025

Fixes #22

  • Move resumeSocket() out of refetchToken() and into setConfig(), which is the only place a pause is initiated. This clarifies where this call actually needs to occur.
  • After setting a new token with refetchToken() or tryToReauthenticate(), if the connection is stopped, restart it. Because it's possible for these two functions to run before or after one another for multiple reasons, this ensures a connection doesn't remain stopped after a potentially successful reauth.
  • Change restartSocket() to no longer throw if attempted on an unstopped connection, similar to how resume works. Restart is only attempted by refetchToken() and tryToReauthenticate(), and there's no guarantee whether the connection will be stopped or not.

@erquhart erquhart marked this pull request as draft January 23, 2025 21:46
@erquhart erquhart force-pushed the fix-dropped-connections branch from ae2c106 to ecbbaec Compare January 23, 2025 21:52
@erquhart erquhart marked this pull request as ready for review January 23, 2025 22:00
@@ -301,7 +307,9 @@ export class AuthenticationManager {
this._logVerbose(
"resuming WS after auth token fetch (if currently paused)",
);
this.resumeSocket();
Copy link
Author

Choose a reason for hiding this comment

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

This isn't removed, it's relocated to the only place where the socket is paused. Every other resume call here (which is every other call after initialization) doesn't do anything.

@erquhart erquhart marked this pull request as draft January 25, 2025 03:28
@erquhart erquhart force-pushed the fix-dropped-connections branch 2 times, most recently from c77f65b to 4a406d1 Compare January 25, 2025 06:51
@erquhart erquhart force-pushed the fix-dropped-connections branch from 4a406d1 to b16a044 Compare January 25, 2025 06:53
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.

Connection closes without recovery
1 participant