Skip to content

fix: [SDK-5481] retry failed operations within the same run - #1775

Merged
fadi-george merged 11 commits into
mainfrom
fadi/sdk-5481
Oct 14, 2026
Merged

fadi-george merged 11 commits into
mainfrom
fadi/sdk-5481

Conversation

@fadi-george

@fadi-george fadi-george commented Oct 12, 2026 •

Copy link
Copy Markdown
Collaborator

Description

One Line Summary

Executors retry a retryable failure in the same run, with backoff, instead of waiting for the next launch.

Details

Motivation

After a retryable failure (0, 5xx, 429 and so on), most executors kept the request with sentToClient still set, so it was only resent after a relaunch. The User executor was worse: its queue is ordered, so a stuck Create User or Fetch User blocked every user request behind it for the rest of the run. Update Subscription went the other way and retried on every 5s poll with no backoff.

This is the shared fix discussed on the sessions PR (#1768), so sessions can be an ordinary executor instead of carrying their own retry code. The same root cause is behind SDK-5468 (Live Activities), though that executor is separate and not changed here.

Scope

  • OSRequestRetryBackoff (OSCore): per-request backoff, 5s doubling to 300s with jitter. Retry-After is the minimum wait, and a 429 without one waits 60s. State is in memory and weakly keyed, so after a relaunch cached requests are sent right away as before.
  • Every executor (User, Identity, Property, Subscription, Custom Events) now backs off on a retryable failure and resends in the same run. Repo executors are picked up by the existing 5s poll. The User executor isn't a repo executor, so the same poll calls into it once its wait is over, and it keeps holding the requests behind the failed one, so order is preserved. A Create User the server rejects stays held until the next session (or is dropped once that user logs out), and an Update Subscription that replaces a waiting one keeps its wait.
  • OSOperationRetryTrigger (OSCore) ends the backoff early on foreground, and when the network returns while in the foreground (NWPathMonitor). Retry-After is still honored. OneSignalTracker drives it through OneSignalUserManagerImpl.onFocus / onUnfocus.
  • Not changed: non-retryable handling (missing, conflict, unauthorized, Identity Verification), and OneSignalClient's own short reattempts, which still run first.
  • Retryable requests still never give up, as before. They now retry at most every 5 minutes instead of waiting for the next launch.

Testing

Unit testing

  • OSRequestRetryBackoffTests: delay growth and cap, jitter, Retry-After and the 429 default, retryNow ending the backoff but not Retry-After.
  • OSOperationRetryTriggerTests: foreground, network return only while foregrounded, monitor started once.
  • ExecutorRetryBackoffTests: a failed request is resent once the backoff passes and removed on success; non-retryable failures are not retried; retryNow and Retry-After; the User executor retries a failed Fetch User from the poll once the backoff passes; a rejected Create User is only resent on a new session, is dropped once its user logs out, and is resent right away if a session started while it was in flight.
  • Full suite passes, except UserConcurrencyTests.testSubscriptionExecutorConcurrency, which also fails on main.

Manual testing

Demo app on an iOS 26.5 simulator, using the retry harness from #1776: mitmproxy fails only SDK requests, Maestro flows drive the demo, and report checks each retry gap against the expected window. Its README has setup and the scenarios.

  • Create User failing (408): retried in the same run, gaps inside the backoff window, then 201. Foregrounding mid-backoff sent the retry right away.
  • Repo executor (tag update failing 3 times): 5.3, 10.7, 16.2s, then 202.
  • Retry-After 20s: waited 21.0s and 21.5s, and background then foreground did not cut it short.
  • 429 without Retry-After: 64s.
  • 503: client resends at 5, 15, 45, 135s, then executor backoff, then 202.
  • 400: sent once, not retried.
  • Update Subscription: gaps grew 5, 11, 16, 27s instead of repeating every 5s.
  • Login while Create User fails: Identify User waited for Create User to succeed.
  • Fetch User failing after login: retried in the same run, then 200.
  • Login, logout, login while everything fails: drained in order once cleared, no errors.
  • Kill mid-backoff: the cached request was sent 1.5s after relaunch.
  • Offline (connections dropped) while adding a tag, alias and email: everything went out once back online.

Identity Verification, on an app that requires it (no anonymous Create User is sent):

  • Login's Create User failing (408): 4.0, 10.7, 21.3, 32.4s, then 201, then Fetch User by external_id.
  • Tag update failing 3 times on the external_id route: 5.3, 5.3, 16.2s, then 202.
  • Offline while adding a tag, an alias and an email claimed in the JWT: all went out once back online.
  • Tag added while the login's Create User was failing: sent only after the 201 and Fetch User.
  • Two 408s, then a token from the wrong key: 401, the request was held with nothing sent for about 2 minutes, then a valid token through updateUserJwt sent it (201).

Not run: network return via NWPathMonitor (drops the Mac's Wi-Fi).

Call outs:

  • For status 0 and 5xx, OneSignalClient resends on its own 5/15/45/135s schedule before the executor sees the failure, and foreground and network return do not shorten it. In the offline run, requests went out on their client resend times, not on foreground. So after airplane mode ends, a request can still wait up to 135s. Pre-existing, but it limits how much the new triggers help for offline.
  • At cold start the first Create User often fails just before the app becomes active, so the foreground trigger sends the second attempt within a second. Expected.
  • The demo's notification prompt keeps the app inactive, so foreground retries do not fire while it is showing.

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

Executors kept a request after a retryable failure but only resent it on the next launch. They now
back off exponentially with jitter (5s to 300s, honoring Retry-After) and resend in the same run.
Foreground and network return end the backoff early.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

@cursor cursor Bot 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.

Multi-model review (Claude Opus 5.5, GPT 5.6 Sol, Grok 4.7) of SDK-5481 same-run executor retry.

Act on

  • (3/3) OSRequestRetryBackoff.retryAfterSeconds accepts non-finite and uncapped values (inf, 1e400, multi-day seconds). The User executor passes that delay to asyncAfter, which can trap converting to nanoseconds; a huge finite wait then holds the ordered user queue for the rest of the run, and retryNow will not override Retry-After. Accept only finite, non-negative delay-seconds and clamp (e.g. maxDelaySeconds or 1h).
  • (Grok; others missed) A non-retryable Create User (400/409/…) stays at the head with sentToClient = false and no backoff entry. retryNow() always calls executePendingRequests(), so every didBecomeActive and every foreground network return resends it. The comment still says this waits for a new session.

Consider

  • (Opus) Tracker maps willResignActive → onUnfocus and didBecomeActive → retryNow, so Control Center / notification shade / Face ID blips cancel exponential backoff (except Retry-After) and flush the operation repo even when nothing is waiting.
  • (2/3 Opus+Grok) Repo-polled executors skip only the waiting request. A later snapshot can succeed first; the older OSRequestUpdateProperties / alias / Create Subscription body is then sent after the backoff. Same class of race as a relaunch, now inside minutes. Update Subscription is safer because it refreshes from the live model.
  • (Opus) A retryable Fetch User for a user who is no longer current stays at the head and blocks a later login until it succeeds or the process dies.
  • (Sol) systemUptime freezes in sleep, so a Retry-After started before lock can still block after hours of wall time.

Noted

  • OneSignalClient still burns 5/15/45/135s on 0/5xx before this layer sees onFailure, so the new trigger cannot speed the first attempt (documented).
  • HTTP-date Retry-After is intentionally unsupported and becomes 60s.
  • Tests cover Identity Add Aliases + one Fetch User path; Create/Identify, onFocus→retryNow, and path-monitor transitions are untested.

Dismissed

  • NSMapTable weak-key zeroing during retryNow enumeration as a likely crash: requests stay strongly held by the executor queues until removed.
Open in Web View Automation 

Sent by Cursor Automation: PR Reviews

Comment thread iOS_SDK/OneSignalSDK/OneSignalUser/Source/OneSignalUserManagerImpl.swift Outdated
fadi-george and others added 2 commits October 12, 2026 13:11
Keeps OneSignalUserManagerImpl.swift under SwiftLint's 1000 line limit.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Retry-After must be a finite, non-negative delay and is capped at an hour. The foreground and
network triggers only wake the executors when a backoff actually ended, so held work such as a
paused Create User is not resent on every foreground. Backoff now uses a clock that counts sleep.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@fadi-george

Copy link
Copy Markdown
Collaborator Author

Thanks. On the Consider items:

  • Resign-active blips: they only end a backoff now if something is actually waiting, and Retry-After still holds.
  • Repo executors skipping only the waiting request: same ordering as before, when it was resent at relaunch. Holding the queue behind it would be a bigger behavior change, so I'm leaving it for a follow-up if we see problems.
  • Fetch User for a non-current user at the head: before this it blocked until relaunch, now it retries with backoff, so it's strictly better. Dropping stale fetches can be a separate change.
  • Sleep: switched the backoff clock to CLOCK_MONOTONIC, which keeps counting during sleep.

fadi-george and others added 4 commits October 12, 2026 13:48
Fixed sleeps were too short on CI, and a late response leaked into the next test.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

@abdulraqeeb33 abdulraqeeb33 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Requesting changes.

This adds a second retry path next to OneSignalClient (5, 15, 45, 135s for status 0 and 5xx) and the existing 5s operation-repo poll. Clear sentToClient on a retryable failure and re-enter the user executor from that poll. Keep the delay in willReattemptRequest.

  1. The user executor timer is mach_absolute_time. retryAt is CLOCK_MONOTONIC. After sleep past retryAt, retryNow does not flush, and the ordered queue waits out the leftover awake time.
  2. An Update Subscription left with sentToClient false is removed by the last-wins coalescer, and the replacement is sent with no wait.
  3. A non-retryable Create User has sentToClient false and no backoff entry, so a foreground that shortens some other request resends it in the same run.

Comment thread iOS_SDK/OneSignalSDK/OneSignalUser/Source/Executors/OSUserExecutor.swift Outdated
Comment thread iOS_SDK/OneSignalSDK/OneSignalUser/Source/Executors/OSUserExecutor.swift Outdated
…aits

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

@abdulraqeeb33 abdulraqeeb33 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Requesting changes.

The timer, the sleep stall, and the Update Subscription wait drop are fixed. The user executor now retries from the poll, and keeping the backoff gate is right: 408 and 429 never enter OneSignalClient's ladder, so a bare 5s poll would ignore Retry-After.

  1. Logout leaves a rejected Create User at the head. The next user's Create User sits behind it and nothing sends until a later session, which sends the logged-out user's request first.
  2. A session that starts while that Create User is still in flight drains an empty list. The 4xx then parks the request and pauses the repo again.

Comment thread iOS_SDK/OneSignalSDK/OneSignalUser/Source/Executors/OSUserExecutor.swift Outdated
…ease one rejected mid-session

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

@abdulraqeeb33 abdulraqeeb33 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Requesting changes.

A rejected Create User for someone who is no longer current is dropped on the next flush, and a 4xx that lands after a new session started is sent again immediately.

Logout or login while that Create User is still in flight is not covered. The session count has not changed, so the 4xx is appended and the block does not re-enter the queue. The new user's Create User stays behind the old one, and the repo stays paused, until some later wake.

…te User

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@fadi-george
fadi-george added this pull request to stack #1777 October 14, 2026 22:22

@nan-li nan-li left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks, I think it is reasonable, so approving. I noticed some of the review rounds got stuck on failed Create User which is inherently a problem that can’t really tackled in this PR, and it was just peeling layers of the onion and exposing the next event that would happen once one thing is addressed, which is not a blocker for me.

@fadi-george

Copy link
Copy Markdown
Collaborator Author

Thanks @nan-li , I filed SDK-5541

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants