fix(upload): keep an upload running in a foreground service and retry it by the kind of its error by ToteMeiSter · Pull Request #6828 · nextcloud/talk-android

ToteMeiSter · 2026-10-05T19:44:40Z

Depends on #6827 (which depends on #6825). Closes #6826. Fixes #4115.

Problem

A file upload stops when the screen goes off or the network is lost. On current master:

UploadAndShareFilesWorker runs without setForeground, so the system can stop it when the screen is off.
Every IOException uses one of MAX_UPLOAD_ATTEMPTS = 4, and a stop by a work constraint counts as an attempt. After that the upload is marked as failed, however long the network was gone.
A request waiting on a dead network is not interrupted when the work is stopped.
The cancel of the user goes through WorkManager.cancelWorkById(). A cancelled work that stays in the queue of a conversation (APPEND_OR_REPLACE) makes the next enqueueUniqueWork delete the other works of that queue, including running ones.

This PR builds on #6827, which makes a retry resume from the parts already on the server.

Solution

Foreground service

The worker runs as a foreground work of type dataSync with an ongoing notification with the progress (updated once a second) and a Cancel action. The manifest gets FOREGROUND_SERVICE_DATA_SYNC, the dataSync type for WorkManager's SystemForegroundService and a non-exported UploadCancelReceiver.
If the system refuses to start the service from the background (Android 12+), the upload continues as a regular work, without the notification.

Retry by the kind of the error (UploadRetryPolicy, a pure class)

A lost network, a dropped TLS connection, 5xx (except 507), 408 and 429: retry without a limit, no attempt is counted. The work waits for the network and resumes.
A refusal of the server (other 4xx, 507, certificate errors): up to four times, counted in the workspace.
A work that ends for good removes its parts from the server and shows the failed notification. It ends as success, not failure, so WorkManager does not fail the uploads queued behind it.
Backoff is LINEAR: with an unlimited number of attempts EXPONENTIAL grows to hours.
FileUploader throws HTTP errors, so the policy applies to small files too.

Cancel and stages

The cancel from the chat or the notification sets a flag in the workspace of the work. WorkManager is not asked to cancel. The worker checks the flag between its stages and in the progress callback, interrupts the upload, removes the parts, the assembled file and the placeholder message. The work is found by its id or, after a restart of the app, by a tag with the reference id.
The stages "uploaded" and "shared" and the id of the upload are kept in the workspace, so a run after a retry or a stop does not upload or share the file a second time.

ChunkedFileUploader

It builds its client with its own Dispatcher. okHttpClient.newBuilder() shares the dispatcher of the application client, so cancelAll() in stop() would also cancel chat requests and the signaling websocket. With its own dispatcher stop() interrupts only the requests of this uploader.
New abortUpload(localFile) (removes the parts of an earlier run for an uploader that did not start) and deleteUploadedFile(path) (removes a file already assembled, when the user cancels before the share).

Known limits

Cancel during video compression, or while the work waits for a retry, takes effect when that phase ends.
Doze and the time limit of dataSync foreground services on newer Android versions were not tested.
The same file sent to two conversations at once can get the same path on the server. Cancelling the second upload before the share can then remove the file of the first.

Tests

Run:

./gradlew :app:testGplayDebugUnitTest --tests "*UploadRetryPolicyTest" --tests "*UploadProgressThrottleTest" --tests "*UploadWorkspaceTest" --tests "*FileUploaderTest" --tests "*ChunkedFileUploaderResumeTest"

UploadRetryPolicyTest: classification of network, TLS, 5xx, 408, 429, 507, 4xx and certificate errors (also when wrapped by RxJava) and the decision (retry without limit, limit of four for refusals).
UploadWorkspaceTest: stages, upload id, cancel flag and server error count survive a new preparation.
FileUploaderTest: HTTP errors of a small upload are thrown.
ChunkedFileUploaderResumeTest: stop() does not cancel calls of other clients built from the same base client, abortUpload(localFile), deleteUploadedFile().
UploadProgressThrottleTest: the notification is updated at most once a second.

The worker itself has no instrumented test.

testGplayDebugUnitTest passes (818 tests, 0 failures, 1 skipped), ktlint is clean. detekt reports no new finding against master; the existing LargeClass finding of ChatViewModel grows from 1748 to 1749 lines (one line in cancelUpload).

Tested by the author on a Huawei DEL-LX9 with a fork build that contains #6825, #6827 and this PR: large file, screen off, airplane mode — the upload resumed and completed.

🏁 Checklist

⛑️ Tests (unit and/or integration) are included or not needed
🔖 Capability is checked or not needed
🔙 Backport requests are created or not needed: /backport to stable-xx.x
📅 Milestone is set
🌸 PR title is meaningful (if it should be in the changelog: is it meaningful to users?)

🤖 AI (if applicable)

The content of this PR was partly or fully generated using AI

The analysis, the code and the tests were prepared with an AI assistant (Claude Code). The commits carry an Assisted-by trailer. I reviewed the change and tested it on a device.

🤖 Generated with Claude Code

coderabbitai · 2026-10-05T20:01:09Z

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Last reviewed commit: 93d293a
Latest commit, not reviewed: 69bbfe4

