Skip to content

Commit 71ebf57

Browse files
committed
Do not retry if the thrown error is CancellationError.
1 parent 1fe81fd commit 71ebf57

3 files changed

Lines changed: 135 additions & 26 deletions

File tree

Sources/Retry/Retry.swift

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -285,7 +285,7 @@ public func retry<ClockType, ReturnType>(
285285
var attempt = 0
286286
while true {
287287
var latestError: any Error
288-
let isErrorRetryable: Bool
288+
var isErrorRetryable: Bool
289289

290290
do {
291291
return try await operation()
@@ -305,6 +305,10 @@ public func retry<ClockType, ReturnType>(
305305
}
306306

307307
latestError = latestError.originalError
308+
309+
if latestError is CancellationError {
310+
isErrorRetryable = false
311+
}
308312
}
309313

310314
logger?[metadataKey: "retry.attempt"] = "\(attempt)"

Tests/RetryTests/Fakes/ClockFake.swift

Lines changed: 52 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -29,53 +29,88 @@ class ClockFake: Clock, @unchecked Sendable {
2929

3030
private let lock = NSLock()
3131

32-
private var _now: Instant
33-
34-
private var _allSleepDurations = [Duration]()
35-
3632
init() {
3733
self._now = realClock.now
3834
}
3935

40-
var now: Instant {
41-
lock.lock()
42-
defer {
43-
lock.unlock()
36+
private var _now: Instant
37+
private(set) var now: Instant {
38+
get {
39+
lock.lock()
40+
defer {
41+
lock.unlock()
42+
}
43+
44+
return _now
4445
}
4546

46-
return _now
47+
set {
48+
lock.lock()
49+
defer {
50+
lock.unlock()
51+
}
52+
53+
_now = max(newValue, _now)
54+
}
4755
}
4856

4957
var minimumResolution: Duration {
5058
return realClock.minimumResolution
5159
}
5260

61+
private var _isSleepEnabled = false
62+
var isSleepEnabled: Bool {
63+
get {
64+
lock.lock()
65+
defer {
66+
lock.unlock()
67+
}
68+
69+
return _isSleepEnabled
70+
}
71+
72+
set {
73+
lock.lock()
74+
defer {
75+
lock.unlock()
76+
}
77+
78+
_isSleepEnabled = newValue
79+
}
80+
}
81+
5382
func sleep(until deadline: Instant,
5483
tolerance: Duration?) async throws {
5584
// Refactored into a non-async method so that `NSLock.lock` and `NSLock.unlock` can be used.
5685
// Cannot use the async-safe `NSLock.withLocking` method until the following change is released:
5786
// https://github.com/apple/swift-corelibs-foundation/pull/4736
58-
sleep(until: deadline)
87+
recordSleepDuration(deadline: deadline)
88+
89+
if isSleepEnabled {
90+
try await realClock.sleep(until: deadline,
91+
tolerance: tolerance)
92+
}
93+
94+
now = deadline
5995
}
6096

61-
private func sleep(until deadline: Instant) {
97+
private var _allSleepDurations = [Duration]()
98+
var allSleepDurations: [Duration] {
6299
lock.lock()
63100
defer {
64101
lock.unlock()
65102
}
66103

67-
let duration = deadline - _now
68-
_allSleepDurations.append(duration)
69-
70-
_now = max(deadline, _now)
104+
return _allSleepDurations
71105
}
72106

73-
var allSleepDurations: [Duration] {
107+
private func recordSleepDuration(deadline: Instant) {
74108
lock.lock()
75109
defer {
76110
lock.unlock()
77111
}
78112

79-
return _allSleepDurations
113+
let duration = deadline - _now
114+
_allSleepDurations.append(duration)
80115
}
81116
}

Tests/RetryTests/RetryTests.swift

Lines changed: 78 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -78,7 +78,7 @@ final class RetryTests: XCTestCase {
7878
}
7979

8080
func testAllAttemptsFail_failureAfterRetries() async throws {
81-
try await assertThrowsErrorFake {
81+
try await assertThrows(ErrorFake.self) {
8282
try await retry(with: testingConfiguration) {
8383
throw ErrorFake()
8484
}
@@ -90,7 +90,7 @@ final class RetryTests: XCTestCase {
9090
func testFailure_shouldRetryReturnsFalse_failureWithoutRetry() async throws {
9191
precondition(Self.maxAttempts > 1)
9292

93-
try await assertThrowsErrorFake {
93+
try await assertThrows(ErrorFake.self) {
9494
try await retry(with: testingConfiguration.withShouldRetry({ _ in false })) {
9595
throw ErrorFake()
9696
}
@@ -102,7 +102,7 @@ final class RetryTests: XCTestCase {
102102
func testFailure_isNotRetryableError_failureWithoutRetry() async throws {
103103
precondition(Self.maxAttempts > 1)
104104

105-
try await assertThrowsErrorFake {
105+
try await assertThrows(ErrorFake.self) {
106106
try await retry(with: testingConfiguration) {
107107
throw NotRetryable(ErrorFake())
108108
}
@@ -130,7 +130,7 @@ final class RetryTests: XCTestCase {
130130
}
131131

132132
func testAllAttemptsFail_latestErrorIsRetryableError_throwsOriginalError() async throws {
133-
try await assertThrowsErrorFake {
133+
try await assertThrows(ErrorFake.self) {
134134
try await retry(with: testingConfiguration) {
135135
throw Retryable(NotRetryable(ErrorFake()))
136136
}
@@ -139,8 +139,8 @@ final class RetryTests: XCTestCase {
139139
assertRetried(times: Self.maxAttempts - 1)
140140
}
141141

142-
func testFailure_errorIsNotRetryableError_throwsOriginalError() async throws {
143-
try await assertThrowsErrorFake {
142+
func testFailure_isNotRetryableError_throwsOriginalError() async throws {
143+
try await assertThrows(ErrorFake.self) {
144144
try await retry(with: testingConfiguration) {
145145
throw NotRetryable(Retryable(ErrorFake()))
146146
}
@@ -149,12 +149,82 @@ final class RetryTests: XCTestCase {
149149
assertRetried(times: 0)
150150
}
151151

152+
func testFailure_isCancellationError_failureWithoutRetry() async throws {
153+
precondition(Self.maxAttempts > 1)
154+
155+
try await assertThrows(CancellationError.self) {
156+
try await retry(with: testingConfiguration) {
157+
throw CancellationError()
158+
}
159+
}
160+
161+
assertRetried(times: 0)
162+
}
163+
164+
func testFailure_isCancellationErrorWrappedInRetryableError_failureWithoutRetry() async throws {
165+
precondition(Self.maxAttempts > 1)
166+
167+
try await assertThrows(CancellationError.self) {
168+
try await retry(with: testingConfiguration) {
169+
throw Retryable(CancellationError())
170+
}
171+
}
172+
173+
assertRetried(times: 0)
174+
}
175+
176+
func testFailure_isCancellationErrorWrappedInNotRetryableError_failureWithoutRetry() async throws {
177+
precondition(Self.maxAttempts > 1)
178+
179+
try await assertThrows(CancellationError.self) {
180+
try await retry(with: testingConfiguration) {
181+
throw NotRetryable(CancellationError())
182+
}
183+
}
184+
185+
assertRetried(times: 0)
186+
}
187+
188+
func testCancelledDuringSleep_immediateFailure() async throws {
189+
precondition(Self.maxAttempts > 1)
190+
191+
clockFake.isSleepEnabled = true
192+
let configuration = testingConfiguration.withBackoff(.constant(.seconds(60)))
193+
194+
let retryTask = Task {
195+
try await retry(with: configuration) {
196+
throw ErrorFake()
197+
}
198+
}
199+
200+
// Wait until the retry task is sleeping after the first attempt.
201+
while clockFake.allSleepDurations.isEmpty {
202+
try await Task.sleep(for: .milliseconds(1))
203+
}
204+
205+
retryTask.cancel()
206+
207+
let realClock = ContinuousClock()
208+
let start = realClock.now
209+
210+
try await assertThrows(CancellationError.self) {
211+
try await retryTask.value
212+
}
213+
214+
let end = realClock.now
215+
let duration = end - start
216+
XCTAssertLessThan(duration, .seconds(1))
217+
}
218+
152219
// MARK: - Assertions
153220

154-
private func assertThrowsErrorFake(operation: () async throws -> Void) async throws {
221+
private func assertThrows<T: Error>(
222+
_ errorType: T.Type,
223+
operation: () async throws -> Void
224+
) async throws {
155225
do {
156226
try await operation()
157-
} catch is ErrorFake {
227+
} catch is T {
158228
// Expected.
159229
}
160230
}

0 commit comments

Comments
 (0)