Expose status code in retriable error and add more precise logging (#…

…3284) * Expose status code in retriable error and add more precise logging * Bubble up errorMessage from connectionError * Comments * Fix tests * Fix tests * Fix lint * Remove comment --------- Co-authored-by: crow <david.crow@airship.com>
urbanairship · Nov 26, 2024 · 4e4a06e · 4e4a06e
1 parent 5cd165b
commit 4e4a06e
Showing 8 changed files with 61 additions and 45 deletions.
diff --git a/Airship/AirshipAutomation/Source/Automation/Engine/AutomationPreparer.swift b/Airship/AirshipAutomation/Source/Automation/Engine/AutomationPreparer.swift
@@ -302,7 +302,7 @@ struct AutomationPreparer: AutomationPreparerProtocol {
         case .notFound:
             await self.remoteDataAccess.notifyOutdated(schedule: schedule)
             return .success(result: .invalidate)
-        case .retriableError(retryAfter: let retryAfter):
+        case .retriableError(retryAfter: let retryAfter, statusCode: _):
             if let retryAfter {
                 return .retryAfter(retryAfter)
             } else {

diff --git a/Airship/AirshipCore/Source/DeferredResolver.swift b/Airship/AirshipCore/Source/DeferredResolver.swift
@@ -8,7 +8,7 @@ public enum AirshipDeferredResult<T : Sendable&Equatable>: Sendable, Equatable {
     case timedOut
     case outOfDate
     case notFound
-    case retriableError(retryAfter: TimeInterval? = nil)
+    case retriableError(retryAfter: TimeInterval? = nil, statusCode: Int? = nil)
 }
 
 /// NOTE: For internal use only. :nodoc:
@@ -147,30 +147,30 @@ final class AirshipDeferredResolver : AirshipDeferredResolverProtocol {
         case 200:
             do {
                 guard let body = result.result else {
-                    return .retriableError()
+                    return .retriableError(statusCode: result.statusCode)
                 }
                 let parsed = try await resultParser(body)
 
                 AirshipLogger.trace("Deferred result body: \(parsed)")
 
                 return .success(parsed)
             } catch {
-                AirshipLogger.error("Failed ot parse deferred body \(error)")
-                return .retriableError()
+                AirshipLogger.error("Failed to parse deferred body \(error) with status code: \(result.statusCode)")
+                return .retriableError(statusCode: result.statusCode)
             }
         case 404: return .notFound
         case 409: return .outOfDate
         case 429:
             if let location = result.locationHeader {
                 locationMap.value[url] = location
             }
-            return .retriableError(retryAfter: result.retryAfter)
+            return .retriableError(retryAfter: result.retryAfter, statusCode: result.statusCode)
         case 307:
             if let location = result.locationHeader {
                 locationMap.value[url] = location
 
                 if let retry = result.retryAfter, retry > 0 {
-                    return .retriableError(retryAfter: retry)
+                    return .retriableError(retryAfter: retry, statusCode: result.statusCode)
                 }
 
                 if (allowRetry) {
@@ -186,9 +186,9 @@ final class AirshipDeferredResolver : AirshipDeferredResolverProtocol {
                     )
                 }
             }
-            return .retriableError(retryAfter: result.retryAfter ?? 0)
+            return .retriableError(retryAfter: result.retryAfter ?? 0, statusCode: result.statusCode)
         default:
-            return .retriableError()
+            return .retriableError(statusCode: result.statusCode)
         }
     }
 }

diff --git a/Airship/AirshipCore/Tests/DeferredResolverTest.swift b/Airship/AirshipCore/Tests/DeferredResolverTest.swift
@@ -129,16 +129,18 @@ final class DeferredResolverTest: XCTestCase {
             notificationOptIn: true
         )
 
+        let statusCode: Int = 200
+
         let body = "some body".data(using: .utf8)
         self.client.onResolve = { _, _, _, _, _, _ in
-            return AirshipHTTPResponse(result: body, statusCode: 200, headers: [:])
+            return AirshipHTTPResponse(result: body, statusCode: statusCode, headers: [:])
         }
 
         let result: AirshipDeferredResult<Data> = await resolver.resolve(request: request) { data in
             throw AirshipErrors.error("parse error")
         }
 
-        XCTAssertEqual(result, .retriableError())
+        XCTAssertEqual(result, .retriableError(statusCode: statusCode))
     }
 
 
@@ -210,16 +212,17 @@ final class DeferredResolverTest: XCTestCase {
             notificationOptIn: true
         )
 
