From 4f00e89dfe66a31911dc86c1d2b776bb59be191b Mon Sep 17 00:00:00 2001 From: Aleksandar Apostolov Date: Tue, 25 Aug 2026 18:38:36 +0200 Subject: [PATCH 1/4] fix(client): keep the cached network state truthful NetworkStateProvider only notifies listeners when its cached flag changes, but ChatSocket concluded "no network" from the live isConnected() read without the provider ever learning. The cached value stayed true, so the network coming back was not seen as a transition and listeners were never told, leaving the socket offline for the lifetime of the process. Record the result of every direct read, and route the "all networks lost" branch of onLost through the same writer so it cannot desync either. Also drop NET_CAPABILITY_VALIDATED from the check. It reports whether the platform's connectivity probe passed, not whether a socket can be opened, so captive portals and revalidating networks read as a hard outage. --- .../client/network/NetworkStateProvider.kt | 51 ++++--- .../network/NetworkStateProviderTest.kt | 130 ++++++++++++++++++ 2 files changed, 163 insertions(+), 18 deletions(-) create mode 100644 stream-chat-android-client/src/test/java/io/getstream/chat/android/client/network/NetworkStateProviderTest.kt diff --git a/stream-chat-android-client/src/main/java/io/getstream/chat/android/client/network/NetworkStateProvider.kt b/stream-chat-android-client/src/main/java/io/getstream/chat/android/client/network/NetworkStateProvider.kt index c23838932b3..cf0347e540c 100644 --- a/stream-chat-android-client/src/main/java/io/getstream/chat/android/client/network/NetworkStateProvider.kt +++ b/stream-chat-android-client/src/main/java/io/getstream/chat/android/client/network/NetworkStateProvider.kt @@ -47,16 +47,17 @@ internal class NetworkStateProvider( override fun onLost(network: Network) { availableNetworks.remove(network) - notifyListenersIfNetworkStateChanged() if (availableNetworks.isEmpty()) { - // No available networks, notify listeners about disconnection - listeners.onDisconnected() + // No available networks, the capability read may still lag behind + setConnected(false) + } else { + notifyListenersIfNetworkStateChanged() } } } @Volatile - private var isConnected: Boolean = isConnected() + private var lastKnownConnected: Boolean = queryConnectivity() @Volatile private var listeners: Set = setOf() @@ -64,15 +65,20 @@ internal class NetworkStateProvider( private val isRegistered: AtomicBoolean = AtomicBoolean(false) private fun notifyListenersIfNetworkStateChanged() { - val isNowConnected = isConnected() - if (!isConnected && isNowConnected) { - logger.i { "Network connected." } - isConnected = true - listeners.onConnected() - } else if (isConnected && !isNowConnected) { - logger.i { "Network disconnected." } - isConnected = false - listeners.onDisconnected() + setConnected(queryConnectivity()) + } + + private fun setConnected(isNowConnected: Boolean) { + synchronized(lock) { + if (lastKnownConnected == isNowConnected) return + lastKnownConnected = isNowConnected + if (isNowConnected) { + logger.i { "Network connected." } + listeners.onConnected() + } else { + logger.i { "Network disconnected." } + listeners.onDisconnected() + } } } @@ -88,14 +94,23 @@ internal class NetworkStateProvider( } } - fun isConnected(): Boolean { + /** + * Reports whether the network is currently usable. + * + * The answer is recorded, so that a caller which concludes "no network" from this cannot leave + * the last known state stale. If it did, the following reconnection would not be seen as a + * transition and listeners would never be notified that the network came back. + */ + fun isConnected(): Boolean = queryConnectivity().also { + synchronized(lock) { lastKnownConnected = it } + } + + private fun queryConnectivity(): Boolean { return if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.M) { runCatching { connectivityManager.run { - getNetworkCapabilities(activeNetwork)?.run { - hasCapability(NetworkCapabilities.NET_CAPABILITY_INTERNET) && - hasCapability(NetworkCapabilities.NET_CAPABILITY_VALIDATED) - } + getNetworkCapabilities(activeNetwork) + ?.hasCapability(NetworkCapabilities.NET_CAPABILITY_INTERNET) } }.getOrNull() ?: false } else { diff --git a/stream-chat-android-client/src/test/java/io/getstream/chat/android/client/network/NetworkStateProviderTest.kt b/stream-chat-android-client/src/test/java/io/getstream/chat/android/client/network/NetworkStateProviderTest.kt new file mode 100644 index 00000000000..f07d8909651 --- /dev/null +++ b/stream-chat-android-client/src/test/java/io/getstream/chat/android/client/network/NetworkStateProviderTest.kt @@ -0,0 +1,130 @@ +/* + * Copyright (c) 2014-2026 Stream.io Inc. All rights reserved. + * + * Licensed under the Stream License; + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://github.com/GetStream/stream-chat-android/blob/main/LICENSE + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package io.getstream.chat.android.client.network + +import android.net.ConnectivityManager +import android.net.Network +import android.net.NetworkCapabilities +import android.net.NetworkRequest +import kotlinx.coroutines.CoroutineScope +import kotlinx.coroutines.ExperimentalCoroutinesApi +import kotlinx.coroutines.test.UnconfinedTestDispatcher +import kotlinx.coroutines.test.advanceUntilIdle +import kotlinx.coroutines.test.runTest +import org.amshove.kluent.`should be equal to` +import org.junit.Test +import org.junit.runner.RunWith +import org.mockito.kotlin.any +import org.mockito.kotlin.anyOrNull +import org.mockito.kotlin.argumentCaptor +import org.mockito.kotlin.doReturn +import org.mockito.kotlin.mock +import org.mockito.kotlin.never +import org.mockito.kotlin.verify +import org.mockito.kotlin.verifyBlocking +import org.mockito.kotlin.whenever +import org.robolectric.RobolectricTestRunner +import org.robolectric.annotation.Config + +@OptIn(ExperimentalCoroutinesApi::class) +@RunWith(RobolectricTestRunner::class) +@Config(sdk = [33]) +internal class NetworkStateProviderTest { + + private val connectivityManager: ConnectivityManager = mock() + private val capabilities: NetworkCapabilities = mock() + + private fun givenNetworkUsable(usable: Boolean) { + whenever(capabilities.hasCapability(NetworkCapabilities.NET_CAPABILITY_INTERNET)) doReturn usable + } + + private fun givenConnectivityManager() { + whenever(connectivityManager.activeNetwork) doReturn mock() + whenever(connectivityManager.getNetworkCapabilities(anyOrNull())) doReturn capabilities + } + + private fun captureCallback(): ConnectivityManager.NetworkCallback { + val captor = argumentCaptor() + verify(connectivityManager).registerNetworkCallback(any(), captor.capture()) + return captor.firstValue + } + + /** + * Regression test. A caller that reads [NetworkStateProvider.isConnected] while the network is + * momentarily unusable must not suppress the later "network is back" notification. Before the + * fix the cached state stayed `true`, so the return of the network was not seen as a + * transition and the socket was never told to reconnect. + */ + @Test + fun `when the network is read as unusable directly, its return is still reported`() = runTest { + givenConnectivityManager() + givenNetworkUsable(true) + val provider = NetworkStateProvider( + CoroutineScope(UnconfinedTestDispatcher(testScheduler)), + connectivityManager, + ) + val listener: NetworkStateProvider.NetworkStateListener = mock() + provider.subscribe(listener) + val callback = captureCallback() + + givenNetworkUsable(false) + provider.isConnected() `should be equal to` false + + givenNetworkUsable(true) + callback.onCapabilitiesChanged(mock(), capabilities) + advanceUntilIdle() + + verifyBlocking(listener) { onConnected() } + } + + @Test + fun `when the network stays usable, listeners are not notified again`() = runTest { + givenConnectivityManager() + givenNetworkUsable(true) + val provider = NetworkStateProvider( + CoroutineScope(UnconfinedTestDispatcher(testScheduler)), + connectivityManager, + ) + val listener: NetworkStateProvider.NetworkStateListener = mock() + provider.subscribe(listener) + val callback = captureCallback() + + callback.onCapabilitiesChanged(mock(), capabilities) + advanceUntilIdle() + + verifyBlocking(listener, never()) { onConnected() } + } + + @Test + fun `when the last network is lost, listeners are notified of the disconnection`() = runTest { + givenConnectivityManager() + givenNetworkUsable(true) + val provider = NetworkStateProvider( + CoroutineScope(UnconfinedTestDispatcher(testScheduler)), + connectivityManager, + ) + val listener: NetworkStateProvider.NetworkStateListener = mock() + provider.subscribe(listener) + val callback = captureCallback() + + givenNetworkUsable(false) + callback.onLost(mock()) + advanceUntilIdle() + + verifyBlocking(listener) { onDisconnected() } + } +} From 319dadde2ef3a941714590e073c68771a405de7d Mon Sep 17 00:00:00 2001 From: Aleksandar Apostolov Date: Tue, 25 Aug 2026 18:38:45 +0200 Subject: [PATCH 2/4] fix(client): allow the socket to leave Disconnected.Network The state stopped the health monitor without scheduling a replacement and declared no transition for Event.Resume, so it could only be left if a network callback arrived. One missed callback stranded the socket until the process was killed. Keep a backed-off retry armed and restart the connection on resume, matching Disconnected.Stopped. The existing state machine test pinned the old behaviour, asserting that a resume left NetworkDisconnected unchanged. --- .../io/getstream/chat/android/client/socket/ChatSocket.kt | 4 +++- .../chat/android/client/socket/ChatSocketStateService.kt | 1 + .../client/socket/experimental/ChatSocketStateServiceTest.kt | 2 +- 3 files changed, 5 insertions(+), 2 deletions(-) diff --git a/stream-chat-android-client/src/main/java/io/getstream/chat/android/client/socket/ChatSocket.kt b/stream-chat-android-client/src/main/java/io/getstream/chat/android/client/socket/ChatSocket.kt index 2cf9c4b7603..20e70c2e185 100644 --- a/stream-chat-android-client/src/main/java/io/getstream/chat/android/client/socket/ChatSocket.kt +++ b/stream-chat-android-client/src/main/java/io/getstream/chat/android/client/socket/ChatSocket.kt @@ -154,7 +154,9 @@ internal open class ChatSocket( } is State.Disconnected.NetworkDisconnected -> { streamWebSocket?.close() - healthMonitor.stop() + // Keep a backed-off retry armed. Recovery must not depend solely on + // a network callback arriving, or a missed one strands the socket. + healthMonitor.onDisconnected() } is State.Disconnected.Stopped -> { streamWebSocket?.close() diff --git a/stream-chat-android-client/src/main/java/io/getstream/chat/android/client/socket/ChatSocketStateService.kt b/stream-chat-android-client/src/main/java/io/getstream/chat/android/client/socket/ChatSocketStateService.kt index 77dad6ceb23..9e0dc87f8db 100644 --- a/stream-chat-android-client/src/main/java/io/getstream/chat/android/client/socket/ChatSocketStateService.kt +++ b/stream-chat-android-client/src/main/java/io/getstream/chat/android/client/socket/ChatSocketStateService.kt @@ -208,6 +208,7 @@ internal class ChatSocketStateService(initialState: State = State.Disconnected.S onEvent { State.Disconnected.DisconnectedByRequest } onEvent { State.Disconnected.Stopped } onEvent { State.RestartConnection(RestartReason.NETWORK_AVAILABLE) } + onEvent { State.RestartConnection(RestartReason.LIFECYCLE_RESUME) } } state { diff --git a/stream-chat-android-client/src/test/java/io/getstream/chat/android/client/socket/experimental/ChatSocketStateServiceTest.kt b/stream-chat-android-client/src/test/java/io/getstream/chat/android/client/socket/experimental/ChatSocketStateServiceTest.kt index cd6880efa5b..523336dffab 100644 --- a/stream-chat-android-client/src/test/java/io/getstream/chat/android/client/socket/experimental/ChatSocketStateServiceTest.kt +++ b/stream-chat-android-client/src/test/java/io/getstream/chat/android/client/socket/experimental/ChatSocketStateServiceTest.kt @@ -413,7 +413,7 @@ internal class ChatSocketStateServiceTest { ), Arguments.of( State.Disconnected.NetworkDisconnected, - State.Disconnected.NetworkDisconnected, + State.RestartConnection(RestartReason.LIFECYCLE_RESUME), ), Arguments.of( State.Disconnected.WebSocketEventLost, From 1c68518918f03954ff3f1dd597617aa91ef63a3c Mon Sep 17 00:00:00 2001 From: Aleksandar Apostolov Date: Tue, 25 Aug 2026 18:38:45 +0200 Subject: [PATCH 3/4] fix(client): deliver the first real lifecycle resume observe() suppressed the first ON_RESUME in order to swallow the replay that addObserver dispatches when the owner is already resumed. Subscribing from a process that is not resumed gets no replay, so the suppression discarded a genuine foregrounding instead. Only suppress when the owner is already resumed at subscribe time. --- .../android/client/StreamLifecycleObserver.kt | 5 +- .../client/StreamLifecycleObserverTest.kt | 77 +++++++++++++++++++ 2 files changed, 81 insertions(+), 1 deletion(-) create mode 100644 stream-chat-android-client/src/test/java/io/getstream/chat/android/client/StreamLifecycleObserverTest.kt diff --git a/stream-chat-android-client/src/main/java/io/getstream/chat/android/client/StreamLifecycleObserver.kt b/stream-chat-android-client/src/main/java/io/getstream/chat/android/client/StreamLifecycleObserver.kt index 9bef151bd36..f9163972c42 100644 --- a/stream-chat-android-client/src/main/java/io/getstream/chat/android/client/StreamLifecycleObserver.kt +++ b/stream-chat-android-client/src/main/java/io/getstream/chat/android/client/StreamLifecycleObserver.kt @@ -42,7 +42,10 @@ internal class StreamLifecycleObserver( withContext(DispatcherProvider.Main) { handlers = handlers + handler if (isObserving.compareAndSet(false, true)) { - recurringResumeEvent = false + // addObserver replays ON_RESUME when the owner is already resumed, and that replay + // is the only event worth ignoring. Subscribing from a non-resumed process gets no + // replay, so the next ON_RESUME is a real foregrounding and must be delivered. + recurringResumeEvent = !lifecycle.currentState.isAtLeast(Lifecycle.State.RESUMED) lifecycle.addObserver(this@StreamLifecycleObserver) logger.v { "[observe] subscribed" } } diff --git a/stream-chat-android-client/src/test/java/io/getstream/chat/android/client/StreamLifecycleObserverTest.kt b/stream-chat-android-client/src/test/java/io/getstream/chat/android/client/StreamLifecycleObserverTest.kt new file mode 100644 index 00000000000..3deddf23870 --- /dev/null +++ b/stream-chat-android-client/src/test/java/io/getstream/chat/android/client/StreamLifecycleObserverTest.kt @@ -0,0 +1,77 @@ +/* + * Copyright (c) 2014-2026 Stream.io Inc. All rights reserved. + * + * Licensed under the Stream License; + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://github.com/GetStream/stream-chat-android/blob/main/LICENSE + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package io.getstream.chat.android.client + +import androidx.lifecycle.Lifecycle +import androidx.lifecycle.testing.TestLifecycleOwner +import io.getstream.chat.android.test.TestCoroutineExtension +import kotlinx.coroutines.test.runTest +import org.junit.jupiter.api.Test +import org.junit.jupiter.api.extension.RegisterExtension +import org.mockito.kotlin.mock +import org.mockito.kotlin.never +import org.mockito.kotlin.verifyBlocking + +internal class StreamLifecycleObserverTest { + + private companion object { + @JvmField + @RegisterExtension + val testCoroutines = TestCoroutineExtension() + } + + /** + * Regression test. Subscribing from a process that is not resumed gets no replayed ON_RESUME, + * so the next one is a real foregrounding and has to reach the handlers. Before the fix it was + * swallowed together with the replay, and a socket waiting on a resume was never woken. + */ + @Test + fun `when subscribed while not resumed, the following resume is delivered`() = runTest(testCoroutines.dispatcher) { + val owner = TestLifecycleOwner(Lifecycle.State.CREATED, testCoroutines.dispatcher) + val observer = StreamLifecycleObserver(testCoroutines.scope, owner.lifecycle) + val handler: LifecycleHandler = mock() + + observer.observe(handler) + owner.setCurrentState(Lifecycle.State.RESUMED) + + verifyBlocking(handler) { resume() } + } + + @Test + fun `when subscribed while already resumed, the replayed resume is ignored`() = runTest(testCoroutines.dispatcher) { + val owner = TestLifecycleOwner(Lifecycle.State.RESUMED, testCoroutines.dispatcher) + val observer = StreamLifecycleObserver(testCoroutines.scope, owner.lifecycle) + val handler: LifecycleHandler = mock() + + observer.observe(handler) + + verifyBlocking(handler, never()) { resume() } + } + + @Test + fun `when resumed again after the first delivery, the resume is still delivered`() = runTest(testCoroutines.dispatcher) { + val owner = TestLifecycleOwner(Lifecycle.State.RESUMED, testCoroutines.dispatcher) + val observer = StreamLifecycleObserver(testCoroutines.scope, owner.lifecycle) + val handler: LifecycleHandler = mock() + observer.observe(handler) + + owner.setCurrentState(Lifecycle.State.CREATED) + owner.setCurrentState(Lifecycle.State.RESUMED) + + verifyBlocking(handler) { resume() } + } +} From d26aa003f6d17593a54c3646ba7748fcbd0f5d44 Mon Sep 17 00:00:00 2001 From: Aleksandar Apostolov Date: Wed, 26 Aug 2026 09:09:20 +0200 Subject: [PATCH 4/4] fix(client): narrow the recorded connectivity read to negatives Recording a positive read consumed the transition the listeners wait for, when isConnected() ran after the network returned but before the system callback landed. That is the same defect this PR fixes, mirrored, and it is reachable from app code through ClientState.isNetworkAvailable. Only a negative answer is recorded now, which is the direction that can strand the socket. Restore healthMonitor.stop() in NetworkDisconnected. The replacement retry emitted Event.WebSocketEventLost, which that state does not handle, so it was inert; worse, a pending one could knock a later Connecting state into Disconnected.WebSocketEventLost. Restore the NET_CAPABILITY_VALIDATED check. With the provider fixed a false negative is no longer terminal, so the behaviour change is not worth its risk in this PR. --- .../client/network/NetworkStateProvider.kt | 22 ++++++++----- .../chat/android/client/socket/ChatSocket.kt | 4 +-- .../network/NetworkStateProviderTest.kt | 31 +++++++++++++++++++ 3 files changed, 47 insertions(+), 10 deletions(-) diff --git a/stream-chat-android-client/src/main/java/io/getstream/chat/android/client/network/NetworkStateProvider.kt b/stream-chat-android-client/src/main/java/io/getstream/chat/android/client/network/NetworkStateProvider.kt index cf0347e540c..131779ab1bd 100644 --- a/stream-chat-android-client/src/main/java/io/getstream/chat/android/client/network/NetworkStateProvider.kt +++ b/stream-chat-android-client/src/main/java/io/getstream/chat/android/client/network/NetworkStateProvider.kt @@ -97,20 +97,28 @@ internal class NetworkStateProvider( /** * Reports whether the network is currently usable. * - * The answer is recorded, so that a caller which concludes "no network" from this cannot leave - * the last known state stale. If it did, the following reconnection would not be seen as a - * transition and listeners would never be notified that the network came back. + * Only a `false` answer is recorded. A caller that concludes "no network" from this must not + * leave the last known state stale, or the following reconnection is not seen as a transition + * and listeners are never told the network came back. + * + * A `true` answer is deliberately not recorded: doing so would consume the very transition the + * listeners are waiting for, whenever this is read after the network returns but before the + * system callback lands. */ - fun isConnected(): Boolean = queryConnectivity().also { - synchronized(lock) { lastKnownConnected = it } + fun isConnected(): Boolean = queryConnectivity().also { connected -> + if (!connected) { + synchronized(lock) { lastKnownConnected = false } + } } private fun queryConnectivity(): Boolean { return if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.M) { runCatching { connectivityManager.run { - getNetworkCapabilities(activeNetwork) - ?.hasCapability(NetworkCapabilities.NET_CAPABILITY_INTERNET) + getNetworkCapabilities(activeNetwork)?.run { + hasCapability(NetworkCapabilities.NET_CAPABILITY_INTERNET) && + hasCapability(NetworkCapabilities.NET_CAPABILITY_VALIDATED) + } } }.getOrNull() ?: false } else { diff --git a/stream-chat-android-client/src/main/java/io/getstream/chat/android/client/socket/ChatSocket.kt b/stream-chat-android-client/src/main/java/io/getstream/chat/android/client/socket/ChatSocket.kt index 20e70c2e185..2cf9c4b7603 100644 --- a/stream-chat-android-client/src/main/java/io/getstream/chat/android/client/socket/ChatSocket.kt +++ b/stream-chat-android-client/src/main/java/io/getstream/chat/android/client/socket/ChatSocket.kt @@ -154,9 +154,7 @@ internal open class ChatSocket( } is State.Disconnected.NetworkDisconnected -> { streamWebSocket?.close() - // Keep a backed-off retry armed. Recovery must not depend solely on - // a network callback arriving, or a missed one strands the socket. - healthMonitor.onDisconnected() + healthMonitor.stop() } is State.Disconnected.Stopped -> { streamWebSocket?.close() diff --git a/stream-chat-android-client/src/test/java/io/getstream/chat/android/client/network/NetworkStateProviderTest.kt b/stream-chat-android-client/src/test/java/io/getstream/chat/android/client/network/NetworkStateProviderTest.kt index f07d8909651..5bc79b55ab8 100644 --- a/stream-chat-android-client/src/test/java/io/getstream/chat/android/client/network/NetworkStateProviderTest.kt +++ b/stream-chat-android-client/src/test/java/io/getstream/chat/android/client/network/NetworkStateProviderTest.kt @@ -50,6 +50,7 @@ internal class NetworkStateProviderTest { private fun givenNetworkUsable(usable: Boolean) { whenever(capabilities.hasCapability(NetworkCapabilities.NET_CAPABILITY_INTERNET)) doReturn usable + whenever(capabilities.hasCapability(NetworkCapabilities.NET_CAPABILITY_VALIDATED)) doReturn usable } private fun givenConnectivityManager() { @@ -91,6 +92,36 @@ internal class NetworkStateProviderTest { verifyBlocking(listener) { onConnected() } } + /** + * Regression test. Reading [NetworkStateProvider.isConnected] after the network has returned + * but before the system callback lands must not consume the transition. Recording a `true` + * answer here would leave the callback with nothing to report, stranding the socket exactly as + * the stale `true` did. + */ + @Test + fun `when the network is read as usable before the callback lands, the callback still reports it`() = runTest { + givenConnectivityManager() + givenNetworkUsable(true) + val provider = NetworkStateProvider( + CoroutineScope(UnconfinedTestDispatcher(testScheduler)), + connectivityManager, + ) + val listener: NetworkStateProvider.NetworkStateListener = mock() + provider.subscribe(listener) + val callback = captureCallback() + + givenNetworkUsable(false) + provider.isConnected() `should be equal to` false + + givenNetworkUsable(true) + provider.isConnected() `should be equal to` true + + callback.onCapabilitiesChanged(mock(), capabilities) + advanceUntilIdle() + + verifyBlocking(listener) { onConnected() } + } + @Test fun `when the network stays usable, listeners are not notified again`() = runTest { givenConnectivityManager()