Fix pull sync reliability: fail on partial page errors and cap transient retries

[Tomeshwari-02]

Summary

This PR fixes a pull-sync reliability gap where per-page failures could be masked, causing the worker to report success for incomplete syncs.

Problem

PullFromAmritWorker currently launches page pulls in parallel. In getBenForPage(...), SQLiteConstraintException was caught and then the function still returned true. This can allow overall success even when one page thread failed to persist data.

Additionally, when a page repeatedly returns transient -2, the loop can continue indefinitely for that page thread.

Changes

1) Fail fast on per-thread persistence/errors

In getBenForPage(...), the worker now returns false when:

• SQLiteConstraintException occurs,
• a hard pull error (ret == -1) is received,
• any unexpected exception is thrown.

This ensures awaitAll(...) results reflect real per-thread failure, and doWork() can fail instead of silently succeeding.

2) Bound transient retries per page

Added bounded retry logic for transient -2 responses per page:

• MAX_TRANSIENT_RETRIES_PER_PAGE = 5
• TRANSIENT_RETRY_DELAY_MS = 500

If transient retries are exhausted for a page thread, it returns false and the overall pull fails.

File changed

• app/src/main/java/org/piramalswasthya/sakhi/work/PullFromAmritWorker.kt

Impact

• Prevents false-positive pull success on partial data failures.
• Avoids indefinite looping on repeated transient page failures.
• Improves sync integrity and operational trust in completion status.

Risk

Low functional risk: behavior only changes for error paths. Successful pull flow remains unchanged.

Summary by CodeRabbit

• Bug Fixes
  • Improved resilience of beneficiary data synchronization with automatic retry logic for transient network errors
  • Enhanced error handling to prevent application crashes from unexpected failures

[coderabbitai]

📝 Walkthrough

Walkthrough

The PullFromAmritWorker class now includes enhanced error handling for getBenForPage method. Transient errors (-2 return value) trigger automatic retries with delays, hard errors (-1) are logged without exceptions, and uncaught exceptions are caught and logged gracefully.

Changes

Cohort / File(s)	Summary
Error Handling & Retry Logic app/src/main/java/org/piramalswasthya/sakhi/work/PullFromAmritWorker.kt	Added transient-error retry mechanism with configurable attempts and delays; converted hard-error exceptions to logged returns; added catch-all exception handler to prevent unhandled exceptions. Imports delay function and fixes trailing newline.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

🐰 A worker so sturdy, now bounces with grace,
When transient troubles appear in the race.
With retries and patience, delays in between,
The most resilient worker you've ever seen! ✨
No exception escapes—all are caught and logged fair,
A robust little system, beyond all compare! 🌟

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately summarizes the main changes: it highlights the two key improvements (failing on partial page errors and capping transient retries) that directly address the reliability problems described in the PR objectives.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

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

• X
• Mastodon
• Reddit
• LinkedIn

_{Review rate limit: 0/1 reviews remaining, refill in 60 minutes.}

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

[sonarqubecloud]

Quality Gate passed

