Avoid race conditions between the home screen's crash alert and Sentry's onLastRunStatusDetermined

By converting the `lastCrashEventID` to a publisher that the VM can just listen to instead of relying on start to match a pre-set event

ElementX/Sources/Application/AppCoordinator.swift

@@ -1018,7 +1018,7 @@ class AppCoordinator: AppCoordinatorProtocol, AuthenticationFlowCoordinatorDeleg
         options.onLastRunStatusDetermined = { status, event in
             guard case .didCrash = status, let event else { return }
             MXLog.error("Sentry detected a crash in the previous run: \(event.eventId.sentryIdString)")
-            bugReportService.lastCrashEventID = event.eventId.sentryIdString
+            bugReportService.lastCrashEventIDSubject.send(event.eventId.sentryIdString)
         }
         
         // Mirror every crash into our own logs (which ship with rageshakes) before it's sent to Sentry.

ElementX/Sources/Mocks/BugReportServiceMock.swift

@@ -17,5 +17,6 @@ extension BugReportServiceMock {
         self.init()
         
         isEnabled = configuration.isEnabled
+        lastCrashEventIDSubject = .init(nil)
     }
 }

ElementX/Sources/Mocks/Generated/GeneratedMocks.swift

@@ -1809,12 +1809,11 @@ nonisolated class BugReportServiceMock: BugReportServiceProtocol, @unchecked Sen
         set(value) { underlyingIsEnabled = value }
     }
     nonisolated(unsafe) var underlyingIsEnabled: Bool!
