[fadi-george]

Description

One Line Summary

A retryable failure now backs off on its own in OperationRepo instead of stalling the whole queue.

Details

Motivation

On FAIL_RETRY, executeOperations re-queued the op and then called delayBeforeNextExecution, which suspended the whole loop. One failing op (for example a 5xx on a tag update) held up every other op, including ones for unrelated records, for retries * 15s or Retry-After. This is the Android counterpart to iOS #1775 (SDK-5481), and lets sessions go through OperationRepo as ordinary operations later. No sessions code here.

Scope

Matches iOS #1775 unless noted.

• Per-op backoff: on FAIL_RETRY each queue item waits opRepoDefaultFailRetryBackoff (5s), doubling per failure up to opRepoMaxFailRetryBackoff (300s), times 0.5-1 jitter. The op also waits out any server Retry-After, capped by maxRetryAfter (1h); 60s if a 429 has none or it can't be parsed. The global delayBeforeNextExecution / retryWaiter stall is removed.
• getNextOps skips items that are still waiting. It also skips any op on the same applyToRecordId as an earlier waiting op, so a create still runs before its updates and two updates to the same user keep their order. getGroupableOperations follows the same rule, so a waiting op is never pulled into another op's request.
• When nothing is ready, the loop sleeps until the earliest retry time instead of waiting only for a new enqueue.
• forceExecuteOperations ends an op's wait and skips the 5s batching wait only when it actually ended one. Retry-After still holds, since HttpClient keeps the request until it passes. Android only: a force that arrives during execute is kept, so the failed op skips its next backoff.
• New OperationRetryTrigger forces on foreground, and when the device goes from offline to online while in the foreground. Connectivity is watched from the first focus, through an injectable IConnectivityMonitor; networks already up at registration and Wi-Fi/mobile switches don't count.
• Retry-After parsing accepts fractions (rounded up) and falls back to 60s on negative values.
• Android only: HttpClient keeps main's app-wide Retry-After gate (iOS has none), which suspends requests until the window ends, so IAM, receipts and outcomes behave as before. It is now on elapsedRealtime and capped by maxRetryAfter. During a Retry-After window the queue waits too, as on main; only plain backoff no longer blocks it. The login executors now pass Retry-After through.
• Deadlines use the new ITime.elapsedRealtimeMillis, so device clock changes don't affect them. Each retry logs its backoff and Retry-After at debug level.
• Not changed: FAIL_PAUSE_OPREPO, non-retryable handling, Identity Verification (FAIL_UNAUTHORIZED, shouldSuppressAnonymousOp, removeOperationsWithoutExternalId), post-create delay, bucketing. Backoff state is in memory, so after a relaunch queued ops are sent right away as before.
• Behavior changes: the first retry comes after 2.5-5s instead of 15s, and later ones grow faster but stop at 300s. forceExecuteOperations (consent, JWT update, foreground, network) no longer cuts Retry-After short.

OperationRepo lines changed

OperationRepo.kt: +89 / -40, for comparison with the +300 session-only lines in the #2777 stack. Plus OperationRetryTrigger, ConnectivityMonitor, and small changes in HttpClient, ITime, ConfigModel, CoreModule and the two login executors.

Testing

Unit testing

OperationRepoTests:

• a retrying op does not block ops on other records
• an op on the same record waits behind a retrying op and runs after it, in order
• a waiting op is not grouped into another op's request
• backoff doubles from the base up to the cap, scaled by jitter
• forceExecuteOperations ends backoff, and skips the batching wait when it ended one
• a force during execute skips the next backoff
• existing retry tests updated now that delayBeforeNextExecution is gone

HttpClientTests: the next request waits out a Retry-After on the monotonic clock; the wait is capped by maxRetryAfter; fractional Retry-After rounds up, negative falls back to 60s.

ConnectivityMonitorTests: networks up at registration and network switches don't count as coming online; offline to online does, every time; a registration failure doesn't throw.

