Always send subscribe request when connecting to streaming #14579
Conversation
Needed for reconnection to streaming after restart of streaming server.
|
Hm, wouldn't that incorrectly bump the number of subscriptions from each channel at each reconnect? |
|
Not on the client side. The server may see extra subscription requests. I naively assumed it is idempotent on the server side. |
|
It is, but as far as I understand the issue, I meant, your change does fix the client not reconnecting, but as far as I understand, the refcounting remains broken in such a way that the count doesn't correspond to active subscriptions anymore, and unsubscribing from the rest of the UI codebase won't actually unsubscribe from the server because of it. |
|
Got it! Honestly, I didn't fully understand the logic behind the counter and I tried to avoid touching it... |
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() This might be a better solution than mastodon#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.
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 #14579 to recover subscriptions after a reconnect.
|
Is this still needed after #14608? |
|
It can't hurt, but I don't believe it is needed. |
|
The original problem is fixed with #14608. This may still be good to have as the code is more resilient to inconsistent counts. On the other hand, this might hide the direct cause of a problem we may discover later. |
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.
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.
I noticed my browser stops receiving notifications after the streaming server restarts. This change makes the browser to send subscribe request every time after the WebSocket is reconnected so that the browser keeps receiving notifications when the streaming server comes back.