Use the following commands to manage reviews:

@coderabbitai resume to resume automatic reviews.
@coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

▶️ Resume reviews
🔍 Trigger review

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI
Review profile: CHILL
Plan: Advanced
Run ID: bf904ef1-5789-4c73-8f00-85aab9f55b76

📥 Commits

Reviewing files that changed from the base of the PR and between 67d86dc and 93d293a.

📒 Files selected for processing (2)

app/src/main/java/com/nextcloud/talk/utils/FileUtils.kt
app/src/test/java/com/nextcloud/talk/utils/FileUtilsContentResolverTest.kt

Included review availability: This review used your included allowance. Your plan provides up to 4 included reviews per hour; 3 remain after this review.

📝 Walkthrough

Upload work persists prepared files and transfer stages so workers can resume after retries or system stops. Chunked uploads validate existing parts, send missing parts, and retry assembly once after an HTTP 400 response. The worker applies failure-specific retry decisions and uses foreground data-sync notifications with throttled progress and a cancel action. User cancellation records workspace state and removes pending upload data. File-copy and compression utilities accept configurable output directories.

Priority: ➖ Normal

Severity of issue fixed: Medium

Merge Risk: 🔵 Low · up to 93d29

Uploads are better protected against interrupted copies, but an older incomplete file may still be reused. Confirm cleanup or validate existing files before merging.

Security Architecture Review

Security architecture risk: 🟡 Moderate · up to 469e2

Resuming uploads improves interruption handling, but cancellation can leave sensitive upload copies retained while offline. Recovery also depends on how the server handles repeated sharing requests. The inspected changes preserve account-scoped credentials and restrict notification cancellation.

Retained concerns

Medium · security · inferred: Cancellation now marks an unfinished workspace without cancelling or rescheduling its network-constrained work. If an upload is waiting offline, cancellation cannot reach worker cleanup until connectivity returns. Prepared copies in noBackupFilesDir can therefore remain for an unbounded interval after the chat removes the visible placeholder, and subsequent uploads in the same conversation remain dependent on that unfinished work. This changes cancellation's data-lifecycle semantics; the copies remain app-private, and reconnection permits cleanup.
Medium · reliability · inferred: The ordinary path no longer checks system-stop state between upload completion and sharing, unlike the base. Sharing can consequently start after a stop, while durable shared state is recorded only after the remote request returns. An ambiguous response or interruption can leave recovery repeating publication of the recorded file. Stable reference IDs and completed shared markers are counterevidence, but their server-side deduplication guarantee is unavailable. The concern is weakened finalization containment and extended retry exposure, not a verified duplicate delivery or new authorization bypass.

Security review details

Security Blast Radius

inferred — The inspected authority is the upload's selected account and destination conversation. Lifecycle failures can affect its local payload, remote chunks or uploaded file, and dependent uploads in that conversation. The inspected URL construction does not establish authority over another account; server-side authorization and normalization remain outside the available evidence.

Security Findings and Attack Paths

inferred — The supported privacy concern requires an upload with retained preparation state, a disconnected worker and user cancellation. The visible item can disappear while its private payload remains pending cleanup. This is a data-retention path, not demonstrated unauthorized reading, credential disclosure or cross-account access.

Trust Boundaries and Controls

observed — Notification cancellation uses an explicit, immutable PendingIntent targeting a non-exported receiver, which rejects invalid work IDs. The chat upload entrypoint retains its attendee-permission check. These controls counter an interpretation that newly public Kotlin methods alone create an unauthenticated Android entrypoint.

Resilience and Maintainability Implications

observed — Retry classification separates transient connectivity from certificate and persistent server failures without relaxing TLS verification. Cancellation and terminal failure return success to contain queue failure, but cancellation's execution still depends on the original network constraint.

Hardening Proposals

proposed — Separate local cancellation completion from network-dependent cleanup. Retain a minimal durable cleanup record containing remote ownership identifiers, rather than retaining the entire prepared payload, and retry remote deletion independently without blocking the conversation queue.
proposed — Establish an explicit idempotency or reconciliation contract for both publication endpoints, and serialize stop-to-publication transitions. Recovery should distinguish a request that never committed from one whose response was lost before the shared marker was written.

Pre-merge checks: 3 · 2

Failed checks (2 warnings)

Check name	Status	Explanation	Resolution
Linked Issues check		The PR implements the main coding requirements in [`#6826`] and [`#4115`]. It adds resumable workspace state, error-based retry, cleanup, cancellation, foreground `dataSync` notifications, and uploader su…	Do not continue the upload as regular work when foreground startup fails. Defer or retry the upload until it can run as foreground `dataSync` work, or obtain an updated [`#6826`] requirement that explicitly accepts regular-work fallback.
Docstring Coverage		Docstring coverage is 21.66% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 217 functions across 20 files.	Write docstrings for the functions missing them to satisfy the coverage threshold.

Passed checks (3 passed)