-    var crashedLastRun: Bool {
+    var lastCrashEventIDSubject: CurrentValueSubject<String?, Never> {
-        get { return underlyingCrashedLastRun }
+        get { return underlyingLastCrashEventIDSubject }
-        set(value) { underlyingCrashedLastRun = value }
+        set(value) { underlyingLastCrashEventIDSubject = value }
     }
-    nonisolated(unsafe) var underlyingCrashedLastRun: Bool!
+    nonisolated(unsafe) var underlyingLastCrashEventIDSubject: CurrentValueSubject<String?, Never>!
-    nonisolated(unsafe) var lastCrashEventID: String?
 
     //MARK: - submitBugReport
 

ElementX/Sources/Screens/HomeScreen/HomeScreenCoordinator.swift

@@ -39,8 +39,6 @@ enum HomeScreenCoordinatorAction {
 
 final class HomeScreenCoordinator: CoordinatorProtocol {
     private var viewModel: HomeScreenViewModelProtocol
-    // periphery:ignore - only used in release builds
-    private let bugReportService: BugReportServiceProtocol
     
     private let actionsSubject: PassthroughSubject<HomeScreenCoordinatorAction, Never> = .init()
     private var cancellables = Set<AnyCancellable>()
@@ -54,9 +52,9 @@ final class HomeScreenCoordinator: CoordinatorProtocol {
                                         selectedRoomPublisher: parameters.selectedRoomPublisher,
                                         appSettings: parameters.appSettings,
                                         analyticsService: parameters.analyticsService,
+                                        bugReportService: parameters.bugReportService,
                                         notificationManager: parameters.notificationManager,
                                         userIndicatorController: parameters.userIndicatorController)
-        bugReportService = parameters.bugReportService
         
         viewModel.actions
             .sink { [weak self] action in
@@ -100,16 +98,6 @@ final class HomeScreenCoordinator: CoordinatorProtocol {
     
     // MARK: - Public
     
-    func start() {
-        #if !DEBUG
-        // Note: bugReportService.isEnabled doesn't determine if a user has opted in to Analytics/Sentry.
-        // Therefore we use lastCrashEventID as this will only be set if we have crash ID from Sentry.
-        if bugReportService.crashedLastRun, bugReportService.lastCrashEventID != nil {
-            viewModel.presentCrashedLastRunAlert()
-        }
-        #endif
-    }
-    
     func toPresentable() -> AnyView {
         AnyView(HomeScreen(context: viewModel.context))
     }

ElementX/Sources/Screens/HomeScreen/HomeScreenViewModel.swift

@@ -17,6 +17,7 @@ class HomeScreenViewModel: HomeScreenViewModelType, HomeScreenViewModelProtocol
     private let userSession: UserSessionProtocol
     private let spaceFilterSubject: CurrentValueSubject<SpaceServiceFilter?, Never>
     private let analyticsService: AnalyticsServiceProtocol
+    private let bugReportService: BugReportServiceProtocol
     private let appSettings: AppSettings
     private let notificationManager: NotificationManagerProtocol
     private let userIndicatorController: UserIndicatorControllerProtocol
@@ -33,10 +34,12 @@ class HomeScreenViewModel: HomeScreenViewModelType, HomeScreenViewModelProtocol
          selectedRoomPublisher: CurrentValuePublisher<String?, Never>,
          appSettings: AppSettings,
          analyticsService: AnalyticsServiceProtocol,
+         bugReportService: BugReportServiceProtocol,
          notificationManager: NotificationManagerProtocol,
          userIndicatorController: UserIndicatorControllerProtocol) {
         self.userSession = userSession
         self.analyticsService = analyticsService
+        self.bugReportService = bugReportService
         self.appSettings = appSettings
         self.notificationManager = notificationManager
         self.userIndicatorController = userIndicatorController
@@ -141,6 +144,15 @@ class HomeScreenViewModel: HomeScreenViewModelType, HomeScreenViewModelProtocol
             .weakAssign(to: \.state.selectedSpaceFilter, on: self)
             .store(in: &cancellables)
         
+        bugReportService.lastCrashEventIDSubject
+            .compactMap { $0 }
+            .first()
+            .receive(on: DispatchQueue.main)
+            .sink { [weak self] _ in
+                self?.presentCrashedLastRunAlert()
+            }
+            .store(in: &cancellables)
+        
         Task {
             state.reportRoomEnabled = await userSession.clientProxy.isReportRoomSupported
         }

ElementX/Sources/Screens/HomeScreen/View/HomeScreen.swift

@@ -220,6 +220,7 @@ struct HomeScreen_Previews: PreviewProvider, TestablePreview {
                                    selectedRoomPublisher: CurrentValueSubject<String?, Never>(nil).asCurrentValuePublisher(),
                                    appSettings: .volatile(),
                                    analyticsService: AnalyticsServiceMock(.init()),
+                                   bugReportService: BugReportServiceMock(.init()),
                                    notificationManager: NotificationManagerMock(),
                                    userIndicatorController: UserIndicatorControllerMock())
     }

ElementX/Sources/Screens/HomeScreen/View/HomeScreenEmptyStateView.swift

@@ -147,6 +147,7 @@ struct HomeScreenEmptyStateView_Previews: PreviewProvider, TestablePreview {
                                    selectedRoomPublisher: CurrentValueSubject<String?, Never>(nil).asCurrentValuePublisher(),
                                    appSettings: .volatile(),
                                    analyticsService: AnalyticsServiceMock(.init()),
+                                   bugReportService: BugReportServiceMock(.init()),
                                    notificationManager: NotificationManagerMock(),
                                    userIndicatorController: UserIndicatorControllerMock())
     }()

ElementX/Sources/Screens/HomeScreen/View/HomeScreenInviteCell.swift

@@ -203,6 +203,7 @@ struct HomeScreenInviteCell_Previews: PreviewProvider, TestablePreview {
                                    selectedRoomPublisher: CurrentValueSubject<String?, Never>(nil).asCurrentValuePublisher(),
                                    appSettings: .volatile(),
                                    analyticsService: AnalyticsServiceMock(.init()),
+                                   bugReportService: BugReportServiceMock(.init()),
                                    notificationManager: NotificationManagerMock(),
                                    userIndicatorController: UserIndicatorControllerMock())
     }

ElementX/Sources/Screens/HomeScreen/View/HomeScreenKnockedCell.swift

@@ -135,6 +135,7 @@ struct HomeScreenKnockedCell_Previews: PreviewProvider, TestablePreview {
                                    selectedRoomPublisher: CurrentValueSubject<String?, Never>(nil).asCurrentValuePublisher(),
                                    appSettings: .volatile(),
                                    analyticsService: AnalyticsServiceMock(.init()),
+                                   bugReportService: BugReportServiceMock(.init()),
                                    notificationManager: NotificationManagerMock(),
                                    userIndicatorController: UserIndicatorControllerMock())
     }

ElementX/Sources/Screens/HomeScreen/View/HomeScreenRecoveryKeyConfirmationBanner.swift

@@ -124,6 +124,7 @@ struct HomeScreenRecoveryKeyConfirmationBanner_Previews: PreviewProvider, Testab
                                    selectedRoomPublisher: CurrentValueSubject<String?, Never>(nil).asCurrentValuePublisher(),
                                    appSettings: .volatile(),
                                    analyticsService: AnalyticsServiceMock(.init()),
+                                   bugReportService: BugReportServiceMock(.init()),
                                    notificationManager: NotificationManagerMock(),
                                    userIndicatorController: UserIndicatorControllerMock())
     }

ElementX/Sources/Services/BugReport/BugReportService.swift

