From 266030f8efc5be165260914e776609d1408e5fcd Mon Sep 17 00:00:00 2001 From: Krainov Gleb Date: Fri, 9 Oct 2026 19:52:55 +0000 Subject: [PATCH 1/2] fix(chat): do not retry a long poll while the chat is paused pullMessagesFlow retries a failed chat request up to four times and never looked at the pause state, so a long poll killed by the system in the background was restarted with a new timeout=30 request. initLongPolling checks itIsPaused only before it starts a cycle. pullMessagesFlow now takes a canRetry predicate. The long poll passes !itIsPaused, a failed request on a paused chat ends with an error and the loop waits for the resume. Other callers keep the default. Signed-off-by: Krainov Gleb Assisted-by: Claude-Code:claude-sonnet-5-5 --- .../chat/data/network/ChatMessageSyncer.kt | 19 +++++++++++++++---- .../network/OfflineFirstChatRepository.kt | 2 +- .../data/network/ChatMessageSyncerTest.kt | 14 ++++++++++++++ 3 files changed, 30 insertions(+), 5 deletions(-) diff --git a/app/src/main/java/com/nextcloud/talk/chat/data/network/ChatMessageSyncer.kt b/app/src/main/java/com/nextcloud/talk/chat/data/network/ChatMessageSyncer.kt index 5a9b74eeb4f..36a9bda731b 100644 --- a/app/src/main/java/com/nextcloud/talk/chat/data/network/ChatMessageSyncer.kt +++ b/app/src/main/java/com/nextcloud/talk/chat/data/network/ChatMessageSyncer.kt @@ -714,9 +714,15 @@ class ChatMessageSyncer @Inject constructor( * [MAX_PULL_ATTEMPTS] times with a linearly growing delay in between. The requested limit is * kept across retries: a background catch-up that succeeds on a later attempt must still * create a full chat block, and backing off in time rather than in payload size also keeps a - * struggling or rate-limiting server from being hammered. + * struggling or rate-limiting server from being hammered. A failed request is not retried + * once [canRetry] returns false, e.g. a long poll must not restart while the chat is in the + * background. */ - fun pullMessagesFlow(target: SyncTarget, fieldMap: HashMap): Flow = + fun pullMessagesFlow( + target: SyncTarget, + fieldMap: HashMap, + canRetry: () -> Boolean = { true } + ): Flow = flow { var attempts = 1 @@ -741,6 +747,10 @@ class ChatMessageSyncer @Inject constructor( onFailure = { e -> Log.e(TAG, "Attempt $attempts failed", e) attempts++ + if (!canRetry()) { + emit(ChatPullResult.Error(IllegalStateException("Retry cancelled"))) + return@flow + } if (attempts < MAX_PULL_ATTEMPTS) { delay(PULL_RETRY_DELAY_MILLIS * (attempts - 1)) } @@ -758,7 +768,8 @@ class ChatMessageSyncer @Inject constructor( suspend fun pullAndPersistMessages( target: SyncTarget, fieldMap: HashMap, - events: Events = NO_EVENTS + events: Events = NO_EVENTS, + canRetry: () -> Boolean = { true } ): SyncOutcome { val isLongPoll = (fieldMap["timeout"] ?: 0) > 0 if (!isLongPoll) events.onLoadingChanged(true) @@ -770,7 +781,7 @@ class ChatMessageSyncer @Inject constructor( val queriedMessageId = fieldMap["lastKnownMessageId"] val lookIntoFuture = fieldMap["lookIntoFuture"] == 1 - return when (val result = pullMessagesFlow(target, fieldMap).first()) { + return when (val result = pullMessagesFlow(target, fieldMap, canRetry).first()) { is ChatPullResult.Success -> handleSuccessfulPull(target, result, queriedMessageId, lookIntoFuture, events) diff --git a/app/src/main/java/com/nextcloud/talk/chat/data/network/OfflineFirstChatRepository.kt b/app/src/main/java/com/nextcloud/talk/chat/data/network/OfflineFirstChatRepository.kt index 180f3cb17eb..1d2eb4902b4 100644 --- a/app/src/main/java/com/nextcloud/talk/chat/data/network/OfflineFirstChatRepository.kt +++ b/app/src/main/java/com/nextcloud/talk/chat/data/network/OfflineFirstChatRepository.kt @@ -318,7 +318,7 @@ class OfflineFirstChatRepository @Inject constructor( // sync database with server // (This is a long blocking call because long polling (lookIntoFuture and timeout) is set) Log.d(TAG, "Starting online request for long polling") - val outcome = syncer.pullAndPersistMessages(syncTarget, fieldMap, syncEvents) + val outcome = syncer.pullAndPersistMessages(syncTarget, fieldMap, syncEvents) { !itIsPaused } // A failed request returns right away, e.g. with rejected credentials, which the server counts as a // failed login. Without waiting, the next requests would follow at once until the server throttles. diff --git a/app/src/test/java/com/nextcloud/talk/chat/data/network/ChatMessageSyncerTest.kt b/app/src/test/java/com/nextcloud/talk/chat/data/network/ChatMessageSyncerTest.kt index 7df743cdcce..5dab1cb8718 100644 --- a/app/src/test/java/com/nextcloud/talk/chat/data/network/ChatMessageSyncerTest.kt +++ b/app/src/test/java/com/nextcloud/talk/chat/data/network/ChatMessageSyncerTest.kt @@ -9,6 +9,7 @@ package com.nextcloud.talk.chat.data.network import android.database.sqlite.SQLiteConstraintException import com.nextcloud.talk.chat.data.model.ChatMessage +import com.nextcloud.talk.chat.domain.ChatPullResult import com.nextcloud.talk.conversationlist.data.network.ConversationListUpdater import com.nextcloud.talk.data.database.dao.ChatBlocksDao import com.nextcloud.talk.data.database.dao.ChatMessagesDao @@ -25,6 +26,7 @@ import com.nextcloud.talk.utils.CapabilitiesUtil import java.io.IOException import kotlinx.coroutines.CompletableDeferred import kotlinx.coroutines.flow.MutableStateFlow +import kotlinx.coroutines.flow.first import kotlinx.coroutines.flow.flowOf import kotlinx.coroutines.launch import kotlinx.coroutines.test.runTest @@ -235,6 +237,18 @@ class ChatMessageSyncerTest { assertEquals(REQUESTED_LIMIT, fieldMapCaptor.lastValue["limit"]) } + @Test + fun `pullMessagesFlow does not retry a failed request once retrying is not allowed`() = + runTest { + wheneverBlocking { network.pullChatMessages(any(), any(), any()) } + .doSuspendableAnswer { throw IOException("connection reset") } + + val result = syncer.pullMessagesFlow(target(), HashMap(), canRetry = { false }).first() + + assertTrue(result is ChatPullResult.Error) + verifyBlocking(network, times(1)) { pullChatMessages(any(), any(), any()) } + } + @Test fun `catchUpRoom marks the sync as failed when the server request errors`() = runTest { From d95640cd081f75ae0556b590d80f0dfd4cf46d9b Mon Sep 17 00:00:00 2001 From: Krainov Gleb Date: Sun, 11 Oct 2026 13:53:59 +0000 Subject: [PATCH 2/2] fix(chat): re-check canRetry after the retry backoff delay pullMessagesFlow evaluated canRetry right after a failed request and then slept for up to four seconds. A chat paused during that backoff still sent the next request when the delay ended. canRetry is now evaluated again after the delay and before the next request. The check before the delay stays, so a chat that is already paused does not wait for nothing. Signed-off-by: Krainov Gleb Assisted-by: Claude-Code:claude-opus-5-5 --- .../talk/chat/data/network/ChatMessageSyncer.kt | 5 +++++ .../chat/data/network/ChatMessageSyncerTest.kt | 14 ++++++++++++++ 2 files changed, 19 insertions(+) diff --git a/app/src/main/java/com/nextcloud/talk/chat/data/network/ChatMessageSyncer.kt b/app/src/main/java/com/nextcloud/talk/chat/data/network/ChatMessageSyncer.kt index 36a9bda731b..71db2106704 100644 --- a/app/src/main/java/com/nextcloud/talk/chat/data/network/ChatMessageSyncer.kt +++ b/app/src/main/java/com/nextcloud/talk/chat/data/network/ChatMessageSyncer.kt @@ -753,6 +753,11 @@ class ChatMessageSyncer @Inject constructor( } if (attempts < MAX_PULL_ATTEMPTS) { delay(PULL_RETRY_DELAY_MILLIS * (attempts - 1)) + // the chat may have been paused while backing off + if (!canRetry()) { + emit(ChatPullResult.Error(IllegalStateException("Retry cancelled"))) + return@flow + } } } ) diff --git a/app/src/test/java/com/nextcloud/talk/chat/data/network/ChatMessageSyncerTest.kt b/app/src/test/java/com/nextcloud/talk/chat/data/network/ChatMessageSyncerTest.kt index 5dab1cb8718..e2f694d0756 100644 --- a/app/src/test/java/com/nextcloud/talk/chat/data/network/ChatMessageSyncerTest.kt +++ b/app/src/test/java/com/nextcloud/talk/chat/data/network/ChatMessageSyncerTest.kt @@ -249,6 +249,20 @@ class ChatMessageSyncerTest { verifyBlocking(network, times(1)) { pullChatMessages(any(), any(), any()) } } + @Test + fun `pullMessagesFlow does not send the retry when the chat is paused during the backoff`() = + runTest { + wheneverBlocking { network.pullChatMessages(any(), any(), any()) } + .doSuspendableAnswer { throw IOException("connection reset") } + // allowed right after the failure, paused by the time the backoff delay has elapsed + val answers = mutableListOf(true, false) + + val result = syncer.pullMessagesFlow(target(), HashMap(), canRetry = { answers.removeFirst() }).first() + + assertTrue(result is ChatPullResult.Error) + verifyBlocking(network, times(1)) { pullChatMessages(any(), any(), any()) } + } + @Test fun `catchUpRoom marks the sync as failed when the server request errors`() = runTest {