Check name	Status	Explanation
Title check		The title clearly summarizes the main changes: foreground-service uploads and error-specific retries.
Description check		The description explains the problem, solution, known limits, tests, dependencies, and checklist. It omits the template's screenshots and TODO sections, but the required technical information is other…
Out of Scope Changes check		The manifest, receiver, notification, workspace, worker, uploader, file-preparation, utility, and test changes support the upload resume, retry, foreground execution, cancellation, cleanup, or prepara…

Full details: Linked Issues check

Explanation

The PR implements the main coding requirements in [#6826] and [#4115]. It adds resumable workspace state, error-based retry, cleanup, cancellation, foreground dataSync notifications, and uploader support. However, UploadAndShareFilesWorker.startForeground() catches foreground startup errors and continues the upload as a regular job. This does not meet [#6826]'s requirement to run the upload as foreground dataSync work. The current PR description confirms this fallback.

Autopilot · 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.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands.}

coderabbitai

Actionable comments posted: 2

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI
Review profile: CHILL
Plan: Advanced
Run ID: 2b9b3b8e-5978-45b2-8d16-2f4f8d2a0b96

📥 Commits

Reviewing files that changed from the base of the PR and between f3c9097 and 4ce288e.

📒 Files selected for processing (20)

app/src/main/AndroidManifest.xml
app/src/main/java/com/nextcloud/talk/chat/viewmodels/ChatViewModel.kt
app/src/main/java/com/nextcloud/talk/jobs/UploadAndShareFilesWorker.kt
app/src/main/java/com/nextcloud/talk/receivers/UploadCancelReceiver.kt
app/src/main/java/com/nextcloud/talk/upload/UploadNotification.kt
app/src/main/java/com/nextcloud/talk/upload/UploadProgressThrottle.kt
app/src/main/java/com/nextcloud/talk/upload/UploadRetryPolicy.kt
app/src/main/java/com/nextcloud/talk/upload/UploadWorkspace.kt
app/src/main/java/com/nextcloud/talk/upload/chunked/ChunkedFileUploader.kt
app/src/main/java/com/nextcloud/talk/upload/normal/FileUploader.kt
app/src/main/java/com/nextcloud/talk/utils/FileUtils.kt
app/src/main/java/com/nextcloud/talk/utils/ImageCompressor.kt
app/src/main/java/com/nextcloud/talk/utils/VideoCompressor.kt
app/src/test/java/com/nextcloud/talk/upload/UploadProgressThrottleTest.kt
app/src/test/java/com/nextcloud/talk/upload/UploadRetryPolicyTest.kt
app/src/test/java/com/nextcloud/talk/upload/UploadWorkspaceTest.kt
app/src/test/java/com/nextcloud/talk/upload/chunked/ChunkedFileUploaderAbortTest.kt
app/src/test/java/com/nextcloud/talk/upload/chunked/ChunkedFileUploaderResumeTest.kt
app/src/test/java/com/nextcloud/talk/upload/normal/FileUploaderTest.kt
app/src/test/java/com/nextcloud/talk/utils/FileUtilsContentResolverTest.kt

Included review availability: This review used your included allowance. Your plan provides up to 4 included reviews per hour; 1 remain after this review.

ToteMeiSter · 2026-10-05T20:58:10Z

@coderabbitai review

coderabbitai · 2026-10-05T21:09:22Z

⚠️ Action not completed

Already reviewed the last commit. Use @coderabbitai full review to rerun a review of the entire changeset.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

ToteMeiSter · 2026-10-05T21:24:53Z

@coderabbitai, on the two retained concerns of the summary:

Cancel while offline. This is intended. Cancelling through WorkManager.cancelWorkById() is what this PR removes: a cancelled work in the APPEND_OR_REPLACE queue of a conversation made the next enqueueUniqueWork delete the other works of that queue, including running ones. The flag keeps the queue intact. Until the network is back, the prepared copy stays in app-private noBackupFilesDir; the next run sees the flag and removes it, and deleteFinishedWorkspaces removes what a finished work left. The uploads queued behind it need the network too, so they do not wait longer than they would anyway.
Share after a stop. A repeated share cannot post a second message: the server rejects it (draft path: the file was already moved, 404; share path: "Path is already shared with this conversation", 403). The narrow case left is a stop exactly between a share request that reached the server and markShared(): the next run gets that error and shows the upload as failed although the file is in the chat. Treating these answers as success after an attempt without a clear response is a follow-up I would keep out of this PR.

Prepared with an AI assistant and reviewed by me.

coderabbitai · 2026-10-05T21:25:15Z

Tip

For best results, initiate chat on the files or code changes.

@ToteMeiSter, thanks for the clarification.

Cancel while offline: I withdraw this concern. Flag-based cancellation preserves the conversation’s APPEND_OR_REPLACE queue. Keeping the prepared copy in noBackupFilesDir until the worker can perform cleanup is intentional.
Share after a stop: I withdraw the duplicate-message concern under the server behavior you described. The remaining case is a false failure: the server completes the share, but the worker stops before markShared(). I accept keeping reconciliation of that result outside this PR.

Would you like me to open a follow-up issue for that false-failure case?

✏️ Learnings added

Learnt from: ToteMeiSter
URL: https://github.com/nextcloud/talk-android/pull/6828

