AutomaticRetry

pubnub · Sep 18, 2024 · 210c4e4 · 210c4e4
1 parent fdc64fd
commit 210c4e4
Show file tree

Hide file tree

Showing 4 changed files with 50 additions and 120 deletions.
diff --git 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
@@ -28,39 +28,41 @@ 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))
 
     /// Reconnect with an exponential backoff
     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
       /// Mitigates receiving 429 status code that's the result of too many requests in a given amount of time
       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):
         return delay + randomDelay
       }
     }
 
-    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<Int> = [500, 429],
     retryableURLErrorCodes: Set<URLError.Code> = 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
@@ -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
@@ -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..<maxRetryCount {
-      let policy = AutomaticRetry.ReconnectionPolicy.legacyExponential(base: base, scale: scale, maxDelay: maxDelay)
-      let delay = policy.delay(for: count)
-      XCTAssertTrue(delayForRetry[count].contains(delay))
-    }
-  }
-
-  func testLegacyExponentialBackoffDelay_MaxDelayHit() {
-    let maxRetryCount = 5
-    let scale = 2.0
-    let base: UInt = 2
-    let maxDelay: UInt = 0
-    let delayForRetry = [2.0...3.0, 2.0...3.0, 2.0...3.0, 2.0...3.0, 2.0...3.0]
-
-    for count in 0..<maxRetryCount {
-      let policy = AutomaticRetry.ReconnectionPolicy.legacyExponential(base: base, scale: scale, maxDelay: maxDelay)
-      let delay = policy.delay(for: count)
-      XCTAssertTrue(delayForRetry[count].contains(delay))
-    }
-  }
 
   // MARK: - exponentialBackoff(minDelay:maxDelay)