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..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 @@ -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,16 @@ 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( + 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()) + .blockingSingle() + } conversationsFromSync = conversationsList.map { it.asEntity(user.id!!) @@ -206,6 +217,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 +355,8 @@ 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 + 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/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) /** 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) + } }