Timestamp: 2026-10-05T21:25:14.039Z
Learning: In nextcloud/talk-android, UploadAndShareFilesWorker uses a persisted cancellation flag instead of WorkManager.cancelWorkById() to preserve the conversation's APPEND_OR_REPLACE work queue. The author explains that cancelling work caused a later enqueueUniqueWork call to replace other queued work, including running work. Offline cancellation intentionally retains the prepared copy in app-private noBackupFilesDir until the worker can run cleanup; deleteFinishedWorkspaces also cleans leftovers from finished work.

Learnt from: ToteMeiSter
URL: https://github.com/nextcloud/talk-android/pull/6828

Timestamp: 2026-10-05T21:25:14.039Z
Learning: For nextcloud/talk-android upload sharing, the author states that repeated requests cannot post a second message: the draft path returns HTTP 404 after the file has moved, and the share path returns HTTP 403 with "Path is already shared with this conversation". A stop after server-side completion but before UploadWorkspace.markShared() can therefore produce a false upload-failure state, not a duplicate message. Reconciliation of ambiguous share results is intentionally deferred beyond PR #6828.

Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.

You are interacting with an AI system.

ToteMeiSter · 2026-10-06T12:53:55Z

This is a bug fix and all CodeRabbit review threads are resolved. Once it is approved and merged, would it be possible to backport it to stable-25.0.x (e.g. for 25.0.3)? The bug is present in v25.0.2.

Suggested command for a maintainer after merge: /backport to stable-25.0.x

This comment was drafted with AI assistance (Claude Code).

coderabbitai

Actionable comments posted: 1

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI
Review profile: CHILL
Plan: Advanced
Run ID: f22b0f6b-ba83-40a4-abd6-f6347d007754

📥 Commits

Reviewing files that changed from the base of the PR and between 469e2bb and 4375439.

📒 Files selected for processing (4)

app/src/main/java/com/nextcloud/talk/chat/viewmodels/ChatViewModel.kt
app/src/main/java/com/nextcloud/talk/upload/chunked/ChunkedFileUploader.kt
app/src/main/java/com/nextcloud/talk/utils/FileUtils.kt
app/src/test/java/com/nextcloud/talk/upload/chunked/ChunkedFileUploaderTest.kt

Included review availability: This review used your included allowance. Your plan provides up to 4 included reviews per hour; 2 remain after this review.

coderabbitai

Actionable comments posted: 1

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI
Review profile: CHILL
Plan: Advanced
Run ID: ebeaba79-89fe-4361-9f17-79e3687bdf51

📥 Commits

Reviewing files that changed from the base of the PR and between 4375439 and 67d86dc.

📒 Files selected for processing (6)

app/src/main/java/com/nextcloud/talk/jobs/UploadAndShareFilesWorker.kt
app/src/main/java/com/nextcloud/talk/upload/chunked/ChunkedFileUploader.kt
app/src/main/java/com/nextcloud/talk/utils/FileUtils.kt
app/src/test/java/com/nextcloud/talk/upload/chunked/ChunkedFileUploaderAbortTest.kt
app/src/test/java/com/nextcloud/talk/upload/chunked/ChunkedFileUploaderResumeTest.kt
app/src/test/java/com/nextcloud/talk/utils/FileUtilsContentResolverTest.kt

Included review availability: This review used your included allowance. Your plan provides up to 4 included reviews per hour; 3 remain after this review.

rapterjet2004

Thanks, this is careful work: the own Dispatcher for stop(), ending failed works as success so the APPEND_OR_REPLACE queue isn't torn down, the cancel flag instead of cancelWorkById, and the stage markers in the workspace all look right. (Reviewed on top of #6827, i.e. only the commits of this PR.)

Change requested: a ceiling for the unlimited retries. The NETWORK class covers every 5xx except 507, 408, 429 and any IOException other than FileNotFoundException. A server that keeps answering 500 (broken app, misconfigured proxy), or a local read error on the prepared file, makes the upload retry forever: the placeholder stays "sending", a foreground notification shows on every run, and the prepared copy (possibly GBs) stays in noBackupFilesDir, until the user finds and cancels it. Waiting for the network without a limit is fine, but please give the "server temporarily unavailable" cases (5xx/408/429) a generous ceiling, e.g. a high count or a maximum total age since the first run (stored in the workspace, e.g. 24 h), after which the upload fails like a refusal.

Other points (non-blocking):

Interaction with #6858: #6858 marks the placeholder failed when the work reaches WorkInfo.State.FAILED. With this PR a failed upload ends as SUCCEEDED, so that branch never fires, and the case #6858 handles (the worker fails before the placeholder exists) would leave the placeholder PENDING. Whichever lands second needs to adapt, e.g. by reporting the failure in the work's output data.
Notification for every upload: startForeground() runs before the preparation for every upload, so even a small photo flashes an "Uploading…" notification. Starting the foreground service only for chunked uploads (> 1 MiB), or after a short delay, would avoid that.
Play Console: FOREGROUND_SERVICE_DATA_SYNC with targetSdk 34+ needs a foreground service declaration in the Play Console. Just flagging it so it's handled before release.
Leftover draft file: a share that finally fails leaves the uploaded file in the draft folder: failUpload → abortRemote() removes the parts but deleteUploadedFile only runs for a cancel.

