From 210c4e40dd5007d43e2011ee5de419dc24c51d43 Mon Sep 17 00:00:00 2001 From: jguz-pubnub Date: Wed, 18 Sep 2024 10:25:00 +0200 Subject: [PATCH] AutomaticRetry --- Documentation/PubNub_8_0_Migration_Guide.md | 7 +- .../Request/Operators/AutomaticRetry.swift | 49 +++----- Sources/PubNub/PubNubConfiguration.swift | 2 +- .../Operators/AutomaticRetryTests.swift | 112 ++++-------------- 4 files changed, 50 insertions(+), 120 deletions(-) diff --git a/Documentation/PubNub_8_0_Migration_Guide.md b/Documentation/PubNub_8_0_Migration_Guide.md index 1b795baf..caf80073 100644 --- a/Documentation/PubNub_8_0_Migration_Guide.md +++ b/Documentation/PubNub_8_0_Migration_Guide.md @@ -8,7 +8,12 @@ ___ ### Module name -* The module name has been changed to `PubNubSDK` due to a compiler error that occurs when a public type shares the same name as a module. As a result, you will need to replace `import PubNub` with `import PubNubSDK` in your Swift code. Additionally, ensure that `PubNubSDK` is listed in the `Frameworks, Libraries, and Embedded Content` section under the `General` tab in Xcode. +* The module name has been changed to `PubNubSDK` due to a compiler error that occurs when a public type shares the same name as a module. As a result, you will need to replace `import PubNub` with `import PubNubSDK` in your Swift code. Additionally, ensure that `PubNubSDK` is listed in the `Frameworks, Libraries, and Embedded Content` section under the `General` tab in Xcode + +### `ReconnectionPolicy` + +* The `.legacyExponential(base, scale, maxDelay)` enumeration case from `AutomaticRetry.ReconnectionPolicy` is no longer supported. Use `.exponential(minDelay, maxDelay)` instead +* `PubNubConfiguration` uses default `AutomaticRetry` with an exponential reconnection policy to retry Subscribe requests in case of failure. If this behavior doesn’t suit your use case, you can pass custom `AutomaticRetry` object ### `ConnectionStatus` diff --git a/Sources/PubNub/Networking/Request/Operators/AutomaticRetry.swift b/Sources/PubNub/Networking/Request/Operators/AutomaticRetry.swift index 95269659..7ccbf247 100644 --- a/Sources/PubNub/Networking/Request/Operators/AutomaticRetry.swift +++ b/Sources/PubNub/Networking/Request/Operators/AutomaticRetry.swift @@ -28,11 +28,13 @@ public struct AutomaticRetry: RequestOperator, Hashable { ) // The minimum value allowed between retries static let minDelay: UInt = 2 + // The maximum value allowed between retries + static let maxDelay: UInt = 150 /// Provides the action taken when a retry is to be performed public enum ReconnectionPolicy: Hashable, Equatable { /// Exponential backoff with base/scale factor of 2, and a 150s max delay - public static let defaultExponential: ReconnectionPolicy = .legacyExponential(base: 2, scale: 2, maxDelay: 300) + public static let defaultExponential: ReconnectionPolicy = .exponential(minDelay: minDelay, maxDelay: maxDelay) /// Linear reconnect every 3 seconds public static let defaultLinear: ReconnectionPolicy = .linear(delay: Double(3)) @@ -40,9 +42,6 @@ public struct AutomaticRetry: RequestOperator, Hashable { case exponential(minDelay: UInt, maxDelay: UInt) /// Attempt to reconnect every X seconds case linear(delay: Double) - /// Reconnect with an exponential backoff - @available(*, deprecated, message: "Use exponential(minDelay:maxDelay:) instead") - case legacyExponential(base: UInt, scale: Double, maxDelay: UInt) func delay(for retryAttempt: Int) -> TimeInterval { /// Generates a random interval that's added to the final value @@ -50,8 +49,6 @@ public struct AutomaticRetry: RequestOperator, Hashable { let randomDelay = Double.random(in: 0...1) switch self { - case let .legacyExponential(base, scale, maxDelay): - return legacyExponentialBackoffDelay(for: base, scale: scale, maxDelay: maxDelay, current: retryAttempt) + randomDelay case let .exponential(minDelay, maxDelay): return min(Double(maxDelay), Double(minDelay) * pow(2, Double(retryAttempt))) + randomDelay case let .linear(delay): @@ -59,8 +56,13 @@ public struct AutomaticRetry: RequestOperator, Hashable { } } - func legacyExponentialBackoffDelay(for base: UInt, scale: Double, maxDelay: UInt, current retryCount: Int) -> Double { - max(min(pow(Double(base), Double(retryCount)) * scale, Double(maxDelay)), Double(AutomaticRetry.minDelay)) + func maximumRetryLimit() -> Int { + switch self { + case .linear: + return 10 + case .exponential: + return 6 + } } } @@ -122,6 +124,7 @@ public struct AutomaticRetry: RequestOperator, Hashable { retryableHTTPStatusCodes: Set = [500, 429], retryableURLErrorCodes: Set = AutomaticRetry.defaultRetryableURLErrorCodes, excluded endpoints: [AutomaticRetry.Endpoint] = [ + .presence, .messageSend, .files, .messageStorage, @@ -131,13 +134,6 @@ public struct AutomaticRetry: RequestOperator, Hashable { .messageActions ] ) { - self.retryLimit = Self.validate( - value: UInt(retryLimit), - using: retryLimit < 10, - replaceOnFailure: UInt(10), - warningMessage: "The `retryLimit` must be less than or equal 10" - ) - switch policy { case let .exponential(minDelay, maxDelay): let validatedMinDelay = Self.validate( @@ -163,24 +159,15 @@ public struct AutomaticRetry: RequestOperator, Hashable { replaceOnFailure: Double(Self.minDelay), warningMessage: "The `linear.delay` must be greater than or equal \(Self.minDelay)." )) - case let .legacyExponential(base, scale, maxDelay): - self.policy = .legacyExponential( - base: Self.validate( - value: base, - using: base >= 2, - replaceOnFailure: 2, - warningMessage: "The `exponential.base` must be a minimum of 2." - ), - scale: Self.validate( - value: scale, - using: scale > 0, - replaceOnFailure: 0, - warningMessage: "The `exponential.scale` must be a positive value." - ), - maxDelay: maxDelay - ) } + self.retryLimit = Self.validate( + value: UInt(retryLimit), + using: retryLimit < policy.maximumRetryLimit(), + replaceOnFailure: UInt(policy.maximumRetryLimit()), + warningMessage: "The `retryLimit` for \(policy) must be less than or equal \(policy.maximumRetryLimit())" + ) + self.retryableHTTPStatusCodes = retryableHTTPStatusCodes self.retryableURLErrorCodes = retryableURLErrorCodes self.excluded = endpoints diff --git a/Sources/PubNub/PubNubConfiguration.swift b/Sources/PubNub/PubNubConfiguration.swift index c17db4cb..380ac600 100644 --- a/Sources/PubNub/PubNubConfiguration.swift +++ b/Sources/PubNub/PubNubConfiguration.swift @@ -85,7 +85,7 @@ public struct PubNubConfiguration: Hashable { origin: String = "ps.pndsn.com", useInstanceId: Bool = false, useRequestId: Bool = false, - automaticRetry: AutomaticRetry? = nil, + automaticRetry: AutomaticRetry? = .default, urlSessionConfiguration: URLSessionConfiguration = .pubnub, durationUntilTimeout: UInt = 300, heartbeatInterval: UInt = 0, diff --git a/Tests/PubNubTests/Networking/Operators/AutomaticRetryTests.swift b/Tests/PubNubTests/Networking/Operators/AutomaticRetryTests.swift index 73caa95b..e5c4059f 100644 --- a/Tests/PubNubTests/Networking/Operators/AutomaticRetryTests.swift +++ b/Tests/PubNubTests/Networking/Operators/AutomaticRetryTests.swift @@ -26,10 +26,9 @@ class AutomaticRetryTests: XCTestCase { func testReconnectionPolicy_DefaultExponentialPolicy() { switch defaultExpoentialPolicy { - case let .legacyExponential(base, scale, max): - XCTAssertEqual(base, 2) - XCTAssertEqual(scale, 2) - XCTAssertEqual(max, 300) + case let .exponential(minDelay, maxDelay): + XCTAssertEqual(minDelay, 2) + XCTAssertEqual(maxDelay, 150) default: XCTFail("Default Exponential Policy should only match to linear case") } @@ -44,59 +43,6 @@ class AutomaticRetryTests: XCTestCase { XCTAssertEqual(testPolicy, policy) } - func testEquatable_Init_Exponential_InvalidBase() { - let invalidBasePolicy = AutomaticRetry.ReconnectionPolicy.legacyExponential( - base: 0, - scale: 3.0, - maxDelay: 1 - ) - let validBasePolicy = AutomaticRetry.ReconnectionPolicy.legacyExponential( - base: 2, scale: 3.0, maxDelay: 1 - ) - let testPolicy = AutomaticRetry( - retryLimit: 2, policy: invalidBasePolicy, retryableHTTPStatusCodes: [], retryableURLErrorCodes: [] - ) - - XCTAssertNotEqual(testPolicy.policy, invalidBasePolicy) - XCTAssertEqual(testPolicy.policy, validBasePolicy) - } - - func testEquatable_Init_Exponential_InvalidScale() { - let invalidBasePolicy = AutomaticRetry.ReconnectionPolicy.legacyExponential( - base: 2, scale: -1.0, maxDelay: 1 - ) - let validBasePolicy = AutomaticRetry.ReconnectionPolicy.legacyExponential( - base: 2, scale: 0.0, maxDelay: 1 - ) - let testPolicy = AutomaticRetry( - retryLimit: 2, - policy: invalidBasePolicy, - retryableHTTPStatusCodes: [], - retryableURLErrorCodes: [] - ) - - XCTAssertNotEqual(testPolicy.policy, invalidBasePolicy) - XCTAssertEqual(testPolicy.policy, validBasePolicy) - } - - func testEquatable_Init_Exponential_InvalidBaseAndScale() { - let invalidBasePolicy = AutomaticRetry.ReconnectionPolicy.legacyExponential( - base: 0, scale: -1.0, maxDelay: 1 - ) - let validBasePolicy = AutomaticRetry.ReconnectionPolicy.legacyExponential( - base: 2, scale: 0.0, maxDelay: 1 - ) - let testPolicy = AutomaticRetry( - retryLimit: 2, - policy: invalidBasePolicy, - retryableHTTPStatusCodes: [], - retryableURLErrorCodes: [] - ) - - XCTAssertNotEqual(testPolicy.policy, invalidBasePolicy) - XCTAssertEqual(testPolicy.policy, validBasePolicy) - } - func testEquatable_Init_Linear_InvalidDelay() { let invalidBasePolicy = AutomaticRetry.ReconnectionPolicy.linear(delay: -1.0) let validBasePolicy = AutomaticRetry.ReconnectionPolicy.linear(delay: 2.0) @@ -162,6 +108,28 @@ class AutomaticRetryTests: XCTestCase { XCTAssertTrue(testPolicy.shouldRetry(response: testResponse, error: PubNubError(.unknown))) } + + func testShouldRetry_True_TooManyRequestsStatusCode() { + guard let url = URL(string: "http://example.com") else { + return XCTFail("Could not create URL") + } + + let testStatusCode = 429 + let testPolicy = AutomaticRetry( + retryLimit: 2, + policy: .linear(delay: 2.0), + retryableHTTPStatusCodes: [testStatusCode], + retryableURLErrorCodes: [] + ) + let testResponse = HTTPURLResponse( + url: url, + statusCode: testStatusCode, + httpVersion: nil, + headerFields: [:] + ) + + XCTAssertTrue(testPolicy.shouldRetry(response: testResponse, error: PubNubError(.unknown))) + } func testShouldRetry_True_ErrorCodeMatch() { let testURLErrorCode = URLError.Code.timedOut @@ -187,36 +155,6 @@ class AutomaticRetryTests: XCTestCase { XCTAssertFalse(testPolicy.shouldRetry(response: nil, error: testError)) } - - // MARK: - legacyExponential(base:scale:maxDelay:) - - func testLegacyExponentialBackoffDelay_DefaultScale() { - let maxRetryCount = 5 - let scale = 2.0 - let base: UInt = 3 - let maxDelay = UInt.max - let delayForRetry = [2.0...3.0, 6.0...7.0, 18.0...19.0, 54.0...55.0, 162.0...163.0] - - for count in 0..