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

[AND-205] Improve network connectivity check in the NetworkStateProvider. #5538

Merged
merged 3 commits into from
Jan 3, 2025

Conversation

VelikovPetar
Copy link
Contributor

@VelikovPetar VelikovPetar commented Dec 30, 2024

🎯 Goal

Linear: https://linear.app/stream/issue/AND-205/race-condition-in-networkstateprovider-causes-the-connectionstate-to
Fixes a rare bug where turning off the network connectivity on the device doesn't update the ConnectionState to Offline.
The cause for this is that it is not safe to call ConnectivityManager.getNetworkCapabilities(Network) from the ConnectivityManager.NetworkCallback.onLost callback, as there is no guarantee that the ConnectivityManager state will be in the updated state from the callback. In some occasions, calling NetworkStateProvider.isConnected() from the NetworkCallback.onLost callback returns true, which is a false positive. See details: https://developer.android.com/reference/android/net/ConnectivityManager.NetworkCallback#onLost(android.net.Network)

🛠 Implementation details

  • Keep track of available networks.
  • If there are no available networks, notify the listeners that the network was disconnected

🎨 UI Changes

NA

🧪 Testing

This was difficult to reproduce with a real device, but it happened quite consistently on an Emulator. To easily reproduce the issue (and then test the fix), you can:

  1. Create emulator (ex: Pixel 7, Android API 34)
  2. Install the compose sample app
  3. Apply the provided patch to easily log the changes in the ConnectionState
  4. Turn all networks off (wifi and mobile)
  5. In roughly 1 in 4 attempts, the ConnectionState will NOT change to Offline when turning off the connectivity, even though the device is not connected to the internet. If you are testing with the fixed version, this should no longer happen.
Provide the patch summary here
Index: stream-chat-android-compose-sample/src/main/java/io/getstream/chat/android/compose/sample/ChatHelper.kt
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/stream-chat-android-compose-sample/src/main/java/io/getstream/chat/android/compose/sample/ChatHelper.kt b/stream-chat-android-compose-sample/src/main/java/io/getstream/chat/android/compose/sample/ChatHelper.kt
--- a/stream-chat-android-compose-sample/src/main/java/io/getstream/chat/android/compose/sample/ChatHelper.kt	(revision f9ca74e2c1b8086e4f02a639b63a8b5d89c9f3b1)
+++ b/stream-chat-android-compose-sample/src/main/java/io/getstream/chat/android/compose/sample/ChatHelper.kt	(date 1735569470902)
@@ -33,7 +33,11 @@
 import io.getstream.chat.android.state.plugin.config.StatePluginConfig
 import io.getstream.chat.android.state.plugin.factory.StreamStatePluginFactory
 import io.getstream.result.Error
+import kotlinx.coroutines.CoroutineScope
+import kotlinx.coroutines.Dispatchers
+import kotlinx.coroutines.SupervisorJob
 import kotlinx.coroutines.flow.transformWhile
+import kotlinx.coroutines.launch
 
 /**
  * A helper class that is responsible for initializing the SDK and connecting/disconnecting
@@ -44,6 +48,8 @@
 
     private const val TAG = "ChatHelper"
 
+    private val scope = CoroutineScope(SupervisorJob() + Dispatchers.IO)
+
     /**
      * Initializes the SDK with the given API key.
      */
@@ -90,6 +96,11 @@
                 }
             }
             .build()
+        scope.launch {
+            ChatClient.instance().clientState.connectionState.collect {
+                Log.d("ChatHelper", "Connection state: $it")
+            }
+        }
     }
 
     /**

@VelikovPetar VelikovPetar marked this pull request as ready for review December 30, 2024 14:53
@VelikovPetar VelikovPetar requested a review from a team as a code owner December 30, 2024 14:53
Copy link
Contributor

github-actions bot commented Jan 3, 2025

SDK Size Comparison 📏

SDK Before After Difference Status
stream-chat-android-client 3.04 MB 3.04 MB 0.00 MB 🟢
stream-chat-android-offline 3.25 MB 3.25 MB 0.00 MB 🟢
stream-chat-android-ui-components 7.91 MB 7.91 MB 0.00 MB 🟢
stream-chat-android-compose 9.04 MB 9.04 MB 0.00 MB 🟢

@JcMinarro JcMinarro enabled auto-merge (squash) January 3, 2025 16:13
@JcMinarro JcMinarro force-pushed the bug/fix_network_state_provider_race_condition branch from 9f99dbd to 27a0bd3 Compare January 3, 2025 16:13
@JcMinarro JcMinarro merged commit c87f2c0 into develop Jan 3, 2025
10 checks passed
@JcMinarro JcMinarro deleted the bug/fix_network_state_provider_race_condition branch January 3, 2025 17:04
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