Update Sessions to send events when FIID empty (#11161)

Author: samedson

FirebaseSessions/Sources/SessionCoordinator.swift

@@ -39,22 +39,39 @@ class SessionCoordinator: SessionCoordinatorProtocol {
                                   callback: @escaping (Result<Void, FirebaseSessionsError>)
                                     -> Void) {
     /// Order of execution
-    /// 1. Fetch the installations Id. If successful, move to 3. Else, drop sending the event.
+    /// 1. Fetch the installations Id. Regardless of success, move to step 2
     /// 2. Log the event. If successful, all is good. Else, log the message with error.
-    installations.installationID { result in
-      switch result {
-      case let .success(fiid):
-        event.setInstallationID(installationId: fiid)
-        self.fireLogger.logEvent(event: event) { logResult in
-          switch logResult {
-          case .success():
-            Logger.logDebug("Successfully logged Session Start event to GoogleDataTransport")
+    /// 3. If there was a FireLog error, expose it to the callback. Otherwise expose the FIID
+    /// error if it exists. Otherwise, success.
+    fillInFIID(event: event) { fiidResult in
+      self.fireLogger.logEvent(event: event) { logResult in
+        switch logResult {
+        case .success():
+          Logger.logDebug("Successfully logged Session Start event to GoogleDataTransport")
+
+          switch fiidResult {
+          case .success(()):
             callback(.success(()))
           case let .failure(error):
-            callback(.failure(FirebaseSessionsError.DataTransportError(error)))
+            callback(.failure(error))
           }
+        case let .failure(error):
+          callback(.failure(FirebaseSessionsError.DataTransportError(error)))
         }
+      }
+    }
+  }
+
+  private func fillInFIID(event: SessionStartEvent,
+                          callback: @escaping (Result<Void, FirebaseSessionsError>)
+                            -> Void) {
+    installations.installationID { result in
+      switch result {
+      case let .success(fiid):
+        event.setInstallationID(installationId: fiid)
+        callback(.success(()))
       case let .failure(error):
+        event.setInstallationID(installationId: "")
         callback(.failure(FirebaseSessionsError.SessionInstallationsError(error)))
       }
     }

FirebaseSessions/Tests/Unit/SessionCoordinatorTests.swift

@@ -111,7 +111,43 @@ class SessionCoordinatorTests: XCTestCase {
     }
 
     // We should have logged the event, but with a failed result
-    XCTAssertNil(fireLogger.loggedEvent)
+    XCTAssertNotNil(fireLogger.loggedEvent)
+    XCTAssertFalse(resultSuccess)
+  }
+
+  func test_attemptLoggingSessionStart_handlesGDTAndInstallationsError() throws {
+    let fireLogError = NSError(domain: "DataTransportError", code: -2)
+    fireLogger.result = .failure(fireLogError)
+    installations
+      .result = .failure(FirebaseSessionsError
+        .SessionInstallationsError(NSError(domain: "TestInstallationsError", code: -1)))
+
+    let event = SessionStartEvent(sessionInfo: defaultSessionInfo, appInfo: appInfo, time: time)
+
+    // Start success so it must be set to false
+    var resultSuccess = true
+    coordinator.attemptLoggingSessionStart(event: event) { result in
+      switch result {
+      case .success(()):
+        resultSuccess = true
+      case let .failure(err):
+        resultSuccess = false
+        // Result should use the FireLog error if there's an error in both
+        // Installations and FireLog
+        XCTAssertEqual(err, FirebaseSessionsError.DataTransportError(fireLogError))
+      }
+    }
+
+    // Make sure we've set the Installation ID to empty because the FIID
+    // fetch failed
+    assertEqualProtoString(
+      event.proto.session_data.firebase_installation_id,
+      expected: "",
+      fieldName: "installation_id"
+    )
+
+    // We should have logged the event, but with a failed result
+    XCTAssertEqual(fireLogger.loggedEvent, event)
     XCTAssertFalse(resultSuccess)
   }
 }