Merge order note: #6825 → #6827 → this PR, each rebased.

… has A chunked upload restarted from zero after every interruption, although the parts it had sent were still on the server: - The PROPFIND on the upload folder named no properties. isFile is set only from a resourcetype property, so it stayed false for every member and the list of uploaded parts was always empty. The request now asks for resourcetype, getcontentlength and displayname. - Parts are accepted only when their name is well formed and their size matches the name. A wrong or foreign entry is uploaded again or ignored. - Only the missing parts are sent. - The assembling MOVE sends OC-Total-Length, so the server can check that the parts add up to the file. - A MOVE the server rejects with 400 removes the parts once and sends the file again. The caller supplies the "already restarted" mark. - stop() interrupts the upload between two parts and keeps the parts on the server. Only abortUpload() removes them. - upload() no longer swallows exceptions. A failed request is thrown (an HttpException for an HTTP error, an IOException for a network error) and the parts stay on the server. A lost network while listing the parts is thrown too instead of being taken for "no parts", which would send the whole file again. Signed-off-by: Krainov Gleb <krajnov.g@kontentplus.ru> Assisted-by: Claude-Code:claude-sonnet-5-5

The folder of the parts on the server is keyed by the name, the modification time and the length of the file. UploadAndShareFilesWorker copied the content uri to the cache and compressed it again on every run. Each run produced a file with a new modification time, so a retry used another folder and sent the whole file again. The worker now prepares the file once into a directory of its own work (cacheDir/uploads/<work id>) and uploads the same file on every run: - UploadWorkspace keeps the prepared file and the "parts were removed once" mark, and holds a lock, because WorkManager does not interrupt the thread of a stopped worker and an old run can still be busy. - A workspace stays after a stop or before a retry. It is removed when the work ends, and the workspaces of finished or unknown works are removed before the next upload. - A run that is stopped while the file is prepared does not store the half-finished file. - A copy of a content uri that could not be read completely is not used as the file to upload. - onStopped() keeps the parts on the server when the system or the network stopped the work. The parts are removed only when the user cancelled it. Signed-off-by: Krainov Gleb <krajnov.g@kontentplus.ru> Assisted-by: Claude-Code:claude-sonnet-5-5

Remove the rejected parts before marking the upload as restarted: when the DELETE fails, the retry must still be allowed to remove them. Keep the upload workspace in noBackupFilesDir instead of cacheDir. The system may clear the cache under storage pressure while an upload waits for the network. A new copy of the file changes its resume key and leaves the parts on the server orphaned, and after a process restart the content URI may no longer be readable. Finished workspaces are still removed by the worker. Signed-off-by: Krainov Gleb <krajnov.g@kontentplus.ru> Assisted-by: Claude-Code:claude-opus-5-5

A stop that came after the last check of doUpload() and before the chunked uploader was assigned found no uploader in onStopped(), so the upload ran on to the end. Assign the uploader first and check for a stop after that: an earlier stop is seen by the check, a later one reaches the uploader. Signed-off-by: Krainov Gleb <krajnov.g@kontentplus.ru> Assisted-by: Claude-Code:claude-opus-5-5

doWork() removes the cancel marker of its reference id when it ends, but WorkManager may call onStopped() after that. onStopped() then saw no cancel and only stopped the uploader, so the parts stayed on the server. Latch the cancel once it is seen, and check it once more right before the marker is removed. Signed-off-by: Krainov Gleb <krajnov.g@kontentplus.ru> Assisted-by: Claude-Code:claude-opus-5-5

- Delete the workspace of a cancelled run even when it was marked to be kept. - Copy a file:// source into the workspace, so the prepared upload does not depend on the caller's cache file. - Fix the upload folder before the uploader is published, so a cancel that arrives early can still remove the parts; abort at once if it already came. - Restart after HTTP 400 only when the assembly was rejected, not a part. - Sort the listed parts before looking for gaps. - Treat 401, 403, 408 and 429 while listing the parts as errors, not as an empty folder. - Delete a partial copy when the stream breaks off. Signed-off-by: Krainov Gleb <krajnov.g@kontentplus.ru> Assisted-by: Claude-Code:claude-sonnet-5-5

copyFileToCache returned any existing target, including the leftover of a copy that died half way, and removed the target only on IOException. - Copy into a temporary file next to the target and rename it after a complete copy; remove the temporary file on any exception. - Treat an empty existing target as no cache hit and copy again. Assisted-by: Claude-Code:claude-sonnet-5-5 Signed-off-by: Krainov Gleb <krajnov.g@kontentplus.ru>