OperationRetryTriggerTests: foreground forces a retry, focus on subscribe does not; connectivity is watched from the first focus, once; coming online forces a retry only in the foreground.

./gradlew spotlessCheck detekt testDebugUnitTest apiCheck assembleRelease passes. core.api is re-dumped to fix drift already on main (ErrorCode, LoginData, ...), with the same content as #2787. This PR adds no public API.

Manual testing

Android 16 emulator, using the harness in #2791, on an earlier revision of this PR (linear 15s backoff):

• Create User failing (408): retried after 15.0s and 30.3s, then 201.
• With a tag update failing, a push toggle PATCH went out between tag attempts instead of waiting.
• Foreground and a 5s network flap each ended a backoff early. Retry-After 40s was honored through a foreground.
• Login during Create User failures: Identify User was sent only after Create User succeeded.
• A tag update got 429 with Retry-After 90s and retried at 90.2s.

The exponential backoff and the foreground-only network wake are covered by unit tests; I haven't re-run the harness against them.

Affected code checklist

• Notifications
  • Display
  • Open
  • Push Processing
  • Confirm Deliveries
• Outcomes
• Sessions
• In-App Messaging
• REST API requests
• Public API changes

Checklist

Overview

• I have filled out all REQUIRED sections above
• PR does one thing
• Any Public API changes are explained in the PR details and conform to existing APIs

Testing

• I have included test coverage for these changes, or explained why they are not needed
• All automated tests pass, or I explained why that is not possible
• I have personally tested this on my device, or explained why that is not possible

Final pass

• Code is as readable as possible.
• I have reviewed this PR myself, ensuring it meets each checklist item

🤖 Generated with Claude Code

[github-actions]

📊 Diff Coverage Report

Diff Coverage Report (Changed Lines Only)

Gate: aggregate coverage on changed executable lines must be ≥ 80% (JaCoCo line data for lines touched in the diff).

Changed Files Coverage

• ConfigModel.kt: 8/8 touched executable lines (100.0%) (23 touched lines in diff)
• ConnectivityMonitor.kt: 32/33 touched executable lines (97.0%) (61 touched lines in diff)
• HttpClient.kt: 6/6 touched executable lines (100.0%) (11 touched lines in diff)
• OperationRepo.kt: 46/46 touched executable lines (100.0%) (86 touched lines in diff)
• OperationRetryTrigger.kt: 14/15 touched executable lines (93.3%) (35 touched lines in diff)
• Time.kt: 1/1 touched executable lines (100.0%) (2 touched lines in diff)
• LoginUserFromSubscriptionOperationExecutor.kt: 0/1 touched executable lines (0.0%) (1 touched lines in diff)
  • 1 uncovered touched lines in this file
• LoginUserOperationExecutor.kt: 1/1 touched executable lines (100.0%) (1 touched lines in diff)
• ⚠️ MockHelper.kt: Not in coverage report (may not be compiled/tested)

Overall (aggregate gate)

108/111 touched executable lines covered (97.3% — requires ≥ 80%)

Per-file detail (informational; gate is aggregate above):

• LoginUserFromSubscriptionOperationExecutor.kt: 0.0% (1 uncovered touched lines)

📥 View workflow run

[cursor]

Multi-model review

Claude Opus 5.5, GPT 5.6 Sol, Grok 4.7 on the 351-line Kotlin change.

Per-op backoff for 5xx without Retry-After looks right, and same-record ordering is tested. Two gaps undermine the “force immediately / don’t stall unrelated ops” claims.

Act on

forceExecuteOperations does not force. It wakes with LoopWaiterMessage(false), so waitForNewOperationAndExecutionInterval still waits a full opRepoExecutionInterval (5s). The retry-timeout path already uses force=true. The “immediately wakes retry waiters” test passes only because MockHelper sets the interval to 1ms. Wake with true.

Consider

