Propagate Sync Cancellation and Exceptions Correctly

[sunkup]

Purpose

It's true, like stated in the issue, that Syncer.invoke()'s catch (e: Exception) has no CancellationException case, so a cancellation falls into the generic else branch, is logged at SEVERE and turned into syncResult.hardError = true instead of being re-thrown. This affects events/tasks/jtx today (contacts never get there because of the extra catch described below).

The message Couldn't sync contacts, as mentioned in the issuecomes fromAddressBookSyncer, not from Syncer.invoke()(whose message isCouldn't sync com.android.contacts), and the logged SyncResult` has no error set at all.

So the "swallow point" is AddressBookSyncer.syncAddressBook() (core/src/main/kotlin/at/bitfire/davdroid/sync/AddressBookSyncer.kt:113):

} catch(e: Exception) {
    logger.log(Level.SEVERE, "Couldn't sync contacts", e)
}
logger.info("Contacts sync complete")

This full on catch "swallows" everything without recording an error in syncResult, so the worker reports Result.success().

The other syncers (CalendarSyncer, JtxSyncer, TaskSyncer) do not have such an exception catch, they let
exceptions reach Syncer.invoke() instead.

Short description

1. AddressBookSyncer.kt: Remove the full on catch (e: Exception) from syncAddressBook() so performSync() re-thrown exceptions (including cancellation and DeadObjectException) and handleGroupMethodChange() exceptions reach Syncer.invoke(). Per-collection error handling already happens in SyncManager/SyncExceptionHandler.

2. Syncer.kt: In invoke(), re-throw CancellationException. Keep DeadObjectException as a soft error, InvalidAccountException as a warning, and all other exceptions as hard errors.

3. BaseSyncWorker.kt: In doIoWork(), explicitly log CancellationException at INFO and re-throw it. Update doSyncWork() KDoc to document the intended result/error mapping.

4. Tests: Update SyncerTest.kt to keep SyncResult and test cancellation propagation, DeadObjectException as softError, and unknown exception as hardError. Add AddressBookSyncerTest.kt to verify a CancellationException from ContactsSyncManager.performSync() travels through syncCollection().

Checklist

• The PR has a proper title, description and label.
• I have self-reviewed the PR.
• I have added documentation to complex functions and functions that can be used by other modules.
• I have added reasonable tests or consciously decided to not add tests.

[Copilot]

Pull request overview

Corrects sync cancellation propagation and exception classification across syncers and WorkManager workers.

Changes:

• Re-throws cancellation instead of recording it as a sync error.
• Classifies provider death and unexpected exceptions correctly.
• Adds regression tests for contacts and shared sync behavior.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
BaseSyncWorker.kt	Documents result mapping and propagates cancellation.
Syncer.kt	Handles cancellation and exception classification.
AddressBookSyncer.kt	Removes exception swallowing.
SyncerTest.kt	Tests exception classification.
AddressBookSyncerTest.kt	Tests contacts exception propagation.

Suppressed comments (1)

core/src/main/kotlin/at/bitfire/davdroid/sync/worker/BaseSyncWorker.kt:68

• issue (non-blocking): Not every hard error has already produced a user notification.

The generic branch in Syncer.invoke() only logs and sets hardError, and exceptions from handleGroupMethodChange() now reach that branch without passing through SyncManager.

 * - Hard error (the user has to take action, for instance fix their credentials): [Result.failure].
 *   [at.bitfire.davdroid.sync.SyncManager] has already notified the user about the details.

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.