MS-1538 Sync worker error propagation fix - #1783
Conversation
There was a problem hiding this comment.
Pull request overview
This PR updates the :infra:event-sync WorkManager sync flow so that up-sync/down-sync workers can always report SUCCEEDED (allowing the end-of-sync reporter worker to run) while still surfacing sync errors to the UI via explicit failure flags in worker outputData.
Changes:
- Propagate specific “terminal” sync exceptions from tasks to workers and encode them as failure flags in worker output data.
- Update sync state resolution to treat a
SUCCEEDEDworker as failed when its output contains any failure flag. - Extend and adjust unit tests to validate the new “success + failure flags” behavior and end-sync success-time handling.
Reviewed changes
Copilot reviewed 19 out of 19 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| infra/event-sync/src/main/java/com/simprints/infra/eventsync/sync/up/workers/EventUpSyncUploaderWorker.kt | Always returns success; encodes error reasons (maintenance/cloud/relogin/429) into output data. |
| infra/event-sync/src/main/java/com/simprints/infra/eventsync/sync/up/tasks/EventUpSyncTask.kt | Re-throws “terminal” exceptions after recording failed state/progress. |
| infra/event-sync/src/main/java/com/simprints/infra/eventsync/sync/down/workers/BaseEventDownSyncDownloaderWorker.kt | Passes progress/max into exception handling; throws relogin exception when project is missing. |
| infra/event-sync/src/main/java/com/simprints/infra/eventsync/sync/down/workers/SimprintsEventDownSyncDownloaderWorker.kt | Returns success with output failure flags instead of failing/retrying for certain exceptions. |
| infra/event-sync/src/main/java/com/simprints/infra/eventsync/sync/down/workers/CommCareEventSyncDownloaderWorker.kt | Returns success with permission/relogin failure flags instead of failing/retrying. |
| infra/event-sync/src/main/java/com/simprints/infra/eventsync/sync/down/tasks/BaseEventDownSyncTask.kt | Defers rethrow of terminal errors until after emitting progress and logging request event. |
| infra/event-sync/src/main/java/com/simprints/infra/eventsync/sync/down/tasks/SimprintsEventDownSyncTask.kt | Expands “shouldRethrowError” set to include maintenance/cloud/429. |
| infra/event-sync/src/main/java/com/simprints/infra/eventsync/sync/master/EventEndSyncReporterWorker.kt | Avoids storing “last successful sync time” when any worker output indicates failure. |
| infra/event-sync/src/main/java/com/simprints/infra/eventsync/sync/EventSyncStateProcessor.kt | Maps SUCCEEDED work into FAILED state when output failure flags exist. |
| infra/event-sync/src/main/java/com/simprints/infra/eventsync/sync/common/WorkInfo.ext.kt | Adds hasAnyFailureReason() helper based on worker output flags. |
| infra/event-sync/src/test/java/com/simprints/infra/eventsync/sync/up/workers/EventUpSyncUploaderWorkerTest.kt | Updates expectations to success + flags and adds coverage for 429. |
| infra/event-sync/src/test/java/com/simprints/infra/eventsync/sync/up/tasks/EventUpSyncTaskTest.kt | Adds tests asserting terminal exceptions are rethrown. |
| infra/event-sync/src/test/java/com/simprints/infra/eventsync/sync/down/workers/SimprintsEventDownSyncDownloaderWorkerTest.kt | Updates expectations to success + flags for down-sync worker. |
| infra/event-sync/src/test/java/com/simprints/infra/eventsync/sync/down/workers/CommCareEventSyncDownloaderWorkerTest.kt | Updates expectations to success + flags and adds “no project” relogin-flag case. |
| infra/event-sync/src/test/java/com/simprints/infra/eventsync/sync/down/tasks/SimprintsEventDownSyncTaskTest.kt | Adds tests for rethrowing terminal exceptions + request-event logging on cloud integration failure. |
| infra/event-sync/src/test/java/com/simprints/infra/eventsync/sync/master/EventEndSyncReporterWorkerTest.kt | Adds test ensuring last-success time is not stored when failure flags are present. |
| infra/event-sync/src/test/java/com/simprints/infra/eventsync/sync/EventSyncStateProcessorTest.kt | Adds tests covering “SUCCEEDED but flagged failure” mapping for multiple failure types. |
| infra/event-sync/src/test/java/com/simprints/infra/eventsync/sync/SubjectsSyncStateProcessorTestHelper.kt | Adds helpers for new flagged-failure scenarios and allows injecting output into WorkInfo builders. |
| infra/event-sync/src/test/java/com/simprints/infra/eventsync/sync/down/tasks/EnrolmentRecordFactoryTest.kt | Adjusts test setup/cleanup (but currently risks breaking deterministic UUID-based assertions). |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
e90d4a7 to
1b364ab
Compare
@luhmirin-s for the down-sync it's easy, you just need to down-sync more than 10 different modules in a 24hours window it would fail when trying to down-sync the 11th module |
1b364ab to
8ac9a23
Compare
|



JIRA ticket
Will be released in: 2026.3.0
Root cause analysis (for bugfixes only)
First known affected version: 2026.3.0 (likely earlier ones as well)
WorkInforeporting failed state for the either up or down sync worker.Notable changes
Testing guidance
Additional work checklist