[MBL-19602][S/T/P] Revision of API rate limit exceeded retrial logic

[suhaibabsi-inst]

refs: MBL-19602
affects: Student, Teacher, Parent
builds: Student, Teacher, Parent
release note: None.

Background

The motive for this was created by the work on solving the issue of Course sync for this account:
https://instructure.atlassian.net/browse/MBL-19512

Multiple parallel requests seems to consistently fail with 429 Too Many Requests (Rate Limit Exceeded) error. Thus the need to rethink how retrial is being done just to leave a considerable time window between those to avoid such failures.

Approach

As per this document was suggesting, headers (x-request-cost & x-rate-limit-remaining) of URLResponse were examined for failed requests, to infer some logic on deciding the right time to wait until perform the next request but turned out we can't. The reason for that is that x-request-cost only received for successful requests, with value of x-rate-limit-remaining that doesn't imply a possible issue along the road. Examples (cost: 0.09859950499787828, limit: 700). And this is expected since we are dealing with failures resulted by multiple costly requests happening at the same time.

Therefore, so to break this unwanted parallelism without carrying on a massive refactor, I went back on applying the solution suggested here by @vargaat :
#3798 (review)

Test Plan

Use account mentioned in this ticket:
https://instructure.atlassian.net/browse/MBL-19512

To trigger course sync for offline mode, the sync should at least work for page contents with no issues. There still could be issues with media sync, but this to be resolved by other PR.

Also, the usual use cases of app (navigating modules, pages, announcements, ...etc, navigating assignments, submission, sending a message) should work with no issues.

Checklist

• Follow-up e2e test ticket created
• A11y checked
• Tested on phone
• Tested on tablet
• Tested in dark mode
• Tested in light mode
• Approve from product

[claude]

Claude Code Review

Updated: 2025-12-14

✅ Approved

• Properly extends rate limit detection to include 429 status code alongside 403
• Implements exponential backoff with jitter (1s + 0-500ms random) to reduce thundering herd
• retrialTimeOnRateLimitExceeded() correctly returns optional DispatchTime only when rate limit exceeded
• Comprehensive test coverage: validates both 403/429 scenarios and confirms randomness across retries
• No crashes, breaking changes, or correctness issues identified

[inst-danger]

Builds

Commit: Update APITests.swift (a521519)
Build Number: 1002
Built At: Dec 14 20:15 CET (12/14 12:15 PM MST)

Student

Teacher

Parent

[inst-danger]

	Warnings
⚠️	One or more files are below the minimum test coverage 50%

Release Note:

None.

Affected Apps: Student, Teacher, Parent

Builds: Student, Teacher, Parent

MBL-19602

Coverage	New %	Master %	Delta
Canvas iOS	91.09%	81.06%	10.03%
Horizon/Horizon/Sources/Common/View/CourseDetailsAccessibility.swift	0%	--	--

Generated by 🚫 dangerJS against a521519

[claude]

Claude Code Review

Updated: 2025-12-14

• ✅ Correctly adds support for both 403 and 429 rate limit responses; detects via status code + message pattern
• ✅ Implements jitter (0-500ms random delay) with fixed 1s base, reducing collision probability on retries
• ✅ New retrialTimeOnRateLimitExceeded() method returns DispatchTime directly—correct pattern for GCD scheduling
• ✅ Tests cover both status codes and verify randomness of jitter (all retrial times differ)
• ✅ Backward compatible: existing 403 handling unchanged; 429 detection is additive
• ✅ No performance regressions; delay logic is only triggered on rate limit errors

✅ Approved

[claude]

Claude Code Review

Updated: $(date)

Summary: Rate limit retry logic improvements with randomized backoff and 429 support.

• ✅ Correct implementation: Adds random jitter (0-500ms) to 1s backoff, reducing thundering herd on rate limit recovery
• ✅ Protocol support: Now handles both 403 and 429 rate limit responses correctly
• ✅ Comprehensive tests: New test cases verify both error codes, randomization, and retry behavior
• ✅ No breaking changes: Refactors existing behavior without API modifications
• ✅ Test cleanup: Properly initializes LoginSession in setUp() to prevent state leakage

No critical issues found.

✅

[petkybenedek]

Code +1

[vargaat]

QA+1, http 429 errors were properly re-tried and the sync successfully finished.

[rh12]

Code + 1