Apple client send and receive support for crash report cohort IDs (CRCIDs)

Package.swift

@@ -514,7 +514,8 @@ let package = Package(
         .testTarget(
             name: "CrashesTests",
             dependencies: [
-                "Crashes"
+                "Crashes",
+                "TestUtils"
             ]
         ),
         .testTarget(

Sources/Crashes/CrashCollection.swift

@@ -18,6 +18,8 @@
 
 import Foundation
 import MetricKit
+import Persistence
+import os.log
 
 public enum CrashCollectionPlatform {
     case iOS, macOS, macOSAppStore
@@ -38,9 +40,10 @@
 @available(iOS 13, macOS 12, *)
 public final class CrashCollection {
 
-    public init(platform: CrashCollectionPlatform) {
-        crashHandler = CrashHandler()
-        crashSender = CrashReportSender(platform: platform)
+    public init(crashReportSender: CrashReportSending, crashCollectionStorage: KeyValueStoring = UserDefaults()) {
+        self.crashHandler = CrashHandler()
+        self.crashSender = crashReportSender
+        self.crashCollectionStorage = crashCollectionStorage
     }
 
     public func start(didFindCrashReports: @escaping (_ pixelParameters: [[String: String]], _ payloads: [Data], _ uploadReports: @escaping () -> Void) -> Void) {
@@ -55,7 +58,11 @@
     ///   - didFindCrashReports: callback called after payload preprocessing is finished.
     ///     Provides processed JSON data to be presented to the user and Pixel parameters to fire a crash Pixel.
     ///     `uploadReports` callback is used when the user accepts uploading the crash report and starts crash upload to the server.
-    public func start(process: @escaping ([MXDiagnosticPayload]) -> [Data], didFindCrashReports: @escaping (_ pixelParameters: [[String: String]], _ payloads: [Data], _ uploadReports: @escaping () -> Void) -> Void) {
+    public func start(process: @escaping ([MXDiagnosticPayload]) -> [Data],
+                      didFindCrashReports: @escaping (_ pixelParameters: [[String: String]],
+                                                      _ payloads: [Data],
+                                                      _ uploadReports: @escaping () -> Void) -> Void,
+                      didFinishHandlingResponse: @escaping (() -> Void) = {}) {
         let first = isFirstCrash
         isFirstCrash = false
 
@@ -80,8 +87,29 @@
             didFindCrashReports(pixelParameters, processedData) {
                 Task {
                     for payload in processedData {
-                        await self.crashSender.send(payload)
+                        let crcid = self.crashCollectionStorage.object(forKey: Const.crcidKey) as? String
+                        let result =  await self.crashSender.send(payload, crcid: crcid)
+
+                        switch result.result {
+                        case .success:
+                            Logger.general.debug("Crash Collection - Sending Crash Report: succeeded")
+                            if let receivedCRCID = result.response?.allHeaderFields[CrashReportSender.httpHeaderCRCID] as? String {
+                                if crcid != receivedCRCID {
+                                    Logger.general.debug("Received new value for CRCID: \(receivedCRCID), setting local crcid value")
+                                    self.crashCollectionStorage.set(receivedCRCID, forKey: Const.crcidKey)
+                                } else {
+                                    Logger.general.debug("Received matching value for CRCID: \(receivedCRCID), no update necessary")
+                                }
+                            } else {
+                                Logger.general.debug("No value for CRCID header: \(Const.crcidKey), clearing local crcid value if present")
+                                self.crashCollectionStorage.removeObject(forKey: Const.crcidKey)
+                            }
+                        case .failure(let failure):
+                            // TODO: Is it worth sending a pixel for this case, so that we can monitor for missing crash reports?
+                            Logger.general.debug("Crash Collection - Sending Crash Report: failed (\(failure))")
+                        }
                     }
+                    didFinishHandlingResponse()
                 }
             }
         }
@@ -173,18 +201,20 @@
 
     var isFirstCrash: Bool {
         get {
-            UserDefaults().object(forKey: Const.firstCrashKey) as? Bool ?? true
+            crashCollectionStorage.object(forKey: Const.firstCrashKey) as? Bool ?? true
         }
 
         set {
-            UserDefaults().set(newValue, forKey: Const.firstCrashKey)
+            crashCollectionStorage.set(newValue, forKey: Const.firstCrashKey)
         }
     }
 
     let crashHandler: CrashHandler
-    let crashSender: CrashReportSender
+    let crashSender: CrashReportSending
+    let crashCollectionStorage: KeyValueStoring
 
     enum Const {
         static let firstCrashKey = "CrashCollection.first"
+        static let crcidKey = "CrashCollection.crcid"
     }
 }

Sources/Crashes/CrashReportSender.swift

@@ -19,28 +19,86 @@
 import Foundation
 import MetricKit
 
-public final class CrashReportSender {
+public protocol CrashReportSending {
+    init(platform: CrashCollectionPlatform)
+    func send(_ crashReportData: Data, crcid: String?) async -> (result: Result<Data?, Error>, response: HTTPURLResponse?)
+    func send(_ crashReportData: Data, crcid: String?, completion: @escaping (_ result: Result<Data?, Error>, _ response: HTTPURLResponse?) -> Void)
+}
+
+enum CrashReportSenderError: Error {
+    case invalidResponse
+}
+
+// By conforming to a protocol, we can sub in mocks more easily
+public final class CrashReportSender: CrashReportSending {
 
     static let reportServiceUrl = URL(string: "https://duckduckgo.com/crash.js")!
+
+    static let httpHeaderCRCID = "crcid"
+
     public let platform: CrashCollectionPlatform
+
+    private let session = URLSession(configuration: .ephemeral)
 
     public init(platform: CrashCollectionPlatform) {
         self.platform = platform
     }
 
-    public func send(_ crashReportData: Data) async {
+    public func send(_ crashReportData: Data, crcid: String?, completion: @escaping (_ result: Result<Data?, Error>, _ response: HTTPURLResponse?) -> Void) {
         var request = URLRequest(url: Self.reportServiceUrl)
         request.setValue("text/plain", forHTTPHeaderField: "Content-Type")
         request.setValue(platform.userAgent, forHTTPHeaderField: "User-Agent")
+        if let crcid {
+            request.setValue(crcid, forHTTPHeaderField: CrashReportSender.httpHeaderCRCID)
+            Logger.general.debug("Configured crash report HTTP request with crcid: \(crcid)")
+        }
         request.httpMethod = "POST"
         request.httpBody = crashReportData
-
-        do {
-            _ = try await session.data(for: request)
-        } catch {
-            assertionFailure("CrashReportSender: Failed to send the crash report")
+
+        Logger.general.debug("CrashReportSender: Awaiting session data")
+        let task = session.dataTask(with: request) { data, response, error in
+            // TODO: Consider pixels for failures that mean we may have lost crash info?
+            if let response = response as? HTTPURLResponse {
+                Logger.general.debug("CrashReportSender: Received HTTP response code: \(response.statusCode)")
+                if response.statusCode == 200 {
+                    response.allHeaderFields.forEach { print("\($0.key): \($0.value)") }    // TODO: Why do we straight-up print these, rather than debug logging?
+                } else {
+                    assertionFailure("CrashReportSender: Failed to send the crash report: \(response.statusCode)")
+                }
+
+                if let data {
+                    completion(.success(data), response)
+                } else if let error {
+                    completion(.failure(error), response)
+                } else {
+                    completion(.failure(CrashReportSenderError.invalidResponse), response)
+                }
+            } else {
+                completion(.failure(CrashReportSenderError.invalidResponse), nil)
+            }
+        }
+        task.resume()
+    }
+
+    public func send(_ crashReportData: Data, crcid: String?) async -> (result: Result<Data?, Error>, response: HTTPURLResponse?) {
+        await withCheckedContinuation { continuation in
+            send(crashReportData, crcid: crcid) { result, response in
+                continuation.resume(returning: (result, response))
+            }
+        }
+    }
+
+    static let crashReportCohortIDKey = "CrashReportSender.crashReportCohortID"
+    var crashReportCohortID: String? {
+        get {
+            if let crcid = UserDefaults().string(forKey: CrashReportSender.crashReportCohortIDKey) {
+                return crcid
+            } else {
+                return nil
+            }
+        }
+        set {
+            UserDefaults().setValue(newValue, forKey: CrashReportSender.crashReportCohortIDKey)
         }
     }
-
-    private let session = URLSession(configuration: .ephemeral)
 }

Sources/TestUtils/MockKeyValueStore.swift

@@ -19,11 +19,11 @@
 import Foundation
 import Persistence
 
-public class MockKeyValueStore: KeyValueStoring {
+public class MockKeyValueStore: NSObject, KeyValueStoring {
 
     public var store = [String: Any?]()
 
-    public init() { }
+    public override init() { }
 
     public func object(forKey defaultName: String) -> Any? {
         return store[defaultName] as Any?