Skip to content

Commit 0ce61d0

Browse files
jaclyncjaclync
authored
[Local Catalog] Add retry logic with exponential backoff to BatchedRequestLoader (#16197)
2 parents 1d4ebce + e679361 commit 0ce61d0

File tree

3 files changed

+256
-5
lines changed

3 files changed

+256
-5
lines changed

‎Modules/Sources/Yosemite/Tools/POS/BatchedRequestLoader.swift‎

Lines changed: 76 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,42 @@
11
import Foundation
2+
import Networking
23

3-
/// Generic utility for loading paginated data with batch processing.
4+
/// Protocol for determining which errors should be retried.
5+
protocol RetryErrorEvaluator {
6+
func shouldRetry(_ error: Error) -> Bool
7+
}
8+
9+
/// Default implementation that retries network errors and server errors.
10+
struct DefaultRetryErrorEvaluator: RetryErrorEvaluator {
11+
func shouldRetry(_ error: Error) -> Bool {
12+
if let networkError = error as? NetworkError {
13+
switch networkError {
14+
case .invalidCookieNonce:
15+
// Don't retry on auth errors that require user action to log in again.
16+
return false
17+
default:
18+
return true
19+
}
20+
}
21+
return true
22+
}
23+
}
24+
25+
/// Generic utility for loading paginated data with batch processing and retry support.
426
final class BatchedRequestLoader {
527
private let batchSize: Int
28+
private let maxRetries: Int
29+
private let retryDelay: TimeInterval
30+
private let errorEvaluator: RetryErrorEvaluator
631

7-
init(batchSize: Int) {
32+
init(batchSize: Int,
33+
maxRetries: Int = 4,
34+
retryDelay: TimeInterval = 2.0,
35+
errorEvaluator: RetryErrorEvaluator = DefaultRetryErrorEvaluator()) {
836
self.batchSize = batchSize
37+
self.maxRetries = maxRetries
38+
self.retryDelay = retryDelay
39+
self.errorEvaluator = errorEvaluator
940
}
1041

1142
/// Loads all items using a paginated request function.
@@ -22,9 +53,15 @@ final class BatchedRequestLoader {
2253

2354
let batchResults = try await withThrowingTaskGroup(of: PageResult<T>.self) { group in
2455
for pageNumber in pagesToFetch {
25-
group.addTask {
26-
let result = try await makeRequest(pageNumber)
27-
return PageResult(pageNumber: pageNumber, items: result)
56+
group.addTask { [maxRetries, retryDelay, errorEvaluator] in
57+
let pagedItems = try await Self.fetchPageWithRetry(
58+
pageNumber: pageNumber,
59+
maxRetries: maxRetries,
60+
retryDelay: retryDelay,
61+
errorEvaluator: errorEvaluator,
62+
makeRequest: makeRequest
63+
)
64+
return PageResult(pageNumber: pageNumber, items: pagedItems)
2865
}
2966
}
3067

@@ -46,6 +83,40 @@ final class BatchedRequestLoader {
4683

4784
return allItems
4885
}
86+
87+
private static func fetchPageWithRetry<T>(
88+
pageNumber: Int,
89+
maxRetries: Int,
90+
retryDelay: TimeInterval,
91+
errorEvaluator: RetryErrorEvaluator,
92+
makeRequest: @escaping (Int) async throws -> PagedItems<T>
93+
) async throws -> PagedItems<T> {
94+
var lastError: Error?
95+
96+
for attempt in 0..<maxRetries {
97+
do {
98+
return try await makeRequest(pageNumber)
99+
} catch {
100+
lastError = error
101+
102+
if !errorEvaluator.shouldRetry(error) {
103+
DDLogError("⛔️ Page \(pageNumber) failed with non-retryable error: \(error)")
104+
throw error
105+
}
106+
107+
// Logs and retries with exponential backoff.
108+
if attempt < maxRetries - 1 {
109+
let delay = retryDelay * pow(2.0, Double(attempt))
110+
DDLogWarn("⚠️ Page \(pageNumber) failed (attempt \(attempt + 1)/\(maxRetries)): \(error). Retrying in \(delay)s...")
111+
try await Task.sleep(nanoseconds: UInt64(delay * Double(NSEC_PER_SEC)))
112+
} else {
113+
DDLogError("⛔️ Page \(pageNumber) failed after \(maxRetries) attempts: \(error)")
114+
}
115+
}
116+
}
117+
118+
throw lastError ?? URLError(.unknown)
119+
}
49120
}
50121

51122
private struct PageResult<T> {

‎Modules/Tests/YosemiteTests/Tools/POS/BatchedRequestLoaderTests.swift‎

Lines changed: 124 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,124 @@
1+
import Foundation
2+
import Testing
3+
@testable import Networking
4+
@testable import Yosemite
5+
6+
struct BatchedRequestLoaderTests {
7+
8+
// MARK: - Basic Functionality Tests
9+
10+
@Test func loadAll_loads_single_page_successfully() async throws {
11+
// Given
12+
let sut = BatchedRequestLoader(batchSize: 2)
13+
let expectedItems = ["item1", "item2", "item3"]
14+
var callCount = 0
15+
16+
let makeRequest: (Int) async throws -> PagedItems<String> = { pageNumber in
17+
callCount += 1
18+
#expect(pageNumber == 1 || pageNumber == 2)
19+
if pageNumber == 1 {
20+
return PagedItems(items: expectedItems, hasMorePages: false, totalItems: 3)
21+
} else {
22+
return PagedItems(items: [], hasMorePages: false, totalItems: 0)
23+
}
24+
}
25+
26+
// When
27+
let result = try await sut.loadAll(makeRequest: makeRequest)
28+
29+
// Then
30+
#expect(result == expectedItems)
31+
#expect(callCount == 2) // batchSize = 2, so it fetches pages 1 and 2
32+
}
33+
34+
// MARK: - Retry Logic Tests
35+
36+
@Test func loadAll_retries_individual_pages_independently() async throws {
37+
// Given
38+
let mockErrorEvaluator = MockRetryErrorEvaluator(shouldRetry: true)
39+
let sut = BatchedRequestLoader(
40+
batchSize: 3,
41+
maxRetries: 3,
42+
retryDelay: 0.01,
43+
errorEvaluator: mockErrorEvaluator
44+
)
45+
46+
var attemptsByPage: [Int: Int] = [:]
47+
let makeRequest: (Int) async throws -> PagedItems<String> = { pageNumber in
48+
attemptsByPage[pageNumber, default: 0] += 1
49+
50+
// Page 2 fails once, then succeeds
51+
if pageNumber == 2 && attemptsByPage[pageNumber] == 1 {
52+
throw URLError(.timedOut)
53+
}
54+
55+
return PagedItems(items: ["page \(pageNumber) success"], hasMorePages: false, totalItems: 1)
56+
}
57+
58+
// When
59+
let result = try await sut.loadAll(makeRequest: makeRequest)
60+
61+
// Then
62+
#expect(result == ["page 1 success", "page 2 success", "page 3 success"])
63+
#expect(attemptsByPage[1] == 1) // Page 1 succeeded immediately
64+
#expect(attemptsByPage[2] == 2) // Page 2 failed once, then succeeded
65+
#expect(attemptsByPage[3] == 1) // Page 3 succeeded immediately
66+
}
67+
68+
@Test func loadAll_does_not_retry_on_non_retryable_error() async throws {
69+
// Given
70+
let mockErrorEvaluator = MockRetryErrorEvaluator(shouldRetry: false)
71+
let sut = BatchedRequestLoader(
72+
batchSize: 2,
73+
maxRetries: 3,
74+
retryDelay: 0.01,
75+
errorEvaluator: mockErrorEvaluator
76+
)
77+
78+
var attemptCount = 0
79+
let makeRequest: (Int) async throws -> PagedItems<String> = { pageNumber in
80+
attemptCount += 1
81+
throw NetworkError.unacceptableStatusCode(statusCode: 401, response: nil)
82+
}
83+
84+
// When/Then
85+
await #expect(throws: NetworkError.self) {
86+
try await sut.loadAll(makeRequest: makeRequest)
87+
}
88+
#expect(attemptCount == 2) // No retries for non-retryable error for both pages
89+
}
90+
91+
@Test func loadAll_fails_after_max_retries() async throws {
92+
// Given
93+
let mockErrorEvaluator = MockRetryErrorEvaluator(shouldRetry: true)
94+
let sut = BatchedRequestLoader(
95+
batchSize: 1,
96+
maxRetries: 3,
97+
retryDelay: 0.01,
98+
errorEvaluator: mockErrorEvaluator
99+
)
100+
101+
var attemptCount = 0
102+
let expectedError = URLError(.networkConnectionLost)
103+
let makeRequest: (Int) async throws -> PagedItems<String> = { pageNumber in
104+
attemptCount += 1
105+
throw expectedError
106+
}
107+
108+
// When/Then
109+
await #expect(throws: URLError.self) {
110+
try await sut.loadAll(makeRequest: makeRequest)
111+
}
112+
#expect(attemptCount == 3) // maxRetries = 3
113+
}
114+
}
115+
116+
// MARK: - Mock Error Evaluator
117+
118+
private struct MockRetryErrorEvaluator: RetryErrorEvaluator {
119+
let shouldRetry: Bool
120+
121+
func shouldRetry(_ error: Error) -> Bool {
122+
shouldRetry
123+
}
124+
}

‎Modules/Tests/YosemiteTests/Tools/POS/DefaultRetryErrorEvaluatorTests.swift‎

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,56 @@
1+
import Foundation
2+
import Testing
3+
@testable import Networking
4+
@testable import Yosemite
5+
6+
struct DefaultRetryErrorEvaluatorTests {
7+
@Test(arguments: [
8+
URLError(.timedOut),
9+
URLError(.networkConnectionLost),
10+
URLError(.notConnectedToInternet),
11+
URLError(.cannotConnectToHost),
12+
URLError(.cancelled)
13+
])
14+
func it_retries_url_errors(_ error: URLError) {
15+
// Given
16+
let sut = DefaultRetryErrorEvaluator()
17+
18+
// When
19+
let shouldRetry = sut.shouldRetry(error)
20+
21+
// Then
22+
#expect(shouldRetry == true)
23+
}
24+
25+
@Test(arguments: [
26+
NetworkError.unacceptableStatusCode(statusCode: 500, response: nil),
27+
NetworkError.unacceptableStatusCode(statusCode: 503, response: nil),
28+
NetworkError.unacceptableStatusCode(statusCode: 429, response: nil),
29+
NetworkError.unacceptableStatusCode(statusCode: 401, response: nil),
30+
NetworkError.unacceptableStatusCode(statusCode: 403, response: nil),
31+
NetworkError.unacceptableStatusCode(statusCode: 404, response: nil),
32+
NetworkError.unacceptableStatusCode(statusCode: 400, response: nil)
33+
])
34+
func it_retries_network_errors(_ error: NetworkError) {
35+
// Given
36+
let sut = DefaultRetryErrorEvaluator()
37+
38+
// When
39+
let shouldRetry = sut.shouldRetry(error)
40+
41+
// Then
42+
#expect(shouldRetry == true)
43+
}
44+
45+
@Test func it_does_not_retry_invalid_cookie_nonce_error() {
46+
// Given
47+
let sut = DefaultRetryErrorEvaluator()
48+
let error = NetworkError.invalidCookieNonce
49+
50+
// When
51+
let shouldRetry = sut.shouldRetry(error)
52+
53+
// Then
54+
#expect(shouldRetry == false)
55+
}
56+
}

0 commit comments

Comments
 (0)