• Retry-After still stalls the loop (3/3). HttpClient.delayNewRequestsUntil is process-wide and uncapped. After a 429, the next unrelated op sleeps on the single repo thread, so per-op retryAfterUntil isolation (and the 1h cap) do not hold for that path. Pre-existing, but it is the case this PR special-cases.
• Network callback is noisy (Opus + Grok). onAvailable fires at registration and on every matching network, each zeroing every backoffUntil. Prefer a validated/default network and ignore the registration snapshot.
• Wall-clock deadlines (Opus + Sol). retryAfterUntil is set to now even when there is no Retry-After. A clock rollback can strand an op past forceExecuteOperations.
• In-flight force is dropped (Sol). A focus/network event during execute only clears queued items; a following FAIL_RETRY installs a fresh backoff. The old retryWaiter buffered the force.
• Login hold can retarget (Grok + Opus). LoginUserOperation.applyToRecordId is existingOnesignalId ?: onesignalId; a merge while waiting changes which later ops are blocked.

Noted

Rebuild ops returned with FAIL_RETRY now run immediately. Retry-After on non-retry results is no longer applied at the repo layer. No e2e test that forceExecuteOperations still honors Retry-After through the loop.

  

_{Sent by Cursor Automation: PR Reviews}

[abdulraqeeb33]

Two things before this lands, since it should not change architecture:

1. Removing delayNewRequestsUntil makes each caller handle Retry-After, but several don't. LoginUserFromSubscriptionOperationExecutor (92-95) and LoginUserOperationExecutor (163) drop retryAfterSeconds. ConfigModelStoreListener params retry and sendSavedOutcomes ignore it. Either move all of them over, or keep the shared gate and scope it.
2. OperationRetryTrigger adds the SDK's first network callback. Is it needed for this fix? If yes: skip the registration callback, force only when connectivity goes from offline to online, and inject the connectivity source.

Also:

• Store deadlines on a monotonic clock (elapsedRealtime, as in DeviceGestureDetector).
• Keep a force that lands during execute so it isn't overwritten by a new backoff.
• Move MAX_RETRY_DELAY_MS to ConfigModel.
• Add back one debug line per retry with the backoff and Retry-After values.

[fadi-george]

@abdulraqeeb33 thanks, addressed in b94a1cd and 317e00a.

1. Kept the shared gate and scoped it. It's capped at 1h and returns a 429 with the seconds left instead of delaying, so nothing stalls. Params and outcomes already back off on a 429, and both login executors now pass retryAfterSeconds through.
2. Kept the network callback since iOS has it, but it now only fires on offline to online while in the foreground. Registration and network switches don't count, and the source is injected through IConnectivityMonitor.

Also:

• Deadlines are on elapsedRealtime.
• A force during execute is kept.
• The caps are in ConfigModel.
• Each retry logs its backoff and Retry-After.

I also matched iOS on backoff: 5s doubling to 300s with jitter, Retry-After kept as a minimum a force can't end.

@cursor the in-flight force, wall clock and noisy callback points are covered above. Login retargeting is fine since the hold follows whatever record the login now applies to. Rebuild ops running right away and Retry-After on non-retry results match the old behavior.

[abdulraqeeb33]

Login Retry-After, the monotonic clock, maxRetryDelay, the debug line and dropping the network callback all look good.

One blocker: the fail-fast 429 in HttpClient changes behavior for every caller outside the operation queue. On main they waited, then sent. Now:

1. IAM display retries three times with no delay, then dismisses the message for the session (attemptToShowInAppMessage).
2. Receive receipts are swallowed and the worker returns success.
3. IAM click, impression and page impression are dropped. The IAM fetch falls back without the RYW token.
4. A foreground during Retry-After increments retries on a request that was never sent, which pushes the real send past the window.

Suggest keeping main's suspending gate (now on elapsedRealtime, capped by maxRetryDelay) and using retryAt only for ordinary backoff. Also, CI hasn't run on this head.

[abdulraqeeb33]

