fix: Improve remote evaluation fetch retry logic (#43)

Sources/Experiment/Experiment.swift

@@ -74,3 +74,13 @@ internal struct ExperimentError: Error {
         self.message = msg
     }
 }
+
+internal struct FetchError: Error {
+    let statusCode: Int
+    let message: String
+
+    init(_ statusCode: Int, _ msg: String) {
+        self.statusCode = statusCode
+        self.message = msg
+    }
+}

Sources/Experiment/ExperimentClient.swift

@@ -389,9 +389,9 @@ internal class DefaultExperimentClient : NSObject, ExperimentClient {
             case .success(let variants):
                 self.storeVariants(variants, options)
                 completion(result)
-            case .failure:
+            case .failure(let error):
                 completion(result)
-                if retry {
+                if retry && self.shouldRetryFetch(error) {
                     self.startRetries(user: user, options: options)
                 }
             }
@@ -511,7 +511,7 @@ internal class DefaultExperimentClient : NSObject, ExperimentClient {
             }
             self.debug("Received fetch response: \(httpResponse)")
             guard httpResponse.statusCode == 200 else {
-                completion(Result.failure(ExperimentError("Error Response: status=\(httpResponse.statusCode)")))
+                completion(Result.failure(FetchError(httpResponse.statusCode, "Error Response: status=\(httpResponse.statusCode)")))
                 return
             }
             guard let data = data else {
@@ -532,7 +532,7 @@ internal class DefaultExperimentClient : NSObject, ExperimentClient {
         return task
     }
 
-    private func startRetries(user: ExperimentUser, options: FetchOptions?) {
+    internal func startRetries(user: ExperimentUser, options: FetchOptions?) {
         backoffLock.wait()
         defer { backoffLock.signal() }
         self.backoff?.cancel()
@@ -677,6 +677,13 @@ internal class DefaultExperimentClient : NSObject, ExperimentClient {
             print("\(formatter.string(from: Date())) [Experiment] \(msg)")
         }
     }
+
+    private func shouldRetryFetch(_ e: Error) -> Bool {
+        guard let e = e as? FetchError else {
+            return true
+        }
+        return e.statusCode < 400 || e.statusCode >= 500 || e.statusCode == 429
+    }
 }
 
 private struct VariantAndSource {

Tests/ExperimentTests/ExperimentClientTests.swift

@@ -1031,6 +1031,7 @@ class ExperimentClientTests: XCTestCase {
         var mockFetch: (() -> Result<[String: Variant], Error>)? = nil
         var flagCalls = 0
         var mockFlags: (() -> Result<[String: EvaluationFlag], Error>)? = nil
+        var startRetriesCalls = 0
 
         override func doFetch(
             user: ExperimentUser,
@@ -1058,6 +1059,11 @@ class ExperimentClientTests: XCTestCase {
                 super.doFlags(timeoutMillis: timeoutMillis, completion: completion)
             }
         }
+
+        override func startRetries(user: ExperimentUser, options: FetchOptions?) {
+            startRetriesCalls += 1
+            return
+        }
     }
 
     func testStart_WithLocalAndRemoteEvaluation_CallsFetch() {
@@ -1187,6 +1193,42 @@ class ExperimentClientTests: XCTestCase {
         XCTAssertEqual("on", variant.key!)
         XCTAssertEqual("on", variant2.key!)
     }
+
+    func testRemoteFetchRetry() {
+        // Response code, error message, and whether retry should be called
+        let testData: [(Int, String, Int)] = [
+            (300, "Fetch Exception 300", 1),
+            (400, "Fetch Exception 400", 0),
+            (429, "Fetch Exception 429", 1),
+            (500, "Fetch Exception 500", 1),
+            (0, "Other Exception", 1)
+        ]
+
+        for (responseCode, errorMessage, retryCalled) in testData {
+            let storage = InMemoryStorage()
+            let config = ExperimentConfigBuilder()
+                .fetchRetryOnFailure(true)
+                .build()
+            let client = MockClient(
+                apiKey: API_KEY,
+                config: config,
+                storage: storage
+            )
+            client.mockFetch = {
+                let error = FetchError(responseCode, errorMessage)
+                return .failure(error)
+            }
+            client.mockFlags = {
+                return .success([:])
+            }
+            let user = ExperimentUserBuilder()
+                .userId("test_retry_fetch")
+                .build()
+
+            client.fetchBlocking(user: user, isTestRetry: true)
+            XCTAssertEqual(retryCalled, client.startRetriesCalls)
+        }
+    }
 }
 
 class TestAnalyticsProvider : ExperimentAnalyticsProvider {
@@ -1271,11 +1313,13 @@ extension DefaultExperimentClient {
             }
         }
     }
-    func fetchBlocking(user: ExperimentUser) {
+    func fetchBlocking(user: ExperimentUser, isTestRetry: Bool = false) {
         let s = DispatchSemaphore(value: 0)
         fetch(user: user) { _, error in
             if let error = error {
-                XCTFail(error.localizedDescription)
+                if (!isTestRetry) {
+                    XCTFail(error.localizedDescription)
+                }
             }
             s.signal()
         }