stop() cancels the calls of the uploader to interrupt a request that is waiting on a dead network. The uploader built its client with okHttpClient.newBuilder(), which shares the Dispatcher of the application client. Cancelling all calls of that dispatcher would also cancel the chat requests, the signaling websocket and the other uploads. The uploader now builds its client with a Dispatcher of its own, so stop() and abortUpload() cancel only its own calls. Also: - abortUpload(localFile) removes the parts of an earlier run of a file for an uploader that has not started an upload, e.g. when the user cancels while the work waits for a retry. - deleteUploadedFile(path) removes a file that was already assembled on the server, e.g. when the user cancels between the upload and the share. - abortUpload() catches every exception: it runs inside the WorkManager cancellation handler, where any exception crashes the process. Signed-off-by: Krainov Gleb <krajnov.g@kontentplus.ru> Assisted-by: Claude-Code:claude-sonnet-5-5

… it by the kind of its error A file upload stopped when the screen went off or the network was lost, and the worker gave up after four attempts, whatever the reason of the stop. Foreground service - The worker runs as a foreground work of type dataSync with an ongoing notification that shows the progress and a Cancel action. The manifest gets the FOREGROUND_SERVICE_DATA_SYNC permission, the dataSync type of the WorkManager SystemForegroundService and a non-exported cancel receiver. When the system refuses to start the service from the background (Android 12+), the upload continues as a regular work. Retry by the kind of the error (UploadRetryPolicy) - A lost network, a dropped TLS connection, 5xx (except 507), 408 and 429 are retried without a limit and do not count as an attempt: the work waits for the network and resumes from the parts on the server. - Only a refusal of the server (4xx, 507, certificate errors) counts, up to four times. The count is kept in the workspace. - A work that ends for good removes its parts from the server and shows the failed notification. It ends as success, because a failed work makes WorkManager fail the uploads queued behind it in the same conversation. - Backoff is linear. Exponential backoff with an unlimited number of attempts grows to hours. - The upload of a small file throws its HTTP errors, so the same policy applies to it. Stages and cancel - The stages "uploaded" and "shared", the id of the upload and the cancel flag are kept in the workspace. A run after a retry or a stop does not upload the file or share it a second time. - The user's cancel (chat or notification) only sets a flag in the workspace of the work. The worker sees it between its stages and in the progress callback, interrupts the upload, removes the parts, the assembled file and the placeholder message. WorkManager is not asked to cancel the work: a cancelled work in the queue of a conversation makes the next APPEND_OR_REPLACE delete the other works of that queue, even running ones. The work is found by its id or, after a restart of the app, by its tag. Signed-off-by: Krainov Gleb <krajnov.g@kontentplus.ru> Assisted-by: Claude-Code:claude-sonnet-5-5

The cancel flag is the only way to stop an upload. When the user taps cancel and leaves the chat at once, the cleared ViewModel could drop the viewModelScope jobs before they set the flag and removed the placeholder, so the cancelled file would still be shared. Signed-off-by: Krainov Gleb <krajnov.g@kontentplus.ru> Assisted-by: Claude-Code:claude-opus-5-5

A stop that came after the last check and before the chunked uploader was assigned found no uploader in onStopped(), so the upload ran on to the end. Assign the uploader first and check for a stop after that: an earlier stop is seen by the check, a later one reaches the uploader. A cancelled work now finds the parts by the prepared file, so they are removed also when the uploader was assigned but did not start. Signed-off-by: Krainov Gleb <krajnov.g@kontentplus.ru> Assisted-by: Claude-Code:claude-opus-5-5

…d it The uploads of a conversation are chained with APPEND_OR_REPLACE, and WorkManager gives every request chained behind a cancelled or failed one the same state without running it. The new test runs three uploads of one conversation on a real WorkManager and cancels the first, a middle, the last and every one of them, and one by the reference tag as after a restart of the app. The other uploads must finish as SUCCEEDED and their placeholder rows must not stay PENDING. doWork() injects its dependencies only when none are set yet, so the test can give the worker a factory with its own dependencies. Signed-off-by: Krainov Gleb <krajnov.g@kontentplus.ru> Assisted-by: Claude-Code:claude-sonnet-5-5

…ads behind it The cancel of an upload ends the work as success, because a failed work makes WorkManager fail every upload queued behind it in the conversation without running it. cancelledResult() removed the placeholder row without a guard: an exception from the database (e.g. a closed database) left doWork() as an exception, the work ended as FAILED, and the uploads behind the cancelled one ended as FAILED with their rows PENDING for ever. The row is removed by the chat as well, so a failed removal is only logged and the work still ends as success. Signed-off-by: Krainov Gleb <krajnov.g@kontentplus.ru> Assisted-by: Claude-Code:claude-sonnet-5-5

…led run WorkManager ignores the result of a work it stopped (for example when the network is lost) and runs the work again. A user's cancel that came while the stopped run was still winding down made cancelledResult() end the run and the finally block of doWork() delete the workspace together with the cancel flag. The next run did not see the flag, uploaded the file and shared it, although its placeholder was already removed. cancelledResult() keeps the workspace when the run was stopped, as successResult() does. The next run finds the flag, removes the parts and the placeholder again (both are safe to repeat) and deletes the workspace. The same holds for the run that gets a retry because an older run still holds the lock of the workspace. The test stops the running work as WorkManager does, cancels it while the old run is still busy and checks that the flag survives and that the next run is cancelled too. The test restores the application it replaced. Signed-off-by: Krainov Gleb <krajnov.g@kontentplus.ru> Assisted-by: Claude-Code:claude-sonnet-5-5