My review above was written against 831dd37, before e80c7b2 landed. Updates:

1. The fail-fast 429 points still stand.
2. Force inflating retries is fixed for an op's own Retry-After. It still happens when the gate was set by another request (e.g. an IAM 429), since that op's retryAfterUntil is 0.
3. Ignore "dropping the network callback looks good". It's back as IConnectivityMonitor (offline to online, foreground only), which addresses the earlier noise concerns, but it's still the SDK's first network observer.
4. Exponential backoff with jitter (5s to 300s) changes retry timing for every operation. That needs an explicit sign-off as a policy change, separate from the queue-blocking fix.

[fadi-george]

@abdulraqeeb33 good catch, I missed that the other callers drop a 429. Went back to main's waiting gate in 9f61748, just on elapsedRealtime and capped at an hour, so IAM and receipts behave like before.

Two things I did differently:

• retryAt still includes Retry-After. There's an existing test that expects the repo to respect it, and it saves the op from getting picked up early just to sit in the gate. A force still clears it, and the gate holds the request anyway.
• I kept the network callback since the ticket asks for it and iOS does it. It only fires when coming back online while the app is in the foreground.

[fadi-george]

@abdulraqeeb33 thanks for the update. 1 and 2 should both be covered by 9f61748, since the gate waits again instead of returning a 429. Nothing gets a fake failure now, so retries only go up on real sends, no matter which request set the gate

On 3, yeah it's the first one, but the ticket asks for it and iOS has the same trigger. It's scoped to coming back online in the foreground and fails quietly if registration throws.

On 4, the backoff matches iOS #1775 exactly (5s doubling to 300s with jitter), so splitting it out would leave the two SDKs retrying differently. It's also all in ConfigModel if we want to tune it

[abdulraqeeb33]

Thanks, the suspending gate is back and IAM, receipts and outcomes behave like main again. ConnectivityMonitor is fine given the ticket and iOS parity. Remaining:

1. CI: FAIL_NORETRY on login keeps grouped create-subscription races because startingOp.wakeWaiters runs before the followers are re-queued (OperationRepo.kt:492-495). Same order on main; moving the wake after the synchronized block should fix it.
2. Upgraded installs likely keep a persisted opRepoDefaultFailRetryBackoff = 15000, since the getter writes its default into the model on first read and the config store saves it. Rename the key or migrate it so the 5s base applies.
3. Jitter is drawn per op, so a batched request splits into separate requests on retry. One jitter sample per executeOperations keeps the group together.
4. Nits: qualify "unrelated ops keep running" (not while Retry-After holds the gate), seed ConnectivityMonitor from the callback instead of activeNetworkInfo, consider NET_CAPABILITY_VALIDATED, and trim the OperationRetryTrigger KDoc to 4 lines.

[fadi-george]

@abdulraqeeb33 thanks, good catches.

1. Already fixed in f2d63a3.
2. Good catch, the getter does persist the default. Renamed it to opRepoBaseFailRetryBackoff in 3f90851 so upgrades pick up 5s.
3. Agreed, and retry counts can also differ inside a batch, so jitter wasn't the only thing splitting it. The batch now gets one wait based on its highest retry count, which is what main did.
4. Took the comment and KDoc nits. I'd keep the other two as they are:
   • Seeding from the callback means guessing which onAvailable calls are the initial replay, and there's no clean way to know when that replay is done. activeNetworkInfo gives the current state right at registration.
   • VALIDATED never shows up on networks that block Google's connectivity check (some corporate networks, China), so those devices would never wake. On a captive portal the worst case is one extra failed attempt, and the backoff just keeps growing. iOS doesn't validate either, it uses .satisfied.

[abdulraqeeb33]

Thanks, all four look good. The key rename and single batch wait are what I was after, and the reasoning on activeNetworkInfo and VALIDATED makes sense. The backoff policy matches iOS so I'm fine shipping it together. Approving, please hold the merge until CI is green.