Issues
1 New issue
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@app/src/main/java/org/piramalswasthya/sakhi/work/PullFromAmritWorker.kt`:
- Around line 160-163: In getBenForPage, the catch block currently swallows
CancellationException by catching Exception; update the handler to rethrow
CancellationException (check if e is kotlinx.coroutines.CancellationException or
java.util.concurrent.CancellationException and rethrow) and only log/return
false for non-cancellation exceptions (keeping the Timber.e logging for other
errors) so worker cancellation is propagated correctly.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 1cb155e3-a931-4339-8ab6-527e2661231f

📥 Commits

Reviewing files that changed from the base of the PR and between 6df8b9b and 1d5eef1.

📒 Files selected for processing (1)

• app/src/main/java/org/piramalswasthya/sakhi/work/PullFromAmritWorker.kt

[coderabbitai]

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

🏁 Script executed:

cat -n "app/src/main/java/org/piramalswasthya/sakhi/work/PullFromAmritWorker.kt" | sed -n '150,170p'

Repository: PSMRI/FLW-Mobile-App

Length of output: 818

---

🏁 Script executed:

rg "getBenForPage" "app/src/main/java/org/piramalswasthya/sakhi/work/PullFromAmritWorker.kt" -B 5 -A 20

Repository: PSMRI/FLW-Mobile-App

Length of output: 2739

---

🏁 Script executed:

head -30 "app/src/main/java/org/piramalswasthya/sakhi/work/PullFromAmritWorker.kt"

Repository: PSMRI/FLW-Mobile-App

Length of output: 1149

---

🌐 Web query:

Kotlin CancellationException class hierarchy inheritance Exception

💡 Result:

Kotlin's CancellationException is defined in the Kotlin standard library (kotlin.coroutines.cancellation package) as an expect open class that extends IllegalStateException. Platform-specific actual implementations vary: - JVM/Android: typealias to java.util.concurrent.CancellationException, which extends IllegalStateException (via RuntimeException). - JS/Wasm/Native: actual open class CancellationException : IllegalStateException. Subclasses in kotlinx.coroutines include: - JobCancellationException extends CancellationException (internal, used for coroutine job cancellation). - TimeoutCancellationException extends CancellationException (thrown by withTimeout). - AbortFlowException extends CancellationException (internal for flows). This hierarchy enables coroutine cancellation signaling while maintaining compatibility with Java's concurrent API on JVM. CancellationException instances thrown during coroutine cancellation are handled specially (not propagated as errors, must be rethrown if caught).

Citations:

• 1: https://kotlinlang.org/api/core/kotlin-stdlib/kotlin.coroutines.cancellation/-cancellation-exception/-cancellation-exception.html
• 2: https://github.com/Kotlin/kotlinx.coroutines/blob/c686eebd996e48fe65c218a474ea80fca8ae3a92/kotlinx-coroutines-core/jvm/src/Exceptions.kt
• 3: https://github.com/kotlin/kotlinx.coroutines/tree/master/kotlinx-coroutines-core/jsAndWasmShared/src/Exceptions.kt
• 4: https://kotlinlang.org/api/core/kotlin-stdlib/kotlin.coroutines.cancellation/-cancellation-exception/
• 5: CancellationException inheriting from IllegalStateException can be error prone Kotlin/kotlinx.coroutines#4073
• 6: https://kotlinlang.org/api/kotlinx.coroutines/kotlinx-coroutines-core/kotlinx.coroutines/-timeout-cancellation-exception
• 7: https://github.com/Kotlin/kotlinx.coroutines/blob/master/kotlinx-coroutines-core/jvm/src/flow/internal/FlowExceptions.kt

---

Re-throw CancellationException in getBenForPage to prevent swallowing worker cancellation.

catch (e: Exception) catches CancellationException due to the exception hierarchy, converting coroutine cancellation into a normal failure flow. When CancellationException is caught, it must be rethrown to properly signal worker cancellation; otherwise, the worker's cancellation mechanism is subverted.

Suggested patch

+import kotlinx.coroutines.CancellationException
 import kotlinx.coroutines.Dispatchers
 import kotlinx.coroutines.async
 import kotlinx.coroutines.awaitAll
 import kotlinx.coroutines.delay
 import kotlinx.coroutines.withContext
@@
-            } catch (e: Exception) {
+            } catch (e: CancellationException) {
+                throw e
+            } catch (e: Exception) {
                 Timber.e("exception $e raised ${e.message} with stacktrace : ${e.stackTrace}")
                 return@withContext false
             }

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change

	} catch (e: Exception) {
	Timber.e("exception $e raised ${e.message} with stacktrace : ${e.stackTrace}")
	return@withContext false
	}
	} catch (e: CancellationException) {
	throw e
	} catch (e: Exception) {
	Timber.e("exception $e raised ${e.message} with stacktrace : ${e.stackTrace}")
	return@withContext false
	}

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@app/src/main/java/org/piramalswasthya/sakhi/work/PullFromAmritWorker.kt`
around lines 160 - 163, In getBenForPage, the catch block currently swallows
CancellationException by catching Exception; update the handler to rethrow
CancellationException (check if e is kotlinx.coroutines.CancellationException or
java.util.concurrent.CancellationException and rethrow) and only log/return
false for non-cancellation exceptions (keeping the Timber.e logging for other
errors) so worker cancellation is propagated correctly.