Answers of 5xx (except 507), 408 and 429 were retried without a limit. They are now a failure kind of their own, UNAVAILABLE, which is retried for 24 hours. The 24 hours count from the first "unavailable" answer after which the upload did not move on. The workspace keeps that time together with the progress it had then; a later answer with more progress (more chunks sent, or the file uploaded) starts the time again. Waiting for the network without an answer of the server is still not limited. Signed-off-by: Krainov Gleb <krajnov.g@kontentplus.ru> Assisted-by: Claude-Code:claude-sonnet-5-5

ChunkFromFileRequestBody wraps every error of reading a chunk in a FileNotFoundException, with the real IOException as its cause. classify saw that cause as a lost network and retried for ever. A FileNotFoundException anywhere in the chain is now OTHER; an HTTP code still wins. The file is in noBackupFilesDir, so waiting does not repair it. Signed-off-by: Krainov Gleb <krajnov.g@kontentplus.ru> Assisted-by: Claude-Code:claude-sonnet-5-5

A failed upload ends as SUCCEEDED on purpose, because WorkManager fails the works chained behind a failed one without running them. An observer of the work could therefore not tell a failure from a delivery, and a placeholder message of an upload that failed before it was inserted stayed PENDING. failUpload now sets KEY_UPLOAD_FAILED in the output data. A cancel does not. Signed-off-by: Krainov Gleb <krajnov.g@kontentplus.ru> Assisted-by: Claude-Code:claude-sonnet-5-5

