From ee756814eca019061f3e3e33eeb66596d635905d Mon Sep 17 00:00:00 2001 From: rapterjet2004 Date: Tue, 1 Sep 2026 10:08:27 -0500 Subject: [PATCH 1/2] fix(conversationlist): Retry conversation sync and surface fetch errors on slow networks On a slow/unstable connection (e.g. GPRS), the room-list fetch can fail with a transient error like StreamResetException, which was silently swallowed. On first launch with an empty local cache this left the conversation list blank with no explanation and no way to recover short of restarting the app. Retry the fetch a few times to ride out transient failures, and only when retries are exhausted and there is nothing cached locally to show, surface the error through the existing error-dialog UI so the user knows the sync failed. Signed-off-by: rapterjet2004 --- .../data/OfflineConversationsRepository.kt | 9 +++++++++ .../OfflineFirstConversationsRepository.kt | 20 +++++++++++++++---- .../viewmodels/ConversationsListViewModel.kt | 7 +++++++ 3 files changed, 32 insertions(+), 4 deletions(-) diff --git a/app/src/main/java/com/nextcloud/talk/conversationlist/data/OfflineConversationsRepository.kt b/app/src/main/java/com/nextcloud/talk/conversationlist/data/OfflineConversationsRepository.kt index 8468cbe90e..72d9efe613 100644 --- a/app/src/main/java/com/nextcloud/talk/conversationlist/data/OfflineConversationsRepository.kt +++ b/app/src/main/java/com/nextcloud/talk/conversationlist/data/OfflineConversationsRepository.kt @@ -22,6 +22,15 @@ interface OfflineConversationsRepository { */ val roomListFlow: Flow> + /** + * Emits when [getRooms] fails to sync with the server (e.g. a dropped/reset connection on a + * slow network) while there are no locally cached conversations to fall back on for that + * account, so the UI can tell the user why the list is empty instead of failing silently. + * A failed sync while conversations are already cached does not emit here, since + * [roomListFlow] already has data to show and the sync is a best-effort background refresh. + */ + val syncErrorFlow: Flow + /** * Stream of a single conversation, for use in each conversations settings. */ diff --git a/app/src/main/java/com/nextcloud/talk/conversationlist/data/network/OfflineFirstConversationsRepository.kt b/app/src/main/java/com/nextcloud/talk/conversationlist/data/network/OfflineFirstConversationsRepository.kt index 25c908f380..6cc07d85f6 100644 --- a/app/src/main/java/com/nextcloud/talk/conversationlist/data/network/OfflineFirstConversationsRepository.kt +++ b/app/src/main/java/com/nextcloud/talk/conversationlist/data/network/OfflineFirstConversationsRepository.kt @@ -26,6 +26,7 @@ import com.nextcloud.talk.models.domain.ConversationModel import com.nextcloud.talk.utils.ApiUtils import com.nextcloud.talk.utils.CapabilitiesUtil.isUserStatusAvailable import com.nextcloud.talk.utils.SpreedFeatures +import com.nextcloud.talk.utils.withRetry import io.reactivex.Observer import io.reactivex.android.schedulers.AndroidSchedulers import io.reactivex.disposables.Disposable @@ -83,6 +84,10 @@ class OfflineFirstConversationsRepository @Inject constructor( get() = _conversationFlow private val _conversationFlow: MutableSharedFlow = MutableSharedFlow() + override val syncErrorFlow: Flow + get() = _syncErrorFlow + private val _syncErrorFlow: MutableSharedFlow = MutableSharedFlow() + private val scope = CoroutineScope(Dispatchers.IO) sealed interface ConversationResult { @@ -181,10 +186,12 @@ class OfflineFirstConversationsRepository @Inject constructor( val includeStatus = isUserStatusAvailable(user) try { - val conversationsList = network.getRooms(user, user.baseUrl!!, includeStatus) - .subscribeOn(Schedulers.io()) - .observeOn(AndroidSchedulers.mainThread()) - .blockingSingle() + val conversationsList = withRetry(NETWORK_FETCH_RETRIES) { + network.getRooms(user, user.baseUrl!!, includeStatus) + .subscribeOn(Schedulers.io()) + .observeOn(AndroidSchedulers.mainThread()) + .blockingSingle() + } conversationsFromSync = conversationsList.map { it.asEntity(user.id!!) @@ -206,6 +213,10 @@ class OfflineFirstConversationsRepository @Inject constructor( scope.launch { catchUpRoomsWithNewMessages(user, roomsWithNewMessages) } } catch (e: Exception) { Log.e(TAG, "Something went wrong when fetching conversations", e) + val hasCachedConversations = dao.getConversationsForUser(user.id!!).first().isNotEmpty() + if (!hasCachedConversations) { + _syncErrorFlow.emit(e) + } } return conversationsFromSync } @@ -340,5 +351,6 @@ class OfflineFirstConversationsRepository @Inject constructor( private const val CHAT_API_VERSION = 1 private const val MAX_ROOMS_TO_CATCH_UP = 20 private const val MAX_CONCURRENT_CATCH_UPS = 3 + private const val NETWORK_FETCH_RETRIES = 3 } } diff --git a/app/src/main/java/com/nextcloud/talk/conversationlist/viewmodels/ConversationsListViewModel.kt b/app/src/main/java/com/nextcloud/talk/conversationlist/viewmodels/ConversationsListViewModel.kt index e90f45cc50..31547455b0 100644 --- a/app/src/main/java/com/nextcloud/talk/conversationlist/viewmodels/ConversationsListViewModel.kt +++ b/app/src/main/java/com/nextcloud/talk/conversationlist/viewmodels/ConversationsListViewModel.kt @@ -58,6 +58,7 @@ import kotlinx.coroutines.flow.catch import kotlinx.coroutines.flow.combine import kotlinx.coroutines.flow.flow import kotlinx.coroutines.flow.flowOn +import kotlinx.coroutines.flow.launchIn import kotlinx.coroutines.flow.map import kotlinx.coroutines.flow.onEach import kotlinx.coroutines.flow.stateIn @@ -150,6 +151,12 @@ class ConversationsListViewModel @Inject constructor( _getRoomsViewState.value = GetRoomsErrorState(it) } + init { + repository.syncErrorFlow + .onEach { throwable -> _getRoomsViewState.value = GetRoomsErrorState(throwable) } + .launchIn(viewModelScope) + } + private val _isLoadingRooms = MutableStateFlow(true) /** From 2a5736a83247a3629bf67e8d99750f6b515ca36f Mon Sep 17 00:00:00 2001 From: Marcel Hibbe Date: Wed, 2 Sep 2026 19:27:28 +0200 Subject: [PATCH 2/2] fix(conversationlist): Add exponential backoff to conversation fetch retries fix(conversationlist): Add exponential backoff to conversation fetch retries Retrying immediately on a slow/unstable connection tends to hit the same transient failure again right away. Wait between retry attempts, backing off exponentially (1s, 2s, 4s, capped at 8s) instead of retrying instantly. Assisted-by: Claude Code:claude-sonnet-5 Signed-off-by: Marcel Hibbe --- .../OfflineFirstConversationsRepository.kt | 8 ++- .../nextcloud/talk/utils/CoroutineUtils.kt | 27 ++++++-- .../talk/utils/CoroutineUtilsTest.kt | 61 +++++++++++++++++++ 3 files changed, 91 insertions(+), 5 deletions(-) diff --git a/app/src/main/java/com/nextcloud/talk/conversationlist/data/network/OfflineFirstConversationsRepository.kt b/app/src/main/java/com/nextcloud/talk/conversationlist/data/network/OfflineFirstConversationsRepository.kt index 6cc07d85f6..1c2bf927cf 100644 --- a/app/src/main/java/com/nextcloud/talk/conversationlist/data/network/OfflineFirstConversationsRepository.kt +++ b/app/src/main/java/com/nextcloud/talk/conversationlist/data/network/OfflineFirstConversationsRepository.kt @@ -186,7 +186,11 @@ class OfflineFirstConversationsRepository @Inject constructor( val includeStatus = isUserStatusAvailable(user) try { - val conversationsList = withRetry(NETWORK_FETCH_RETRIES) { + val conversationsList = withRetry( + retries = NETWORK_FETCH_RETRIES, + initialDelayMillis = NETWORK_FETCH_RETRY_INITIAL_DELAY_MS, + maxDelayMillis = NETWORK_FETCH_RETRY_MAX_DELAY_MS + ) { network.getRooms(user, user.baseUrl!!, includeStatus) .subscribeOn(Schedulers.io()) .observeOn(AndroidSchedulers.mainThread()) @@ -352,5 +356,7 @@ class OfflineFirstConversationsRepository @Inject constructor( private const val MAX_ROOMS_TO_CATCH_UP = 20 private const val MAX_CONCURRENT_CATCH_UPS = 3 private const val NETWORK_FETCH_RETRIES = 3 + private const val NETWORK_FETCH_RETRY_INITIAL_DELAY_MS = 1000L + private const val NETWORK_FETCH_RETRY_MAX_DELAY_MS = 8000L } } diff --git a/app/src/main/java/com/nextcloud/talk/utils/CoroutineUtils.kt b/app/src/main/java/com/nextcloud/talk/utils/CoroutineUtils.kt index c46382a487..1c8c134fcc 100644 --- a/app/src/main/java/com/nextcloud/talk/utils/CoroutineUtils.kt +++ b/app/src/main/java/com/nextcloud/talk/utils/CoroutineUtils.kt @@ -7,19 +7,38 @@ package com.nextcloud.talk.utils +import kotlinx.coroutines.delay +import kotlin.math.min +import kotlin.math.pow + /** - * Executes [block] and, if it throws, retries up to [retries] additional times. - * Equivalent to RxJava's `.retry(retries)`. + * Executes [block] and, if it throws, retries up to [retries] additional times, waiting + * between attempts starting at [initialDelayMillis] and multiplying by [backoffFactor] after + * each failed attempt, capped at [maxDelayMillis]. + * Equivalent to RxJava's `.retry(retries)`, with exponential backoff. * The last exception is rethrown if all attempts fail. */ -@Suppress("TooGenericExceptionCaught") -suspend fun withRetry(retries: Int = 1, block: suspend () -> T): T { +@Suppress("TooGenericExceptionCaught", "LongParameterList") +suspend fun withRetry( + retries: Int = 1, + initialDelayMillis: Long = 0, + backoffFactor: Double = 2.0, + maxDelayMillis: Long = Long.MAX_VALUE, + block: suspend () -> T +): T { var attempt = 0 while (true) { try { return block() } catch (e: Exception) { if (attempt >= retries) throw e + if (initialDelayMillis > 0) { + val delayMillis = min( + initialDelayMillis * backoffFactor.pow(attempt), + maxDelayMillis.toDouble() + ).toLong() + delay(delayMillis) + } attempt++ } } diff --git a/app/src/test/java/com/nextcloud/talk/utils/CoroutineUtilsTest.kt b/app/src/test/java/com/nextcloud/talk/utils/CoroutineUtilsTest.kt index 983d4322a0..1a0db4e72a 100644 --- a/app/src/test/java/com/nextcloud/talk/utils/CoroutineUtilsTest.kt +++ b/app/src/test/java/com/nextcloud/talk/utils/CoroutineUtilsTest.kt @@ -7,9 +7,11 @@ package com.nextcloud.talk.utils +import kotlinx.coroutines.test.currentTime import kotlinx.coroutines.test.runTest import org.junit.Assert.assertEquals import org.junit.Assert.assertSame +import org.junit.Assert.assertTrue import org.junit.Test @Suppress("TooGenericExceptionThrown") @@ -76,4 +78,63 @@ class CoroutineUtilsTest { } assertEquals(1, callCount) } + + @Test + fun `withRetry waits initialDelayMillis before the first retry`() = + runTest { + var callCount = 0 + withRetry(retries = 1, initialDelayMillis = 1000) { + callCount++ + if (callCount < 2) throw RuntimeException("transient error") + "success" + } + assertEquals(2, callCount) + assertEquals(1000, currentTime) + } + + @Test + fun `withRetry does not wait when initialDelayMillis is zero`() = + runTest { + var callCount = 0 + withRetry(retries = 2, initialDelayMillis = 0) { + callCount++ + if (callCount < 3) throw RuntimeException("transient error") + "success" + } + assertEquals(3, callCount) + assertTrue(currentTime == 0L) + } + + @Test + fun `withRetry backs off exponentially between attempts`() = + runTest { + var callCount = 0 + withRetry(retries = 2, initialDelayMillis = 1000, backoffFactor = 2.0) { + callCount++ + if (callCount < 3) throw RuntimeException("transient error") + "success" + } + assertEquals(3, callCount) + // 1000ms before the 2nd attempt, then 2000ms before the 3rd + assertEquals(3000, currentTime) + } + + @Test + fun `withRetry caps the backoff delay at maxDelayMillis`() = + runTest { + var callCount = 0 + withRetry( + retries = 3, + initialDelayMillis = 1000, + backoffFactor = 2.0, + maxDelayMillis = 1500 + ) { + callCount++ + if (callCount < 4) throw RuntimeException("transient error") + "success" + } + assertEquals(4, callCount) + // Uncapped would be 1000 + 2000 + 4000; capped at 1500 each time it exceeds it + assertEquals(1000 + 1500 + 1500, currentTime) + } }