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

Better manage subscriptionCounters #14608

Merged
merged 1 commit into from
Aug 24, 2020

Conversation

zunda
Copy link
Contributor

@zunda zunda commented Aug 20, 2020

Before this change:

  • unsubscribe() was not called for a disconnection
  • subscriptionCounters were incremented twice for a single reconnection, first from connected() and second from reconnected(). It seems that WebSocketClient calls both connected() and reconnected() when a reconnection happens.

This might be a an additional change to #14579 to recover subscriptions after a reconnect.

Before this change:
- unsubscribe() was not called for a disconnection
- It seems that WebSocketClient calls connected() and reconnected().
  subscriptionCounters were incremented twice for a single reconnection,
  first from connected() and second from reconnected()

This might be a an additional change to
mastodon#14579
to recover subscriptions after a reconnect.
Copy link
Member

@Gargron Gargron left a comment

Choose a reason for hiding this comment

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

I understand removing reconnect makes sense. But with unsubscribe calling send, wouldn't it cause problems to try that on a disconnected websocket?

@ClearlyClaire
Copy link
Contributor

I understand removing reconnect makes sense. But with unsubscribe calling send, wouldn't it cause problems to try that on a disconnected websocket?

It only does that if sharedConnection.readyState === WebSocketClient.OPEN so I guess it should be fine?

@Gargron Gargron merged commit 9669557 into mastodon:master Aug 24, 2020
thenameisnigel-old pushed a commit to ChatterlyOSE/Chatterly that referenced this pull request Sep 6, 2020
Before this change:
- unsubscribe() was not called for a disconnection
- It seems that WebSocketClient calls connected() and reconnected().
  subscriptionCounters were incremented twice for a single reconnection,
  first from connected() and second from reconnected()

This might be a an additional change to
mastodon#14579
to recover subscriptions after a reconnect.
thenameisnigel-old pushed a commit to ChatterlyOSE/Chatterly that referenced this pull request Sep 7, 2020
Before this change:
- unsubscribe() was not called for a disconnection
- It seems that WebSocketClient calls connected() and reconnected().
  subscriptionCounters were incremented twice for a single reconnection,
  first from connected() and second from reconnected()

This might be a an additional change to
mastodon#14579
to recover subscriptions after a reconnect.
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.

3 participants