+        let statusCode: Int = 429
         let body = "some body".data(using: .utf8)!
         self.client.onResolve = { _, _, _, _, _, _ in
-            return AirshipHTTPResponse(result: body, statusCode: 429, headers: ["Location": self.altExampleURL.absoluteString])
+            return AirshipHTTPResponse(result: body, statusCode: statusCode, headers: ["Location": self.altExampleURL.absoluteString])
         }
 
         let result: AirshipDeferredResult<Data> = await resolver.resolve(request: request) { data in
             return data
         }
 
-        XCTAssertEqual(result, .retriableError())
+        XCTAssertEqual(result, .retriableError(statusCode: statusCode))
 
         self.client.onResolve = { url, _, _, _, _, _ in
             XCTAssertEqual(url, self.altExampleURL)
@@ -242,16 +245,17 @@ final class DeferredResolverTest: XCTestCase {
             notificationOptIn: true
         )
 
+        let statusCode: Int = 429
         let body = "some body".data(using: .utf8)!
         self.client.onResolve = { _, _, _, _, _, _ in
-            return AirshipHTTPResponse(result: body, statusCode: 429, headers: ["Retry-After": "100.0"])
+            return AirshipHTTPResponse(result: body, statusCode: statusCode, headers: ["Retry-After": "100.0"])
         }
 
         let result: AirshipDeferredResult<Data> = await resolver.resolve(request: request) { data in
             return data
         }
 
-        XCTAssertEqual(result, .retriableError(retryAfter: 100.0))
+        XCTAssertEqual(result, .retriableError(retryAfter: 100.0,  statusCode: statusCode))
     }
 
     func testResolve307() async throws {
@@ -287,11 +291,13 @@ final class DeferredResolverTest: XCTestCase {
         )
 
         let body = "some body".data(using: .utf8)!
+
+        let statusCode: Int = 307
         self.client.onResolve = { url, _, _, _, _, _ in
             if (url == self.exampleURL) {
                 return AirshipHTTPResponse(
                     result: nil,
-                    statusCode: 307,
+                    statusCode: statusCode,
                     headers: [
                         "Location": self.altExampleURL.absoluteString,
                         "Retry-After": "20.0"
@@ -306,7 +312,7 @@ final class DeferredResolverTest: XCTestCase {
             return data
         }
 
-        XCTAssertEqual(result, .retriableError(retryAfter: 20.0))
+        XCTAssertEqual(result, .retriableError(retryAfter: 20.0, statusCode: statusCode))
 
         let anotherResult: AirshipDeferredResult<Data> = await resolver.resolve(request: request) { data in
             return data

diff --git a/Airship/AirshipCore/Tests/TestDeferredResolver.swift b/Airship/AirshipCore/Tests/TestDeferredResolver.swift
@@ -18,12 +18,12 @@ final class TestDeferredResolver: AirshipDeferredResolverProtocol, @unchecked Se
                 let value = try await resultParser(data)
                 return .success(value)
             } catch {
-                return .retriableError()
+                return .retriableError(statusCode: 200)
             }
         case .timedOut: return .timedOut
         case .outOfDate: return .outOfDate
         case .notFound: return .notFound
-        case .retriableError(retryAfter: let retryAfter): return .retriableError(retryAfter: retryAfter)
+        case .retriableError(retryAfter: let retryAfter, statusCode: let statusCode): return .retriableError(retryAfter: retryAfter, statusCode:statusCode)
         }
     }
 }
diff --git a/Airship/AirshipFeatureFlags/Source/DeferredFlagResolver.swift b/Airship/AirshipFeatureFlags/Source/DeferredFlagResolver.swift
@@ -103,18 +103,26 @@ actor FeatureFlagDeferredResolver: FeatureFlagDeferredResolverProtocol {
         case .notFound:
             return .notFound
 
-        case .retriableError(let retryAfter):
+        case .retriableError(let retryAfter, let statusCode):
             let backoff = retryAfter ?? FeatureFlagDeferredResolver.defaultBackoff
 
             guard allowRetry, backoff <= FeatureFlagDeferredResolver.immediateBackoffRetryCutoff else {
                 backOffDates[requestID] = self.date.now.advanced(by: backoff)
-                throw FeatureFlagEvaluationError.connectionError
+                if let statusCode = statusCode {
+                    throw FeatureFlagEvaluationError.connectionError(errorMessage: "Failed to resolve flag. Status code: \(statusCode)")
+                }
+
+                throw FeatureFlagEvaluationError.connectionError(errorMessage: "Failed to resolve flag.")
             }
 
             if (backoff > 0) {
+                AirshipLogger.debug(statusCode == nil ? "Backing off deferred flag request \(requestID) for \(backoff) seconds" : "Backing off deferred flag request \(requestID) for \(backoff) seconds with status code: \(statusCode!)")
+
                 try await self.taskSleeper.sleep(timeInterval: backoff)
             }
 
+            AirshipLogger.error(statusCode == nil ? "Retrying deferred flag request \(requestID)" : "Retrying deferred flag request \(requestID) with status code: \(statusCode!)")
+
             return try await self.fetchFlag(
                 request: request,
                 requestID: requestID,
@@ -126,7 +134,7 @@ actor FeatureFlagDeferredResolver: FeatureFlagDeferredResolverProtocol {
             throw FeatureFlagEvaluationError.outOfDate
 
         default:
-            throw FeatureFlagEvaluationError.connectionError
+            throw FeatureFlagEvaluationError.connectionError(errorMessage: "Failed to resolve flag.")
         }
     }
 }

diff --git a/Airship/AirshipFeatureFlags/Source/FeatureFlagManager.swift b/Airship/AirshipFeatureFlags/Source/FeatureFlagManager.swift
@@ -9,15 +9,17 @@ import AirshipCore
 
 
 /// Feature flag errors
-public enum FeatureFlagError: Error {
-    /// Failed to fetch the data for the feature flag. The app should retry on this error.
+public enum FeatureFlagError: Error, Equatable {
     case failedToFetchData
+    case staleData
+    case outOfDate
+    case connectionError(errorMessage: String)
 }
 
-enum FeatureFlagEvaluationError: Error {
-    case connectionError
+enum FeatureFlagEvaluationError: Error, Equatable {
     case outOfDate
     case staleNotAllowed
+    case connectionError(errorMessage: String)
 }
 
 /// Airship feature flag manager
@@ -92,8 +94,8 @@ public final class FeatureFlagManager: Sendable {
             )
         } catch {
             switch (error) {
-            case FeatureFlagEvaluationError.connectionError:
-                throw FeatureFlagError.failedToFetchData
+            case FeatureFlagEvaluationError.connectionError(let errorMessage):
+                throw FeatureFlagError.connectionError(errorMessage: errorMessage)
             case FeatureFlagEvaluationError.outOfDate:
                 await self.remoteDataAccess.notifyOutdated(
                     remoteDateInfo: remoteDataFeatureFlagInfo.remoteDataInfo
@@ -103,14 +105,14 @@ public final class FeatureFlagManager: Sendable {
                     await self.remoteDataAccess.waitForRefresh()
                     return try await self.flag(name: name, allowRefresh: false)
                 }
-                throw FeatureFlagError.failedToFetchData
+                throw FeatureFlagError.outOfDate
 
             case FeatureFlagEvaluationError.staleNotAllowed:
                 if (allowRefresh) {
                     await self.remoteDataAccess.waitForRefresh()
                     return try await self.flag(name: name, allowRefresh: false)
                 }
-                throw FeatureFlagError.failedToFetchData
+                throw FeatureFlagError.staleData
             default:
                 AirshipLogger.error("Unexpected error \(error)")
                 throw FeatureFlagError.failedToFetchData

diff --git a/Airship/AirshipFeatureFlags/Tests/FeatureFlagDeferredResolverTest.swift b/Airship/AirshipFeatureFlags/Tests/FeatureFlagDeferredResolverTest.swift
@@ -167,7 +167,7 @@ final class FeatureFlagDeferredResolverTest: XCTestCase {
             )
             XCTFail()
         } catch {
-            XCTAssertEqual(FeatureFlagEvaluationError.connectionError, error as! FeatureFlagEvaluationError)
+            XCTAssertEqual(FeatureFlagEvaluationError.connectionError(errorMessage: "Failed to resolve flag."), error as! FeatureFlagEvaluationError)
         }
 
         await fulfillment(of: [expectation])
@@ -177,7 +177,7 @@ final class FeatureFlagDeferredResolverTest: XCTestCase {
         let expectation = expectation(description: "flag resolved")
         self.deferredResolver.onData = { _ in
             expectation.fulfill()
-            return .retriableError()
+            return .retriableError(statusCode: nil)
         }
 
         do {
@@ -187,7 +187,7 @@ final class FeatureFlagDeferredResolverTest: XCTestCase {
             )
             XCTFail()
         } catch {
-            XCTAssertEqual(FeatureFlagEvaluationError.connectionError, error as! FeatureFlagEvaluationError)
+            XCTAssertEqual(FeatureFlagEvaluationError.connectionError(errorMessage: "Failed to resolve flag."), error as! FeatureFlagEvaluationError)
         }
 
         await fulfillment(of: [expectation])
@@ -210,7 +210,7 @@ final class FeatureFlagDeferredResolverTest: XCTestCase {
             )
             XCTFail()
         } catch {
-            XCTAssertEqual(FeatureFlagEvaluationError.connectionError, error as! FeatureFlagEvaluationError)
+            XCTAssertEqual(FeatureFlagEvaluationError.connectionError(errorMessage: "Failed to resolve flag."), error as! FeatureFlagEvaluationError)
         }
 
         await fulfillment(of: [expectation])
@@ -232,7 +232,7 @@ final class FeatureFlagDeferredResolverTest: XCTestCase {
             )
             XCTFail()
         } catch {
-            XCTAssertEqual(FeatureFlagEvaluationError.connectionError, error as! FeatureFlagEvaluationError)
+            XCTAssertEqual(FeatureFlagEvaluationError.connectionError(errorMessage: "Failed to resolve flag."), error as! FeatureFlagEvaluationError)
         }
 
         await fulfillment(of: [expecation])
@@ -371,7 +371,7 @@ fileprivate final class TestDeferredResolver: AirshipDeferredResolverProtocol, @
         case .timedOut: return .timedOut
         case .outOfDate: return .outOfDate
         case .notFound: return .notFound
-        case .retriableError(retryAfter: let retryAfter): return .retriableError(retryAfter: retryAfter)
+        case .retriableError(retryAfter: let retryAfter, statusCode: let statusCode): return .retriableError(retryAfter: retryAfter, statusCode: statusCode)
         @unknown default:
             fatalError()
         }

diff --git a/Airship/AirshipFeatureFlags/Tests/FeatureFlagManagerTest.swift b/Airship/AirshipFeatureFlags/Tests/FeatureFlagManagerTest.swift
@@ -838,10 +838,10 @@ final class AirshipFeatureFlagsTest: XCTestCase {
         do {
             let _ = try await featureFlagManager.flag(name: "foo")
             XCTFail("Should throw")
-        } catch FeatureFlagError.failedToFetchData {
+        } catch FeatureFlagError.staleData {
             // No-op
         } catch {
-            XCTFail("Should throw failedToFetchData")
+            XCTFail("Should throw staleData")
         }
     }
 
@@ -881,10 +881,10 @@ final class AirshipFeatureFlagsTest: XCTestCase {
         do {
             let _ = try await featureFlagManager.flag(name: "foo")
             XCTFail("Should throw")
-        } catch FeatureFlagError.failedToFetchData {
+        }catch FeatureFlagError.staleData {
             // No-op
         } catch {
-            XCTFail("Should throw failedToFetchData")
+            XCTFail("Should throw staleData")
         }
     }
 
@@ -894,10 +894,10 @@ final class AirshipFeatureFlagsTest: XCTestCase {
         do {
             let _ = try await featureFlagManager.flag(name: "foo")
             XCTFail("Should throw")
-        } catch FeatureFlagError.failedToFetchData {
+        } catch FeatureFlagError.outOfDate {
             // No-op
         } catch {
-            XCTFail("Should throw failedToFetchData")
+            XCTFail("Should throw outOfDate")
         }
     }
 
@@ -1223,7 +1223,7 @@ final class AirshipFeatureFlagsTest: XCTestCase {
         do {
             _ = try await featureFlagManager.flag(name: "foo")
         } catch {
-            XCTAssertEqual(error as! FeatureFlagError, FeatureFlagError.failedToFetchData)
+            XCTAssertEqual(error as! FeatureFlagError, FeatureFlagError.outOfDate)
         }
 
         XCTAssertEqual(remoteDataAccess.lastOutdatedRemoteInfo, self.remoteDataAccess.remoteDataInfo)
@@ -1259,13 +1259,13 @@ final class AirshipFeatureFlagsTest: XCTestCase {
         }
 
         await self.deferredResolver.setOnResolve { _, _ in
-            throw FeatureFlagEvaluationError.connectionError
+            throw FeatureFlagEvaluationError.connectionError(errorMessage: "Failed to resolve flag.")
         }
 
         do {
             _ = try await featureFlagManager.flag(name: "foo")
         } catch {
-            XCTAssertEqual(error as! FeatureFlagError, FeatureFlagError.failedToFetchData)
+            XCTAssertEqual(error as! FeatureFlagError, FeatureFlagError.connectionError(errorMessage: "Failed to resolve flag."))
         }
 
         XCTAssertNil(remoteDataAccess.lastOutdatedRemoteInfo)