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

Fix connection not resuming after guest user goes to background #3483

Merged

Conversation

nuno-vieira
Copy link
Member

@nuno-vieira nuno-vieira commented Nov 7, 2024

🔗 Issue Links

#3338
https://linear.app/stream/issue/IOS-75/[reconnection]-[github]-websocket-disconnectes-when-guest-user-comes

🎯 Goal

Fixes connection not resuming after guest user goes to background

📝 Summary

  • Fixes connection not resuming after guest user goes to background
  • Improves Demo App Token Refreshing UX

🛠 Implementation

The root of the problem was that, in a previous PR, we introduced stopping the recovery handler on disconnect().
The reason why this is problematic is because of how our connect works. Especially for guest users and anonymous users, we have logOutFirst: true. This means that when connecting guest users and anonymous users, we will always logout first, and this means we call disconnect() which will stop the recovery handler. So the next time the user goes to background, the recovery handler will not be running, and so, it won't reconnect automatically. Here is the flow:

flowchart LR 
    B["ChatClient.connectUser()"]
    B --> C["RecoveryHandler.start()"]
    C --> D{logOutFirst?}
    D -->|Yes| E["ChatClient.logout()"]
    E --> F["RecoveryHandler.stop()"]
    F --> G["authRepository.connect()"]
    D -->|No| G
Loading

So the problem is that when we logout a user first, and then connect it back again, we do not call again start() on the recovery handler. Changing this would require a bit of a refactoring so better to play safe and simple for now.

All of this means that this could happen even for a regular user, not only guest users. If a regular user is logged in, then disconnects (without logging out) and then connects with a different user, and then going to background, it would happen the same issue, although this flow is quite rare and usually means a bad integration, since for different users, customers should always logout.

🧪 Manual Testing Notes

  • Open Demo App Configuration
  • Disable staysConnectedInBackground
  • Login with Guest User
  • Go to background, wait 5s to make sure everything is disconnected
  • Go back to foreground
  • Connection should resume

☑️ Contributor Checklist

  • I have signed the Stream CLA (required)
  • This change should be manually QAed
  • Changelog is updated with client-facing changes
  • Changelog is updated with new localization keys
  • New code is covered by unit tests
  • Comparison screenshots added for visual changes
  • Affected documentation updated (docusaurus, tutorial, CMS)

@nuno-vieira nuno-vieira requested a review from a team as a code owner November 7, 2024 18:34
@Stream-SDK-Bot
Copy link
Collaborator

Stream-SDK-Bot commented Nov 7, 2024

SDK Size

title develop branch diff status
StreamChat 6.92 MB 6.92 MB 0 KB 🟢
StreamChatUI 4.95 MB 4.95 MB 0 KB 🟢

@Stream-SDK-Bot
Copy link
Collaborator

SDK Performance

target metric benchmark branch performance status
MessageList Hitches total duration 10 ms 11.68 ms -16.8% 🔽 🔴
Duration 2.6 s 2.55 s 1.92% 🔼 🟢
Hitch time ratio 4 ms per s 4.59 ms per s -14.75% 🔽 🔴
Frame rate 75 fps 78.12 fps 4.16% 🔼 🟢
Number of hitches 1 1.2 -20.0% 🔽 🔴

Copy link

sonarqubecloud bot commented Nov 7, 2024

@@ -456,7 +456,6 @@ public class ChatClient {
/// Disconnects the chat client from the chat servers. No further updates from the servers
/// are received.
public func disconnect(completion: @escaping () -> Void) {
connectionRecoveryHandler?.stop()
Copy link
Contributor

Choose a reason for hiding this comment

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

Seems to be safe because recovery handler checks for current connection state and does not attempt to reconnect if the state is disconnected with userInitiated. 🤔

var canReconnectFromOffline: Bool {
        …
        switch webSocketClient.connectionState {
        case .disconnected(let source) where source == .userInitiated:
            return false

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes exactly the reconnection handler although alive, it won't do nothing

@nuno-vieira nuno-vieira merged commit 7d7145b into develop Nov 8, 2024
15 checks passed
@nuno-vieira nuno-vieira deleted the fix/connection-not-resuming-after-app-foregrounded branch November 8, 2024 12:18
@Stream-SDK-Bot Stream-SDK-Bot mentioned this pull request Nov 25, 2024
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.

4 participants