@@ -26,7 +26,7 @@ class BugReportService: NSObject, BugReportServiceProtocol {
         rageshakeURL != .disabled
     }
     
-    var lastCrashEventID: String?
+    let lastCrashEventIDSubject = CurrentValueSubject<String?, Never>(nil)
     
     init(rageshakeURLPublisher: CurrentValuePublisher<RageshakeConfiguration, Never>,
          applicationID: String,
@@ -48,10 +48,6 @@ class BugReportService: NSObject, BugReportServiceProtocol {
     
     // MARK: - BugReportServiceProtocol
     
-    var crashedLastRun: Bool {
-        SentrySDK.lastRunStatus == .didCrash
-    }
-    
     // swiftlint:disable:next cyclomatic_complexity
     func submitBugReport(_ bugReport: BugReport,
                          progressListener: CurrentValueSubject<Double, Never>) async -> Result<SubmitBugReportResponse, BugReportServiceError> {
@@ -81,7 +77,7 @@ class BugReportService: NSObject, BugReportServiceProtocol {
             params.append(.init(key: "device_keys", type: .text(value: compactKeys)))
         }
         
-        if let crashEventID = lastCrashEventID {
+        if let crashEventID = lastCrashEventIDSubject.value {
             params.append(MultipartFormData(key: "crash_report", type: .text(value: "<https://sentry.tools.element.io/organizations/element/issues/?project=44&query=\(crashEventID)>")))
             bugReport.githubLabels.append("crash")
         }
@@ -155,7 +151,7 @@ class BugReportService: NSObject, BugReportServiceProtocol {
             let decoder = JSONDecoder()
             let uploadResponse = try decoder.decode(SubmitBugReportResponse.self, from: data)
             
-            lastCrashEventID = nil
+            lastCrashEventIDSubject.send(nil)
             
             MXLog.info("Feedback submitted.")
             

ElementX/Sources/Services/BugReport/BugReportServiceProtocol.swift

@@ -50,9 +50,8 @@ enum BugReportServiceError: LocalizedError {
 // sourcery: AutoMockable
 protocol BugReportServiceProtocol: AnyObject {
     var isEnabled: Bool { get }
-    var crashedLastRun: Bool { get }
     
-    var lastCrashEventID: String? { get set }
+    var lastCrashEventIDSubject: CurrentValueSubject<String?, Never> { get }
     
     func submitBugReport(_ bugReport: BugReport,
                          progressListener: CurrentValueSubject<Double, Never>) async -> Result<SubmitBugReportResponse, BugReportServiceError>

UnitTests/Sources/BugReportServiceTests.swift

@@ -21,7 +21,7 @@ final class BugReportServiceTests {
         appSettings.bugReportRageshakeURL.reset()
         
         let bugReportServiceMock = BugReportServiceMock()
-        bugReportServiceMock.crashedLastRun = false
+        bugReportServiceMock.lastCrashEventIDSubject = .init(nil)
         bugReportServiceMock.submitBugReportProgressListenerReturnValue = .success(SubmitBugReportResponse(reportURL: "https://www.example.com/123"))
         bugReportService = bugReportServiceMock
     }
@@ -32,7 +32,7 @@ final class BugReportServiceTests {
     
     @Test
     func initialStateWithMockService() {
-        #expect(!bugReportService.crashedLastRun)
+        #expect(bugReportService.lastCrashEventIDSubject.value == nil)
     }
     
     @Test
@@ -62,7 +62,7 @@ final class BugReportServiceTests {
                                        session: .mock,
                                        appHooks: AppHooks())
         #expect(service.isEnabled)
-        #expect(!service.crashedLastRun)
+        #expect(bugReportService.lastCrashEventIDSubject.value == nil)
     }
     
     @Test
@@ -74,7 +74,7 @@ final class BugReportServiceTests {
                                        session: .mock,
                                        appHooks: AppHooks())
         #expect(!service.isEnabled)
-        #expect(!service.crashedLastRun)
+        #expect(bugReportService.lastCrashEventIDSubject.value == nil)
     }
     
     @Test

UnitTests/Sources/HomeScreenViewModelTests.swift

@@ -432,6 +432,7 @@ final class HomeScreenViewModelTests {
                                         selectedRoomPublisher: CurrentValueSubject<String?, Never>(nil).asCurrentValuePublisher(),
                                         appSettings: appSettings,
                                         analyticsService: AnalyticsServiceMock(.init()),
+                                        bugReportService: BugReportServiceMock(.init()),
                                         notificationManager: notificationManager,
                                         userIndicatorController: UserIndicatorControllerMock())
     }