Skip to content

MS-1538 Sync worker error propagation fix - #1783

Open
luhmirin-s wants to merge 5 commits into
mainfrom
feature/MS-1538-sync-worker-errors
Open

MS-1538 Sync worker error propagation fix#1783
luhmirin-s wants to merge 5 commits into
mainfrom
feature/MS-1538-sync-worker-errors

Conversation

@luhmirin-s

Copy link
Copy Markdown
Contributor

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)

  • Sync errors in UI were depending on the WorkInfo reporting failed state for the either up or down sync worker.
  • When we added sync event scopes, it became mandatory for sync end report worker to execute even if individual worker failed. Therefore, all up and down sync worker did not report failures anymore.
  • This resulted in UI that does report any errors.

Notable changes

  • Reviewed which errors are thrown from the sync tasks to the workers.
  • Explicitly adding the error information to worker data while always reporting "success".
  • For state resolution we are now checking the error flags when the worker reported success.

Testing guidance

  • It is pretty hard to reproduce this behaviour manually. I have manually added the exception throwing inside the task to validate that correct UI is displayed while the workers are still executed correctly.
  • Open to ideas how to break sync without manipulating the code.

Additional work checklist

  • Effect on other features and security has been considered
  • Design document marked as "In development" (if applicable)
  • External (Gitbook) and internal (Confluence) Documentation is up to date (or ticket created)
  • Test cases in Testiny are up to date (or ticket created)
  • Other teams notified about the changes (if applicable)

@luhmirin-s
luhmirin-s requested review from a team, BurningAXE, TristramN, alex-vt, alexandr-simprints, meladRaouf and ybourgery and a lite review from Copilot and removed request for a team August 12, 2026 12:08
@cla-bot cla-bot Bot added the ... label Aug 12, 2026
@luhmirin-s luhmirin-s changed the title Feature/ms 1538 sync worker errors MS-1538 Sync worker errors Aug 12, 2026
@luhmirin-s luhmirin-s changed the title MS-1538 Sync worker errors MS-1538 Sync worker error propagation fix Aug 12, 2026

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 SUCCEEDED worker 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.

@sonarqubecloud

Copy link
Copy Markdown

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants