Repository navigation
fix(chat): do not retry a long poll while the chat is paused - #6847
ToteMeiSter wants to merge 2 commits into
Conversation
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 <krajnov.g@kontentplus.ru> Assisted-by: Claude-Code:claude-sonnet-5-5
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info
📝 Walkthrough
Priority: ⬇️ Low Merge Risk: ⚪ Minimal · up to A failed long poll will not retry while chat is paused, and polling waits for resume. The change is mergeable after normal checks. Pre-merge checks |
  |
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Docstring Coverage |  |
Docstring coverage is 37.50% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 8 functions across 3 files. | Write docstrings for the functions missing them to satisfy the coverage threshold. |
✅ Passed checks (4 passed)
-
Check name Status Explanation Title check The title clearly and concisely describes the main change: preventing chat long-poll retries while the chat is paused. Description check The description explains the problem, change, testing, notes, and checklist status. The screenshots and TODO sections are not included, but they are non-critical for this change. Linked Issues check Check skipped because no linked issues were found for this pull request. Out of Scope Changes check Check skipped because no linked issues were found for this pull request.
- Fix all pre-merge checks with AI
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.
Comment @coderabbitai help to get the list of available commands.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
ℹ️ Review info
⚙️ Run configuration
- Configuration used: Organization UI
- Review profile: CHILL
- Plan: Advanced
- Run ID:
4d300af6-4360-4968-b47d-fdf5bcc507ae
📒 Files selected for processing (3)
app/src/main/java/com/nextcloud/talk/chat/data/network/ChatMessageSyncer.ktapp/src/main/java/com/nextcloud/talk/chat/data/network/OfflineFirstChatRepository.ktapp/src/test/java/com/nextcloud/talk/chat/data/network/ChatMessageSyncerTest.kt
Included review availability: This review used your included allowance. Your plan provides up to 4 included reviews per hour; 0 remain after this review.
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 <krajnov.g@kontentplus.ru> Assisted-by: Claude-Code:claude-opus-5-5
2 participants
Problem
On a server without HPB the chat is kept up to date by a long poll (
initLongPolling,timeout=30). It checksitIsPausedonly before it starts a new cycle.ChatMessageSyncer.pullMessagesFlowretries a failed request up to four times with a 1–3 s delay and does not check the pause state. When the system closes the socket of a backgrounded app, the request fails and is sent again, so a newtimeout=30request starts in the background.Change
pullMessagesFlowandpullAndPersistMessagestake acanRetry: () -> Booleanpredicate (default: always true). The long poll passes!itIsPaused. When the chat is paused, a failed request ends with an error andinitLongPollingwaits for the resume as before. Other callers are unchanged.Notes
markNotificationsAsRead=0here: with this change no long poll request is sent or retried while paused, and a flag built at the end of a cycle would be stale after the resume.OfflineFirstChatRepository.ktlike fix(chat): keep notifications when fetching new messages while paused #6841 and fix(chat): skip insurance requests while the chat is paused #6842, in other lines (initLongPollingcall andgetAndPersistMessages). A trial merge with both branches is clean.How to test
ChatMessageSyncerTest(pullMessagesFlow does not retry a failed request once retrying is not allowed) fails without the change (TooManyActualInvocations), passes with it.🏁 Checklist
/backport to stable-xx.x🤖 AI (if applicable)
The change was drafted with Claude Code (
Assisted-by: Claude-Code:claude-sonnet-5-5).