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,9 @@
 
 import Foundation
 import MetricKit
+import Persistence
+import os.log
+import Common
 
 public enum CrashCollectionPlatform {
     case iOS, macOS, macOSAppStore
@@ -38,9 +41,12 @@
 @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
+        self.crcidManager = CRCIDManager(store: crashCollectionStorage)
     }
 
     public func start(didFindCrashReports: @escaping (_ pixelParameters: [[String: String]], _ payloads: [Data], _ uploadReports: @escaping () -> Void) -> Void) {
@@ -55,7 +61,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 +90,10 @@
             didFindCrashReports(pixelParameters, processedData) {
                 Task {
                     for payload in processedData {
-                        await self.crashSender.send(payload)
+                        let result =  await self.crashSender.send(payload, crcid: self.crcidManager.crcid)
+                        self.crcidManager.handleCrashSenderResult(result: result.result, response: result.response)
                     }
+                    didFinishHandlingResponse()
                 }
             }
         }
@@ -171,20 +183,72 @@
         }, didFindCrashReports: didFindCrashReports)
     }
 
+    public func clearCRCID() {
+        self.crcidManager.crcid = ""
+    }
+
     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
+    let crcidManager: CRCIDManager
 
     enum Const {
         static let firstCrashKey = "CrashCollection.first"
     }
 }
+
+// TODO: This should really be its own file, but adding a new file to BSK and propagating it to iOS and macOS projects is hard.  This can be done as a separate PR once the main changes land across all 3 repos
+// import Foundation
+// import Persistence
+// import os.log
+
+// Cohort identifier used exclusively to distinguish systemic crashes, only after the user opts in to send them.
+// Its purpose is strictly limited to improving the reliability of crash reporting and is never used elsewhere.
+public class CRCIDManager {
+    static let crcidKey = "CRCIDManager.crcidKey"
+    var store: KeyValueStoring
+
+    public init(store: KeyValueStoring = UserDefaults()) {
+        self.store = store
+    }
+
+    public func handleCrashSenderResult(result: Result<Data?, Error>, response: HTTPURLResponse?) {
+        switch result {
+        case .success:
+            Logger.general.debug("Crash Collection - Sending Crash Report: succeeded")
+            if let receivedCRCID = response?.allHeaderFields[CrashReportSender.httpHeaderCRCID] as? String {
+                if crcid != receivedCRCID {
+                    Logger.general.debug("Crash Collection - Received new value for CRCID: \(receivedCRCID), setting local crcid value")
+                    crcid =  receivedCRCID
+                } else {
+                    Logger.general.debug("Crash Collection - Received matching value for CRCID: \(receivedCRCID), no update necessary")
+                }
+            } else {
+                Logger.general.debug("Crash Collection - No value for CRCID header: \(CRCIDManager.crcidKey), clearing local crcid value if present")
+                crcid = ""
+            }
+        case .failure(let failure):
+            Logger.general.debug("Crash Collection - Sending Crash Report: failed (\(failure))")
+        }
+    }
+
+    public var crcid: String {
+        get {
+            return self.store.object(forKey: CRCIDManager.crcidKey) as? String ?? ""
+        }
+
+        set {
+            store.set(newValue, forKey: CRCIDManager.crcidKey)
+        }
+    }
+}

Sources/Crashes/CrashReportSender.swift

@@ -18,29 +18,88 @@
 
 import Foundation
 import MetricKit
+import Common
 
-public final class CrashReportSender {
+public protocol CrashReportSending {
+    var pixelEvents: EventMapping<CrashReportSenderError>? { get }
+
+    init(platform: CrashCollectionPlatform, pixelEvents: EventMapping<CrashReportSenderError>?)
+
+    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)
+}
+
+public enum CrashReportSenderError: Error {
+    case crcidMissing
+    case submissionFailed(HTTPURLResponse?)
+}
+
+// By conforming to a protocol, we can sub in mocks more easily
+public final class CrashReportSender: CrashReportSending {
+    static let reportServiceUrl = URL(string: "https://9e3c-20-75-144-152.ngrok-free.app/crash.js")!
+
+    static let httpHeaderCRCID = "crcid"
 
-    static let reportServiceUrl = URL(string: "https://duckduckgo.com/crash.js")!
     public let platform: CrashCollectionPlatform
+    public var pixelEvents: EventMapping<CrashReportSenderError>?
 
-    public init(platform: CrashCollectionPlatform) {
+    private let session = URLSession(configuration: .ephemeral)
+
+    public init(platform: CrashCollectionPlatform, pixelEvents: EventMapping<CrashReportSenderError>?) {
         self.platform = platform
+        self.pixelEvents = pixelEvents
     }
 
-    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")
+
+        request.setValue(crcid, forHTTPHeaderField: CrashReportSender.httpHeaderCRCID)
+        Logger.general.debug("Configured crash report HTTP request with crcid: \(crcid ?? "nil")")
+
         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
+            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?
+                    if response.allHeaderFields[CrashReportSender.httpHeaderCRCID] == nil {
+                        let crashReportError = CrashReportSenderError.crcidMissing
+                        self.pixelEvents?.fire(crashReportError)
+                    }
+                } else {
+                    assertionFailure("CrashReportSender: Failed to send the crash report: \(response.statusCode)")
+                }
+
+                if let data {
+                    completion(.success(data), response)
+                } else if let error {
+                    let crashReportError = CrashReportSenderError.submissionFailed(response)
+                    self.pixelEvents?.fire(crashReportError)
+                    completion(.failure(error), response)
+                } else {
+                    let crashReportError = CrashReportSenderError.submissionFailed(response)
+                    self.pixelEvents?.fire(crashReportError)
+                    completion(.failure(crashReportError), response)
+                }
+            } else {
+                let crashReportError = CrashReportSenderError.submissionFailed(nil)
+                self.pixelEvents?.fire(crashReportError)
+                completion(.failure(crashReportError), nil)
+            }
         }
+        task.resume()
     }
 
-    private let session = URLSession(configuration: .ephemeral)
+    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))
+            }
+        }
+    }
 }

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?