…oaded in chunks startForeground() ran before the preparation for every upload, so a 10 KB photo flashed a progress notification. The worker now asks for the size of the source first (File.length() for file://, FileUtils.querySize otherwise) and starts the foreground service only for files above the chunk threshold or of unknown size. An upload whose file is already on the server has one request left and needs no service either. The progress callback already checks isForeground. Signed-off-by: Krainov Gleb <krajnov.g@kontentplus.ru> Assisted-by: Claude-Code:claude-sonnet-5-5

A failed upload removed the parts of a chunked upload but left the assembled file in the draft folder of the conversation. The server moves that file out of the draft folder only when the message is posted, so after a failure nobody claims it. failUpload now deletes it; a file the server already moved answers 404, which deleteUploadedFile swallows. The workspace remembers that the file went to the draft folder, so a failure that comes in a later run knows it too. A file in the attachment folder of Talk (servers without conversation subfolders) is a file of the user and stays. Signed-off-by: Krainov Gleb <krajnov.g@kontentplus.ru> Assisted-by: Claude-Code:claude-sonnet-5-5

…red copy A later run of an upload asked the provider of the source for the size again, an IPC call that can be slow or answer differently, while the copy in the workspace is what is uploaded. The foreground service is now decided on the size of the prepared copy when there is one; the source is asked only before the copy exists. Signed-off-by: Krainov Gleb <krajnov.g@kontentplus.ru> Assisted-by: Claude-Code:claude-sonnet-5-5

The progress for the 24 hours lived in memory of one run, so a run that began after a long pause counted from -1 and failed an upload the server had been taking data from. Now: - the furthest progress is stored in the workspace (written under the progress throttle, only when it grows), and the point of the clock is computed by the workspace: the stored maximum, or "uploaded" once the file is on the server; - the upload fails only after 24 hours AND at least 10 "unavailable" answers, so time without a network does not count as a refusing server; - a clock that went back starts the time again; - a workspace that cannot be written no longer throws out of the work (that failed the work and the uploads queued behind it): the error is logged and the upload is tried again. KEY_UPLOAD_FAILED now says to read the flag from the output data only, because the queue copies the output into the input of the next work. Signed-off-by: Krainov Gleb <krajnov.g@kontentplus.ru> Assisted-by: Claude-Code:claude-sonnet-5-5

…eginning After the assembly was rejected (markRestarted) or the prepared file was made again (a new md5, so new chunks), the stored furthest progress, say 100, stayed. The new send goes 0..99 and never passes it, so every "unavailable" answer counted as no progress and the upload failed after 24 hours although the server took data. Both cases are a new send: the progress and the clock of the unavailable answers are removed. Signed-off-by: Krainov Gleb <krajnov.g@kontentplus.ru> Assisted-by: Claude-Code:claude-sonnet-5-5

ToteMeiSter · 2026-10-15T18:11:35Z

Thanks for the review. All points are addressed in new commits; both branches are rebased onto master with a force-push.

Ceiling for the retries (69aa7bd, 4d48c3c, ee4c2b7)
5xx except 507, 408 and 429 are now their own class, "server unavailable". Such an upload is retried until the server has answered "unavailable" for 24 h and at least 10 times without the upload getting further; then it fails like a refusal: parts removed, placeholder marked as failed, the prepared copy in noBackupFilesDir deleted. The clock starts at the first "unavailable" answer, not at the first run, and restarts when the upload got further since then (a higher percentage sent in any run, or the upload stage finished) or starts again from the beginning. So a long wait for the network still has no limit: failing needs the 10 answers as well as the 24 h, and one 503 after a day offline does not end the upload. A slow multi-GB upload that sees a 503 at hour 1 and another one at hour 30 keeps going as long as the server accepted data in between. The time, the progress point and the number of answers are stored in the workspace and survive a reboot. An IOException without an HTTP answer is still waited for without a limit.

Local read error of the prepared file (0bcba5d)
This is not a network error and now fails at once. ChunkFromFileRequestBody turns any read error of a chunk into a FileNotFoundException with the IOException as its cause; the classifier skipped the FileNotFoundException itself but matched its cause as a network error. A FileNotFoundException anywhere in the cause chain is now a local failure. The normal (non-chunked) upload already failed at once on a read error, because createRequestBody returns null.

fix(chat): resolve the name, type and size of a file to send off the main thread #6858 (68012a3): a failed upload still ends as SUCCEEDED, so the uploads queued behind it run, and it now puts KEY_UPLOAD_FAILED = true into the output data of the work. A cancel does not set it. fix(chat): resolve the name, type and size of a file to send off the main thread #6858 then marks the placeholder when the work is FAILED or SUCCEEDED with this flag; whichever PR lands second gets this line. In our fork build, which has both, this is in place and covered by a test (worker reports the failure before the placeholder exists → placeholder failed; a successful upload → placeholder untouched).
Notification for every upload (949a887, fafd1a5): the foreground service now starts only when the source is larger than the chunk threshold (1 MiB) or its size is unknown, and not at all when only the share is left. Small photos upload as a regular job without a notification.
Play Console: noted, thanks. The FOREGROUND_SERVICE_DATA_SYNC declaration is a Play Console form, no code change.
Leftover draft file (5872eac): a failed upload that used the conversation subfolder now also deletes its file in the draft folder (postAttachmentToRoom moves the file out of the draft folder, so a file that was already posted is not touched). The legacy path (attachment folder without subfolders) keeps the uploaded file as before: there it is a regular file in the user's Talk folder, not a draft.

Each fix has a unit test that fails without it. Unit tests, ktlint and detekt pass locally; detekt count unchanged.

#6827 and this PR are rebased onto master in that order.

This reply was drafted with AI assistance (Claude Code).

This was referenced Oct 5, 2026

fix(upload): resume chunked uploads from the parts the server already has #6827

Open

Large file uploads to a chat do not survive screen-off, Doze or network loss #6826

Open

fix(upload): retry chunked uploads after network errors #6805

Merged

coderabbitai Bot reviewed Oct 5, 2026

View reviewed changes

Comment thread app/src/main/java/com/nextcloud/talk/chat/viewmodels/ChatViewModel.kt Outdated

Comment thread app/src/main/java/com/nextcloud/talk/jobs/UploadAndShareFilesWorker.kt Outdated

AndyScherzinger added 3. to review Waiting for reviews AI assisted labels Oct 10, 2026

ToteMeiSter force-pushed the fix/upload-foreground-service branch from 469e2bb to 4375439 Compare October 12, 2026 20:32

coderabbitai Bot reviewed Oct 12, 2026

View reviewed changes

Comment thread app/src/main/java/com/nextcloud/talk/upload/chunked/ChunkedFileUploader.kt

ToteMeiSter force-pushed the fix/upload-foreground-service branch from 4375439 to 67d86dc Compare October 13, 2026 05:17

coderabbitai Bot reviewed Oct 13, 2026

View reviewed changes

Comment thread app/src/main/java/com/nextcloud/talk/utils/FileUtils.kt Outdated

ToteMeiSter force-pushed the fix/upload-foreground-service branch from 67d86dc to 93d293a Compare October 13, 2026 06:29

rapterjet2004 mentioned this pull request Oct 14, 2026

fix(upload): never throw from abortUpload when the worker is stopped #6825

Merged

4 of 6 tasks

rapterjet2004 requested changes Oct 15, 2026

View reviewed changes

ToteMeiSter added 11 commits October 15, 2026 18:05

ToteMeiSter force-pushed the fix/upload-foreground-service branch from 69bbfe4 to ee4c2b7 Compare October 15, 2026 18:09

rapterjet2004 mentioned this pull request Oct 15, 2026

fix(chat): resolve the name, type and size of a file to send off the main thread #6858

Open

4 of 6 tasks

Conversation

ToteMeiSter commented Oct 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Problem

Solution

Known limits

Tests

🏁 Checklist

🤖 AI (if applicable)

Uh oh!

coderabbitai Bot commented Oct 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Reviews paused

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

ToteMeiSter commented Oct 5, 2026

Uh oh!

coderabbitai Bot commented Oct 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ToteMeiSter commented Oct 5, 2026

Uh oh!

coderabbitai Bot commented Oct 5, 2026

Uh oh!

ToteMeiSter commented Oct 6, 2026

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

rapterjet2004 left a comment

Choose a reason for hiding this comment

Uh oh!

ToteMeiSter commented Oct 15, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

ToteMeiSter commented Oct 5, 2026 •

edited

Loading

coderabbitai Bot commented Oct 5, 2026 •

edited

Loading

coderabbitai Bot commented Oct 5, 2026 •

edited

Loading