From 8ddef58df8af9af958ab036bcec8987fcac51044 Mon Sep 17 00:00:00 2001 From: Sasha Weiss Date: Wed, 20 May 2026 15:04:17 -0700 Subject: [PATCH] Stop logging Backup frames on error --- Signal.xcodeproj/project.pbxproj | 20 +- Signal/AppLaunch/AppDelegate.swift | 1 - .../InternalSettingsViewController.swift | 13 +- .../Linked Devices/LinkedDevicesView.swift | 3 - ...upArchiveInternalErrorViewController.swift | 299 ------------------ .../ChatListFYISheetCoordinator.swift | 50 +++ .../Chat List/ChatListViewController.swift | 8 +- SignalNSE/NSEEnvironment.swift | 1 - .../Archivers/BackupArchive+Errors.swift | 72 +---- .../Chat/BackupArchiveChatStyleArchiver.swift | 7 +- .../BackupArchiveErrorPresenter.swift | 61 ---- .../Archiving/BackupArchiveErrorStore.swift | 34 ++ .../Archiving/BackupArchiveManagerImpl.swift | 189 +++++------ SignalServiceKit/Environment/AppSetup.swift | 8 +- .../Environment/DependenciesBridge.swift | 3 - .../TestUtils/MockSSKEnvironment.swift | 1 - .../ShareViewController.swift | 1 - 17 files changed, 187 insertions(+), 584 deletions(-) delete mode 100644 Signal/src/ViewControllers/BackupArchive/BackupArchiveInternalErrorViewController.swift delete mode 100644 SignalServiceKit/Backups/Archiving/BackupArchiveErrorPresenter.swift create mode 100644 SignalServiceKit/Backups/Archiving/BackupArchiveErrorStore.swift diff --git a/Signal.xcodeproj/project.pbxproj b/Signal.xcodeproj/project.pbxproj index c5abe52989..0c70e4f273 100644 --- a/Signal.xcodeproj/project.pbxproj +++ b/Signal.xcodeproj/project.pbxproj @@ -1015,8 +1015,7 @@ 6646573F2AC3B9190099DE1C /* MockRegistrationStateChangeManager.swift in Sources */ = {isa = PBXBuildFile; fileRef = 6646573E2AC3B9190099DE1C /* MockRegistrationStateChangeManager.swift */; }; 664657412AC4FB720099DE1C /* NotificationPresenter.swift in Sources */ = {isa = PBXBuildFile; fileRef = 664657402AC4FB720099DE1C /* NotificationPresenter.swift */; }; 664657472ACB66630099DE1C /* TSAccountManagerObjcBridge.swift in Sources */ = {isa = PBXBuildFile; fileRef = 664657462ACB66630099DE1C /* TSAccountManagerObjcBridge.swift */; }; - 66485EB02CCC515A00B8613F /* BackupArchiveInternalErrorViewController.swift in Sources */ = {isa = PBXBuildFile; fileRef = 66485EAF2CCC50FA00B8613F /* BackupArchiveInternalErrorViewController.swift */; }; - 66485EB32CD03F6400B8613F /* BackupArchiveErrorPresenter.swift in Sources */ = {isa = PBXBuildFile; fileRef = 66485EB22CD03F5D00B8613F /* BackupArchiveErrorPresenter.swift */; }; + 66485EB32CD03F6400B8613F /* BackupArchiveErrorStore.swift in Sources */ = {isa = PBXBuildFile; fileRef = 66485EB22CD03F5D00B8613F /* BackupArchiveErrorStore.swift */; }; 66485EB92CD17D6400B8613F /* DbRollbackTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 66485EB82CD17D5D00B8613F /* DbRollbackTests.swift */; }; 6649651C2BDC6EAD00E2DE98 /* AVAsset+Attachment.swift in Sources */ = {isa = PBXBuildFile; fileRef = 6649651B2BDC6EAD00E2DE98 /* AVAsset+Attachment.swift */; }; 6649651E2BDF169F00E2DE98 /* UIImage+Attachment.swift in Sources */ = {isa = PBXBuildFile; fileRef = 6649651D2BDF169F00E2DE98 /* UIImage+Attachment.swift */; }; @@ -5284,8 +5283,7 @@ 6646573E2AC3B9190099DE1C /* MockRegistrationStateChangeManager.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = MockRegistrationStateChangeManager.swift; sourceTree = ""; }; 664657402AC4FB720099DE1C /* NotificationPresenter.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = NotificationPresenter.swift; sourceTree = ""; }; 664657462ACB66630099DE1C /* TSAccountManagerObjcBridge.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = TSAccountManagerObjcBridge.swift; sourceTree = ""; }; - 66485EAF2CCC50FA00B8613F /* BackupArchiveInternalErrorViewController.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = BackupArchiveInternalErrorViewController.swift; sourceTree = ""; }; - 66485EB22CD03F5D00B8613F /* BackupArchiveErrorPresenter.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = BackupArchiveErrorPresenter.swift; sourceTree = ""; }; + 66485EB22CD03F5D00B8613F /* BackupArchiveErrorStore.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = BackupArchiveErrorStore.swift; sourceTree = ""; }; 66485EB82CD17D5D00B8613F /* DbRollbackTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = DbRollbackTests.swift; sourceTree = ""; }; 6649651B2BDC6EAD00E2DE98 /* AVAsset+Attachment.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = "AVAsset+Attachment.swift"; sourceTree = ""; }; 6649651D2BDF169F00E2DE98 /* UIImage+Attachment.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = "UIImage+Attachment.swift"; sourceTree = ""; }; @@ -9361,7 +9359,6 @@ 340FC87A204DAC8C007AEB0F /* AppSettings */, 8809CE8822F93C0D00D38867 /* Attachment Keyboard */, 883A7FC1269F4BE700841DF9 /* Avatars */, - 66485EB12CD03F3300B8613F /* BackupArchive */, 342FFE6C271EF580000AC89F /* Categories */, F0B872B4269CF01E00D26481 /* ContextMenus */, 34D8C0221ED3673300188D7C /* DebugUI */, @@ -10248,14 +10245,6 @@ path = RegistrationStateChangeManager; sourceTree = ""; }; - 66485EB12CD03F3300B8613F /* BackupArchive */ = { - isa = PBXGroup; - children = ( - 66485EAF2CCC50FA00B8613F /* BackupArchiveInternalErrorViewController.swift */, - ); - path = BackupArchive; - sourceTree = ""; - }; 6649651A2BDC6E8D00E2DE98 /* Playback */ = { isa = PBXGroup; children = ( @@ -13686,7 +13675,7 @@ 665C0D5F2ADF57D000539A37 /* BackupArchive+Shims.swift */, 661429182D35B9EA0043AA22 /* BackupArchive+Timestamp.swift */, 04BC94D12E061D7500446C52 /* BackupArchiveAttachmentByteCounter.swift */, - 66485EB22CD03F5D00B8613F /* BackupArchiveErrorPresenter.swift */, + 66485EB22CD03F5D00B8613F /* BackupArchiveErrorStore.swift */, 66232AE02CC0271F00AE6A76 /* BackupArchiveFullTextSearchIndexer.swift */, 665C0D5B2ADF538100539A37 /* BackupArchiveManager.swift */, 665C0D5D2ADF53E200539A37 /* BackupArchiveManagerImpl.swift */, @@ -18090,7 +18079,6 @@ 4C2F454F214C00E1004871FF /* AvatarTableViewCell.swift in Sources */, 32C584A825B81C6600256804 /* AvatarViewController.swift in Sources */, B95A765C2B76C5BB00AA7E97 /* AvatarViewPresentationContextProvider.swift in Sources */, - 66485EB02CCC515A00B8613F /* BackupArchiveInternalErrorViewController.swift in Sources */, D932C0EB2E13AD3F00FEF9C3 /* BackupAttachmentDownloadTracker.swift in Sources */, D93964B62E038C7B00094117 /* BackupAttachmentUploadTracker.swift in Sources */, 66A1F4E62E03641D0095DE4B /* BackupBGProcessingTaskRunner.swift in Sources */, @@ -19015,7 +19003,7 @@ 66F6D69E2C77E4C500EFAF75 /* BackupArchiveContactAttachmentArchiver.swift in Sources */, 66CD256E2B06E14F00139E17 /* BackupArchiveContactRecipientArchiver.swift in Sources */, C1CA5F8E2BE2F21C00D733CA /* BackupArchiveDistributionListRecipientArchiver.swift in Sources */, - 66485EB32CD03F6400B8613F /* BackupArchiveErrorPresenter.swift in Sources */, + 66485EB32CD03F6400B8613F /* BackupArchiveErrorStore.swift in Sources */, D91D9C8C2C3F06400009E4F7 /* BackupArchiveExpirationTimerChatUpdateArchiver.swift in Sources */, 66232AE12CC0272900AE6A76 /* BackupArchiveFullTextSearchIndexer.swift in Sources */, D9A85DC22BE1719C003F7045 /* BackupArchiveGroupCallArchiver.swift in Sources */, diff --git a/Signal/AppLaunch/AppDelegate.swift b/Signal/AppLaunch/AppDelegate.swift index 231fcf43ae..175ba7d6ed 100644 --- a/Signal/AppLaunch/AppDelegate.swift +++ b/Signal/AppLaunch/AppDelegate.swift @@ -400,7 +400,6 @@ final class AppDelegate: UIResponder, UIApplicationDelegate { let dataMigrationContinuation = globalsContinuation.initGlobals( appContext: launchContext.appContext, appReadiness: appReadiness, - backupArchiveErrorPresenterFactory: BackupArchiveErrorPresenterFactoryInternal(), deviceBatteryLevelManager: DeviceBatteryLevelManagerImpl(), deviceSleepManager: launchContext.deviceSleepManager, paymentsEvents: PaymentsEventsMainApp(), diff --git a/Signal/src/ViewControllers/AppSettings/Internal/InternalSettingsViewController.swift b/Signal/src/ViewControllers/AppSettings/Internal/InternalSettingsViewController.swift index 2f1f27bb75..7454a51482 100644 --- a/Signal/src/ViewControllers/AppSettings/Internal/InternalSettingsViewController.swift +++ b/Signal/src/ViewControllers/AppSettings/Internal/InternalSettingsViewController.swift @@ -409,15 +409,8 @@ private extension InternalSettingsViewController { let accountKeyStore = DependenciesBridge.shared.accountKeyStore let backupArchiveManager = DependenciesBridge.shared.backupArchiveManager let db = DependenciesBridge.shared.db - let errorPresenter = DependenciesBridge.shared.backupArchiveErrorPresenter let tsAccountManager = DependenciesBridge.shared.tsAccountManager - func presentBackupErrorsAndToast(_ message: String) { - errorPresenter.presentOverTopmostViewController { [self] in - presentToast(text: message) - } - } - let backupKey: MessageBackupKey let exportMetadata: Upload.EncryptedBackupUploadMetadata do { @@ -450,7 +443,7 @@ private extension InternalSettingsViewController { } catch { let message = "Failed to export Backup proto! \(error)" Logger.error(message) - presentBackupErrorsAndToast(message) + presentToast(text: message) return } @@ -462,13 +455,13 @@ private extension InternalSettingsViewController { applicationActivities: nil, ) activityVC.popoverPresentationController?.sourceView = view - activityVC.completionWithItemsHandler = { _, _, _, _ in + activityVC.completionWithItemsHandler = { [self] _, _, _, _ in UIPasteboard.general.setItems( [[UIPasteboard.typeAutomatic: keyString]], options: [.expirationDate: Date().addingTimeInterval(120)], ) - presentBackupErrorsAndToast("Success! Encryption key copied.") + presentToast(text: "Success! Encryption key copied.") } present(activityVC, animated: true) diff --git a/Signal/src/ViewControllers/AppSettings/Linked Devices/LinkedDevicesView.swift b/Signal/src/ViewControllers/AppSettings/Linked Devices/LinkedDevicesView.swift index d37fe6a681..03f81c9c26 100644 --- a/Signal/src/ViewControllers/AppSettings/Linked Devices/LinkedDevicesView.swift +++ b/Signal/src/ViewControllers/AppSettings/Linked Devices/LinkedDevicesView.swift @@ -54,7 +54,6 @@ class LinkedDevicesViewModel: ObservableObject { private var deviceIdToIgnore: DeviceId? fileprivate var shouldShowFinishLinkingSheet = false - private let backupArchiveErrorPresenter: BackupArchiveErrorPresenter private let databaseChangeObserver: DatabaseChangeObserver private let db: any DB private let deviceService: OWSDeviceService @@ -69,7 +68,6 @@ class LinkedDevicesViewModel: ObservableObject { #if DEBUG self.isPreview = isPreview #endif - backupArchiveErrorPresenter = DependenciesBridge.shared.backupArchiveErrorPresenter databaseChangeObserver = DependenciesBridge.shared.databaseChangeObserver db = DependenciesBridge.shared.db deviceService = DependenciesBridge.shared.deviceService @@ -655,7 +653,6 @@ class LinkedDevicesHostingController: HostingContainer { actionSheet.onDismiss = { [weak self] in self?.viewModel.expectMoreDevices() - DependenciesBridge.shared.backupArchiveErrorPresenter.presentOverTopmostViewController(completion: {}) } presentActionSheet(actionSheet) } diff --git a/Signal/src/ViewControllers/BackupArchive/BackupArchiveInternalErrorViewController.swift b/Signal/src/ViewControllers/BackupArchive/BackupArchiveInternalErrorViewController.swift deleted file mode 100644 index 044f720ddd..0000000000 --- a/Signal/src/ViewControllers/BackupArchive/BackupArchiveInternalErrorViewController.swift +++ /dev/null @@ -1,299 +0,0 @@ -// -// Copyright 2024 Signal Messenger, LLC -// SPDX-License-Identifier: AGPL-3.0-only -// - -import LibSignalClient -import SignalServiceKit -import SignalUI -import UIKit - -/// For internal (nightly) use only. Produces BackupArchiveErrorPresenterInternal. -class BackupArchiveErrorPresenterFactoryInternal: BackupArchiveErrorPresenterFactory { - func build( - db: any DB, - tsAccountManager: TSAccountManager, - ) -> BackupArchiveErrorPresenter { - return BackupArchiveErrorPresenterInternal( - db: db, - tsAccountManager: tsAccountManager, - ) - } -} - -/// For internal (nightly) use only. Presents BackupArchiveInternalErrorViewController when backups emits errors. -class BackupArchiveErrorPresenterInternal: BackupArchiveErrorPresenter { - - private let db: any DB - private let tsAccountManager: TSAccountManager - - private let kvStore: KeyValueStore - - private static let stringifiedErrorsKey = "stringifiedErrors" - private static let validationErrorKey = "validationError" - private static let hadFatalErrorKey = "hadFatalError" - private static let hasBeenDisplayedKey = "hasBeenDisplayed" - - init( - db: any DB, - tsAccountManager: TSAccountManager, - ) { - self.db = db - self.tsAccountManager = tsAccountManager - self.kvStore = KeyValueStore(collection: "BackupArchiveErrorPresenterImpl") - } - - func persistErrors( - _ errors: [SignalServiceKit.BackupArchive.CollapsedErrorLog], - didFail: Bool, - tx outerTx: DBWriteTransaction, - ) { - guard BuildFlags.Backups.archiveErrorDisplay else { - return - } - - if errors.isEmpty { - return - } - - let stringified = errors - .map { - var text = ($0.typeLogString) + "\n" - + "Were frames dropped? \($0.wasFrameDropped)\n" - + "Repeated \($0.errorCount) times\n" - + "Example callsite: \($0.exampleCallsiteString)" - if let exampleProtoFrameJson = $0.exampleProtoFrameJson { - text.append("\nProto:\n\(exampleProtoFrameJson)") - } - return text - } - .joined(separator: "\n-------------------\n") - - // The outer transaction might get rolled back because of these very errors. - // At the risk of losing these errors in a crash (this is internal only, its fine) - // do the actual write in a separate transaction (that happens synchronously) - // so it is never rolled back. - outerTx.addSyncCompletion { [weak self] in - Task { - guard let self else { return } - - await self.db.awaitableWrite { innerTx in - self.kvStore.setString(stringified, key: Self.stringifiedErrorsKey, transaction: innerTx) - self.kvStore.setBool(didFail, key: Self.hadFatalErrorKey, transaction: innerTx) - self.kvStore.setBool(false, key: Self.hasBeenDisplayedKey, transaction: innerTx) - } - } - } - } - - func persistValidationError(_ error: MessageBackupValidationError) async { - await self.db.awaitableWrite { tx in - self.kvStore.setString(error.errorMessage, key: Self.validationErrorKey, transaction: tx) - // Validator errors are fatal failures - self.kvStore.setBool(true, key: Self.hadFatalErrorKey, transaction: tx) - self.kvStore.setBool(false, key: Self.hasBeenDisplayedKey, transaction: tx) - } - } - - func presentOverTopmostViewController(completion: @escaping () -> Void) { - guard BuildFlags.Backups.archiveErrorDisplay else { - completion() - return - } - let isRegistered = tsAccountManager.registrationStateWithMaybeSneakyTransaction.isRegistered - let errorString: String? - let hadFatalError: Bool - let validationErrorString: String? - (errorString, hadFatalError, validationErrorString) = db.write { tx in - let hadFatalError = kvStore.getBool(Self.hadFatalErrorKey, defaultValue: false, transaction: tx) - if kvStore.getBool(Self.hasBeenDisplayedKey, defaultValue: false, transaction: tx) { - return (nil, hadFatalError, nil) - } - let errorString = kvStore.getString(Self.stringifiedErrorsKey, transaction: tx) - let validationErrorString = self.kvStore.getString(Self.validationErrorKey, transaction: tx) - kvStore.setBool(true, key: Self.hasBeenDisplayedKey, transaction: tx) - kvStore.setString(nil, key: Self.stringifiedErrorsKey, transaction: tx) - kvStore.setString(nil, key: Self.validationErrorKey, transaction: tx) - return (errorString, hadFatalError, validationErrorString) - } - guard errorString != nil || validationErrorString != nil else { - completion() - return - } - - let vc = BackupArchiveInternalErrorViewController( - errorString: errorString, - hadFatalError: hadFatalError, - validationErrorString: validationErrorString, - isRegistered: isRegistered, - completion: completion, - ) - let navVc = OWSNavigationController(rootViewController: vc) - UIApplication.shared.frontmostViewController?.present(navVc, animated: true) - } -} - -private class BackupArchiveInternalErrorViewController: OWSViewController { - - // MARK: - Properties - - private let originalText: String - private let completion: (() -> Void)? - - var textView: UITextView! - let isRegistered: Bool - let footer = UIToolbar.clear() - - // MARK: Initializers - - fileprivate init( - errorString: String?, - hadFatalError: Bool, - validationErrorString: String?, - isRegistered: Bool, - completion: (() -> Void)?, - ) { - var text: String - if hadFatalError { - text = "!!!Backup import or export FAILED!!!" - } else { - text = "Backup import or export SUCCESS!!!" - text.append("\n\nSUCCESS SUCCESS SUCCESS\n\n") - } - text.append(""" - \n\nPlease send the errors below to your nearest iOS dev.\n - Feel free to edit to remove any private info before sending.\n\n - """) - - text.append("\n" + AppVersionImpl.shared.currentAppVersion4.debugDescription + "\n") - - if let errorString, let validationErrorString { - text.append("Hit both iOS and validator errors\n\n") - text.append("------Validator error------\n") - text.append(validationErrorString) - text.append("\n\n------iOS errors------\n") - text.append(errorString) - } else if let errorString { - text.append(errorString) - } else if let validationErrorString { - text.append("------Validator error------\n") - text.append(validationErrorString) - } - self.originalText = text - self.isRegistered = isRegistered - self.completion = completion - super.init() - } - - // MARK: View Lifecycle - - override func viewDidLoad() { - super.viewDidLoad() - - navigationItem.title = "Backup errors" - - createViews() - - self.textView.contentOffset = CGPoint(x: 0, y: self.textView.contentInset.top) - } - - override func viewDidDisappear(_ animated: Bool) { - super.viewDidDisappear(animated) - - completion?() - } - - override func themeDidChange() { - super.themeDidChange() - - loadContent() - } - - func loadContent() { - view.backgroundColor = Theme.backgroundColor - textView.backgroundColor = Theme.backgroundColor - textView.textColor = Theme.primaryTextColor - footer.tintColor = Theme.primaryIconColor - } - - // MARK: - Create Views - - private func createViews() { - view.backgroundColor = Theme.backgroundColor - - let textView = OWSTextView() - self.textView = textView - textView.font = UIFont.dynamicTypeBody - textView.backgroundColor = Theme.backgroundColor - textView.isOpaque = true - textView.isEditable = true - textView.isSelectable = true - textView.isScrollEnabled = true - textView.showsHorizontalScrollIndicator = false - textView.showsVerticalScrollIndicator = true - textView.isUserInteractionEnabled = true - textView.textColor = Theme.primaryTextColor - textView.text = originalText - - view.addSubview(textView) - textView.autoPinEdge(toSuperviewEdge: .top) - textView.autoPinEdge(toSuperviewEdge: .leading) - textView.autoPinEdge(toSuperviewEdge: .trailing) - textView.textContainerInset = UIEdgeInsets(top: 0, leading: 16, bottom: 0, trailing: 16) - - view.addSubview(footer) - footer.autoPinWidthToSuperview() - footer.autoPinEdge(.top, to: .bottom, of: textView) - footer.autoPin(toBottomLayoutGuideOf: self, withInset: 0) - footer.tintColor = Theme.primaryIconColor - - var footerItems = [ - UIBarButtonItem( - image: Theme.iconImage(.buttonShare), - style: .plain, - target: self, - action: #selector(shareButtonPressed), - ), - .flexibleSpace(), - ] - if isRegistered { - footerItems.append(.button(icon: .buttonForward, style: .plain) { [weak self] in - self?.sendAsMessage() - }) - } - footer.items = footerItems - - loadContent() - } - - // MARK: - Actions - - @objc - private func shareButtonPressed(_ sender: UIBarButtonItem) { - AttachmentSharing.showShareUI(for: textView.text, sender: sender) - } - - private func sendAsMessage() { - ForwardMessageViewController.present( - forMessageBody: .init(text: textView.text, ranges: .empty), - from: self, - delegate: self, - ) - } -} - -extension BackupArchiveInternalErrorViewController: ForwardMessageDelegate { - public func forwardMessageFlowDidComplete(items: [ForwardMessageItem], recipientThreads: [TSThread]) { - dismiss(animated: true) { - ForwardMessageViewController.finalizeForward( - items: items, - recipientThreads: recipientThreads, - fromViewController: self, - ) - } - } - - public func forwardMessageFlowDidCancel() { - dismiss(animated: true) - } -} diff --git a/Signal/src/ViewControllers/HomeView/Chat List/ChatListFYISheetCoordinator.swift b/Signal/src/ViewControllers/HomeView/Chat List/ChatListFYISheetCoordinator.swift index a60a205d13..d1c5e63100 100644 --- a/Signal/src/ViewControllers/HomeView/Chat List/ChatListFYISheetCoordinator.swift +++ b/Signal/src/ViewControllers/HomeView/Chat List/ChatListFYISheetCoordinator.swift @@ -44,6 +44,8 @@ class ChatListFYISheetCoordinator { let timestampMs: UInt64 } + struct BackupArchiveError {} + case badgeThanks(BadgeThanks) case badgeIssue(BadgeIssue) case badgeExpiration(BadgeExpiration) @@ -51,8 +53,10 @@ class ChatListFYISheetCoordinator { case backupSubscriptionFailedToRenew(BackupSubscriptionFailedToRenew) case keyTransparencySelfCheckFailed(KeyTransparencySelfCheckFailed) case smsVerificationCodeSent(SMSVerificationCodeSent) + case backupArchiveError(BackupArchiveError) } + private let backupArchiveErrorStore: BackupArchiveErrorStore private let backupExportJobRunner: BackupExportJobRunner private let backupSubscriptionIssueStore: BackupSubscriptionIssueStore private let donationReceiptCredentialResultStore: DonationReceiptCredentialResultStore @@ -63,6 +67,7 @@ class ChatListFYISheetCoordinator { private let profileManager: ProfileManager init( + backupArchiveErrorStore: BackupArchiveErrorStore, backupExportJobRunner: BackupExportJobRunner, backupSubscriptionIssueStore: BackupSubscriptionIssueStore, donationReceiptCredentialResultStore: DonationReceiptCredentialResultStore, @@ -72,6 +77,7 @@ class ChatListFYISheetCoordinator { networkManager: NetworkManager, profileManager: ProfileManager, ) { + self.backupArchiveErrorStore = backupArchiveErrorStore self.backupExportJobRunner = backupExportJobRunner self.backupSubscriptionIssueStore = backupSubscriptionIssueStore self.donationReceiptCredentialResultStore = donationReceiptCredentialResultStore @@ -128,6 +134,8 @@ class ChatListFYISheetCoordinator { return .backupSubscriptionFailedToRenew(FYISheet.BackupSubscriptionFailedToRenew()) } else if keyTransparencyStore.shouldWarnSelfCheckFailed(tx: tx) { return .keyTransparencySelfCheckFailed(FYISheet.KeyTransparencySelfCheckFailed()) + } else if backupArchiveErrorStore.hasError(tx: tx) { + return .backupArchiveError(FYISheet.BackupArchiveError()) } else { return nil } @@ -257,6 +265,8 @@ class ChatListFYISheetCoordinator { await _present(backupSubscriptionFailedToRenew: backupSubscriptionFailedToRenew, from: chatListViewController) case .keyTransparencySelfCheckFailed(let keyTransparencySelfCheckFailed): await _present(keyTransparencySelfCheckFailed: keyTransparencySelfCheckFailed, from: chatListViewController) + case .backupArchiveError(let backupArchiveError): + await _present(backupArchiveError: backupArchiveError, from: chatListViewController) } } @@ -469,6 +479,22 @@ class ChatListFYISheetCoordinator { } } + private func _present( + backupArchiveError: FYISheet.BackupArchiveError, + from chatListViewController: ChatListViewController, + ) async { + let logger = PrefixedLogger(prefix: "[Backups]") + logger.info("Showing BackupArchiveError FYI sheet.") + + let sheet = BackupArchiveErrorHeroSheet(fromViewController: chatListViewController) + + chatListViewController.present(sheet, animated: true) { [self] in + db.write { tx in + backupArchiveErrorStore.setHasError(false, tx: tx) + } + } + } + private func _present( keyTransparencySelfCheckFailed: FYISheet.KeyTransparencySelfCheckFailed, from chatListViewController: ChatListViewController, @@ -690,3 +716,27 @@ private final class KeyTransparencySelfCheckFailedHeroSheet: HeroSheetViewContro ) } } + +// MARK: - + +private final class BackupArchiveErrorHeroSheet: HeroSheetViewController { + init(fromViewController: UIViewController) { + super.init( + hero: .image(.errorCircle, tintColor: .Signal.label), + title: "Backup Archive Error! (Internal-only)", + body: "Backup import or export hit errors. Please submit a debug log so the iOS team can investigate.", + primaryButton: HeroSheetViewController.Button( + title: "Submit debug log", + action: { sheet in + sheet.dismiss(animated: true) { + DebugLogs.submitLogs(supportTag: "BackupArchive", dumper: .fromGlobals()) + } + }, + ), + secondaryButton: .dismissing( + title: CommonStrings.notNowButton, + style: .secondary, + ), + ) + } +} diff --git a/Signal/src/ViewControllers/HomeView/Chat List/ChatListViewController.swift b/Signal/src/ViewControllers/HomeView/Chat List/ChatListViewController.swift index 95ffdc754a..99bed9c821 100644 --- a/Signal/src/ViewControllers/HomeView/Chat List/ChatListViewController.swift +++ b/Signal/src/ViewControllers/HomeView/Chat List/ChatListViewController.swift @@ -231,8 +231,6 @@ public class ChatListViewController: OWSViewController, HomeTabViewController { } } - private var hasPresentedBackupErrors = false - override public func viewDidAppear(_ animated: Bool) { super.viewDidAppear(animated) @@ -244,11 +242,6 @@ public class ChatListViewController: OWSViewController, HomeTabViewController { presentGetStartedBannerIfNecessary() } - if !hasPresentedBackupErrors { - hasPresentedBackupErrors = true - DependenciesBridge.shared.backupArchiveErrorPresenter.presentOverTopmostViewController(completion: {}) - } - requestReviewIfAppropriate() viewState.searchResultsController.viewDidAppear(animated) @@ -373,6 +366,7 @@ public class ChatListViewController: OWSViewController, HomeTabViewController { @objc func showFYISheetIfNecessary() { let fyiSheetCoordinator = ChatListFYISheetCoordinator( + backupArchiveErrorStore: BackupArchiveErrorStore(), backupExportJobRunner: DependenciesBridge.shared.backupExportJobRunner, backupSubscriptionIssueStore: BackupSubscriptionIssueStore(), donationReceiptCredentialResultStore: DependenciesBridge.shared.donationReceiptCredentialResultStore, diff --git a/SignalNSE/NSEEnvironment.swift b/SignalNSE/NSEEnvironment.swift index 369b85a38c..d6d819ffc3 100644 --- a/SignalNSE/NSEEnvironment.swift +++ b/SignalNSE/NSEEnvironment.swift @@ -63,7 +63,6 @@ class NSEEnvironment { .initGlobals( appContext: appContext, appReadiness: appReadiness, - backupArchiveErrorPresenterFactory: NoOpBackupArchiveErrorPresenterFactory(), deviceBatteryLevelManager: nil, deviceSleepManager: nil, paymentsEvents: PaymentsEventsAppExtension(), diff --git a/SignalServiceKit/Backups/Archiving/Archivers/BackupArchive+Errors.swift b/SignalServiceKit/Backups/Archiving/Archivers/BackupArchive+Errors.swift index 339dfea0bf..ee307c46eb 100644 --- a/SignalServiceKit/Backups/Archiving/Archivers/BackupArchive+Errors.swift +++ b/SignalServiceKit/Backups/Archiving/Archivers/BackupArchive+Errors.swift @@ -3,8 +3,6 @@ // SPDX-License-Identifier: AGPL-3.0-only // -import SwiftProtobuf - extension BackupArchive { public typealias RawError = Swift.Error @@ -1150,12 +1148,13 @@ extension BackupArchive { extension BackupArchive { - public enum LogLevel: Int { - /// Log these, but don't pull up the internal - /// dialog if all errors are warnings. + public enum LogLevel: Int, Comparable { case warning - /// Log these and show the internal dialog if these happen. case error + + public static func <(lhs: Self, rhs: Self) -> Bool { + return lhs.rawValue < rhs.rawValue + } } } @@ -1174,46 +1173,10 @@ extension BackupArchive { var logLevel: BackupArchive.LogLevel { get } } - struct LoggableErrorAndProto { - let error: any BackupArchive.LoggableError - let wasFrameDropped: Bool - /// Nil for archiving, if we fail to even parse the proto on restore, - /// or if the feature flag is disabled such that this would be unused. - let protoJson: String? - - init( - error: any BackupArchive.LoggableError, - wasFrameDropped: Bool, - protoFrame: SwiftProtobuf.Message? = nil, - ) { - self.error = error - self.wasFrameDropped = wasFrameDropped - // Don't serialize proto frames if we aren't displaying errors. - if let protoFrame, BuildFlags.Backups.archiveErrorDisplay { - do { - self.protoJson = try String( - data: JSONSerialization.data( - withJSONObject: JSONSerialization.jsonObject( - with: protoFrame.jsonUTF8Data(), - options: .mutableContainers, - ), - options: .prettyPrinted, - ), - encoding: .utf8, - ) - } catch let jsonError { - self.protoJson = "Unable to json encode proto: \(jsonError)" - } - } else { - self.protoJson = nil - } - } - } - - static func collapse(_ errors: [LoggableErrorAndProto]) -> [CollapsedErrorLog] { + static func collapse(_ errors: [any LoggableError]) -> [CollapsedErrorLog] { var collapsedLogs = OrderedDictionary() for error in errors { - let collapseKey = error.error.collapseKey ?? UUID().uuidString + let collapseKey = error.collapseKey ?? UUID().uuidString if var existingLog = collapsedLogs[collapseKey] { existingLog.collapse(error) @@ -1231,35 +1194,26 @@ extension BackupArchive { public private(set) var typeLogString: String public private(set) var exampleCallsiteString: String - public private(set) var exampleProtoFrameJson: String? public private(set) var errorCount: UInt = 0 - public private(set) var wasFrameDropped: Bool public private(set) var logLevel: BackupArchive.LogLevel - init(_ error: LoggableErrorAndProto) { + init(_ error: any LoggableError) { self.logger = PrefixedLogger(prefix: "[Backups]") - self.typeLogString = error.error.typeLogString - self.exampleCallsiteString = error.error.callsiteLogString - self.exampleProtoFrameJson = error.protoJson - self.wasFrameDropped = error.wasFrameDropped - self.logLevel = error.error.logLevel + self.typeLogString = error.typeLogString + self.exampleCallsiteString = error.callsiteLogString + self.logLevel = error.logLevel self.collapse(error) } - mutating func collapse(_ error: LoggableErrorAndProto) { + mutating func collapse(_ error: any LoggableError) { self.errorCount += 1 - self.wasFrameDropped = wasFrameDropped || error.wasFrameDropped - self.logLevel = LogLevel(rawValue: max(self.logLevel.rawValue, error.error.logLevel.rawValue))! - if exampleProtoFrameJson == nil, let protoJson = error.protoJson { - self.exampleProtoFrameJson = protoJson - } + self.logLevel = LogLevel(rawValue: max(self.logLevel.rawValue, error.logLevel.rawValue))! } func log() { let logString = typeLogString + " " - + "Dropped frame(s)? \(wasFrameDropped). " + "Repeated \(errorCount) times. " + "example callsite: \(exampleCallsiteString)" switch logLevel { diff --git a/SignalServiceKit/Backups/Archiving/Archivers/Chat/BackupArchiveChatStyleArchiver.swift b/SignalServiceKit/Backups/Archiving/Archivers/Chat/BackupArchiveChatStyleArchiver.swift index 64af0bfba8..3731cd0313 100644 --- a/SignalServiceKit/Backups/Archiving/Archivers/Chat/BackupArchiveChatStyleArchiver.swift +++ b/SignalServiceKit/Backups/Archiving/Archivers/Chat/BackupArchiveChatStyleArchiver.swift @@ -83,12 +83,7 @@ public class BackupArchiveChatStyleArchiver: BackupArchiveProtoStreamWriter { if !partialErrors.isEmpty { // Just log these errors, but count as success and proceed. - BackupArchive - .collapse( - partialErrors - .map { BackupArchive.LoggableErrorAndProto(error: $0, wasFrameDropped: false) }, - ) - .forEach { $0.log() } + BackupArchive.collapse(partialErrors).forEach { $0.log() } } return .success(protos) diff --git a/SignalServiceKit/Backups/Archiving/BackupArchiveErrorPresenter.swift b/SignalServiceKit/Backups/Archiving/BackupArchiveErrorPresenter.swift deleted file mode 100644 index 9f6b3b5d7a..0000000000 --- a/SignalServiceKit/Backups/Archiving/BackupArchiveErrorPresenter.swift +++ /dev/null @@ -1,61 +0,0 @@ -// -// Copyright 2024 Signal Messenger, LLC -// SPDX-License-Identifier: AGPL-3.0-only -// - -public import LibSignalClient - -public protocol BackupArchiveErrorPresenterFactory { - - func build( - db: any DB, - tsAccountManager: TSAccountManager, - ) -> BackupArchiveErrorPresenter -} - -public protocol BackupArchiveErrorPresenter { - - /// Persist a set of errors for future display. - /// We persist because display may be deferred until certain UI actions occur (finishing registration) - /// during which time the app may be interrupted. - /// We only care to hold onto the latest set of backup errors. - func persistErrors(_ errors: [BackupArchive.CollapsedErrorLog], didFail: Bool, tx: DBWriteTransaction) - - /// Persist a validation error for future display. - /// We persist because display may be deferred until certain UI actions occur (finishing registration) - /// during which time the app may be interrupted. - /// We only care to hold onto the latest validation error. - func persistValidationError(_ error: MessageBackupValidationError) async - - /// Present over the current view controller; calls completion when presentation has finished. - func presentOverTopmostViewController(completion: @escaping () -> Void) -} - -public class NoOpBackupArchiveErrorPresenterFactory: BackupArchiveErrorPresenterFactory { - - public init() {} - - public func build( - db: any DB, - tsAccountManager: TSAccountManager, - ) -> BackupArchiveErrorPresenter { - return NoOpBackupArchiveErrorPresenter() - } -} - -public class NoOpBackupArchiveErrorPresenter: BackupArchiveErrorPresenter { - - public init() {} - - public func persistErrors(_ errors: [BackupArchive.CollapsedErrorLog], didFail: Bool, tx: DBWriteTransaction) { - // do nothing - } - - public func persistValidationError(_ error: MessageBackupValidationError) async { - // do nothing - } - - public func presentOverTopmostViewController(completion: @escaping () -> Void) { - // do nothing - } -} diff --git a/SignalServiceKit/Backups/Archiving/BackupArchiveErrorStore.swift b/SignalServiceKit/Backups/Archiving/BackupArchiveErrorStore.swift new file mode 100644 index 0000000000..defe09fbcd --- /dev/null +++ b/SignalServiceKit/Backups/Archiving/BackupArchiveErrorStore.swift @@ -0,0 +1,34 @@ +// +// Copyright 2024 Signal Messenger, LLC +// SPDX-License-Identifier: AGPL-3.0-only +// + +/// Tracks errors that occur during Backup archive/restore. +/// - Important +/// Errors are only tracked for internal users, and result in presenting +/// internal-only UI. Take care if expanding this beyond internal. +public struct BackupArchiveErrorStore { + + private let kvStore: KeyValueStore + private static let hasPendingErrorsKey = "hasPendingErrors" + + public init() { + kvStore = KeyValueStore(collection: "BackupArchiveErrorStore") + } + + public func setHasError(_ value: Bool, tx: DBWriteTransaction) { + guard BuildFlags.Backups.archiveErrorDisplay else { + return + } + + kvStore.setBool(value, key: Self.hasPendingErrorsKey, transaction: tx) + } + + public func hasError(tx: DBReadTransaction) -> Bool { + guard BuildFlags.Backups.archiveErrorDisplay else { + return false + } + + return kvStore.getBool(Self.hasPendingErrorsKey, defaultValue: false, transaction: tx) + } +} diff --git a/SignalServiceKit/Backups/Archiving/BackupArchiveManagerImpl.swift b/SignalServiceKit/Backups/Archiving/BackupArchiveManagerImpl.swift index 60b81f0872..2fc4efc394 100644 --- a/SignalServiceKit/Backups/Archiving/BackupArchiveManagerImpl.swift +++ b/SignalServiceKit/Backups/Archiving/BackupArchiveManagerImpl.swift @@ -6,13 +6,6 @@ import GRDB public import LibSignalClient -public enum BackupValidationError: Error { - case unknownFields([String]) - case validationFailed(message: String, unknownFields: [String]) - case ioError(String) - case unknownError -} - public enum BackupImportError: Error { case unsupportedVersion } @@ -31,7 +24,6 @@ public class BackupArchiveManagerImpl: BackupArchiveManager { private class NotImplementedError: Error {} private class BackupError: Error {} - private typealias LoggableErrorAndProto = BackupArchive.LoggableErrorAndProto private let accountDataArchiver: BackupArchiveAccountDataArchiver private let adHocCallArchiver: BackupArchiveAdHocCallArchiver @@ -39,7 +31,7 @@ public class BackupArchiveManagerImpl: BackupArchiveManager { private let attachmentDownloadManager: AttachmentDownloadManager private let attachmentUploadManager: AttachmentUploadManager private let avatarFetcher: BackupArchiveAvatarFetcher - private let backupArchiveErrorPresenter: BackupArchiveErrorPresenter + private let backupArchiveErrorStore: BackupArchiveErrorStore private let backupAttachmentCoordinator: BackupAttachmentCoordinator private let backupAttachmentUploadEraStore: BackupAttachmentUploadEraStore private let backupNonceMetadataStore: BackupNonceMetadataStore @@ -80,7 +72,7 @@ public class BackupArchiveManagerImpl: BackupArchiveManager { attachmentDownloadManager: AttachmentDownloadManager, attachmentUploadManager: AttachmentUploadManager, avatarFetcher: BackupArchiveAvatarFetcher, - backupArchiveErrorPresenter: BackupArchiveErrorPresenter, + backupArchiveErrorStore: BackupArchiveErrorStore, backupAttachmentCoordinator: BackupAttachmentCoordinator, backupAttachmentUploadEraStore: BackupAttachmentUploadEraStore, backupNonceMetadataStore: BackupNonceMetadataStore, @@ -117,7 +109,7 @@ public class BackupArchiveManagerImpl: BackupArchiveManager { self.attachmentDownloadManager = attachmentDownloadManager self.attachmentUploadManager = attachmentUploadManager self.avatarFetcher = avatarFetcher - self.backupArchiveErrorPresenter = backupArchiveErrorPresenter + self.backupArchiveErrorStore = backupArchiveErrorStore self.backupAttachmentCoordinator = backupAttachmentCoordinator self.backupAttachmentUploadEraStore = backupAttachmentUploadEraStore self.backupNonceMetadataStore = backupNonceMetadataStore @@ -469,7 +461,7 @@ public class BackupArchiveManagerImpl: BackupArchiveManager { throw OWSAssertionError("MRBK unset as backup being created!") } - var errors = [LoggableErrorAndProto]() + var errors = [any BackupArchive.LoggableError]() let result = Result(catching: { logger.info("Exporting for \(purposeString) with version \(backupVersion), timestamp \(startDate.ows_millisecondsSince1970)") @@ -505,7 +497,7 @@ public class BackupArchiveManagerImpl: BackupArchiveManager { case .success: break case .failure(let error): - errors.append(LoggableErrorAndProto(error: error, wasFrameDropped: true)) + errors.append(error) throw OWSAssertionError("Failed to archive account data") } } @@ -525,7 +517,7 @@ public class BackupArchiveManagerImpl: BackupArchiveManager { case .success(let success): localRecipientId = success case .failure(let error): - errors.append(LoggableErrorAndProto(error: error, wasFrameDropped: true)) + errors.append(error) throw OWSAssertionError("Failed to archive local recipient!") } @@ -559,7 +551,7 @@ public class BackupArchiveManagerImpl: BackupArchiveManager { case .success: break case .failure(let error): - errors.append(LoggableErrorAndProto(error: error, wasFrameDropped: true)) + errors.append(error) throw OWSAssertionError("Failed to archive release notes channel!") } } @@ -572,9 +564,9 @@ public class BackupArchiveManagerImpl: BackupArchiveManager { case .success: break case .partialSuccess(let partialFailures): - errors.append(contentsOf: partialFailures.map { LoggableErrorAndProto(error: $0, wasFrameDropped: false) }) + errors.append(contentsOf: partialFailures) case .completeFailure(let error): - errors.append(LoggableErrorAndProto(error: error, wasFrameDropped: true)) + errors.append(error) throw BackupError() } @@ -585,9 +577,9 @@ public class BackupArchiveManagerImpl: BackupArchiveManager { case .success: break case .partialSuccess(let partialFailures): - errors.append(contentsOf: partialFailures.map { LoggableErrorAndProto(error: $0, wasFrameDropped: false) }) + errors.append(contentsOf: partialFailures) case .completeFailure(let error): - errors.append(LoggableErrorAndProto(error: error, wasFrameDropped: true)) + errors.append(error) throw BackupError() } @@ -598,9 +590,9 @@ public class BackupArchiveManagerImpl: BackupArchiveManager { case .success: break case .partialSuccess(let partialFailures): - errors.append(contentsOf: partialFailures.map { LoggableErrorAndProto(error: $0, wasFrameDropped: false) }) + errors.append(contentsOf: partialFailures) case .completeFailure(let error): - errors.append(LoggableErrorAndProto(error: error, wasFrameDropped: true)) + errors.append(error) throw BackupError() } @@ -611,9 +603,9 @@ public class BackupArchiveManagerImpl: BackupArchiveManager { case .success: break case .partialSuccess(let partialFailures): - errors.append(contentsOf: partialFailures.map { LoggableErrorAndProto(error: $0, wasFrameDropped: false) }) + errors.append(contentsOf: partialFailures) case .completeFailure(let error): - errors.append(LoggableErrorAndProto(error: error, wasFrameDropped: true)) + errors.append(error) throw BackupError() } @@ -637,9 +629,9 @@ public class BackupArchiveManagerImpl: BackupArchiveManager { case .success: break case .partialSuccess(let partialFailures): - errors.append(contentsOf: partialFailures.map { LoggableErrorAndProto(error: $0, wasFrameDropped: false) }) + errors.append(contentsOf: partialFailures) case .completeFailure(let error): - errors.append(LoggableErrorAndProto(error: error, wasFrameDropped: true)) + errors.append(error) throw BackupError() } @@ -651,9 +643,9 @@ public class BackupArchiveManagerImpl: BackupArchiveManager { case .success: break case .partialSuccess(let partialFailures): - errors.append(contentsOf: partialFailures.map { LoggableErrorAndProto(error: $0, wasFrameDropped: false) }) + errors.append(contentsOf: partialFailures) case .completeFailure(let error): - errors.append(LoggableErrorAndProto(error: error, wasFrameDropped: true)) + errors.append(error) throw BackupError() } @@ -675,9 +667,9 @@ public class BackupArchiveManagerImpl: BackupArchiveManager { case .success: break case .partialSuccess(let partialFailures): - errors.append(contentsOf: partialFailures.map { LoggableErrorAndProto(error: $0, wasFrameDropped: false) }) + errors.append(contentsOf: partialFailures) case .completeFailure(let error): - errors.append(LoggableErrorAndProto(error: error, wasFrameDropped: true)) + errors.append(error) throw BackupError() } @@ -689,9 +681,9 @@ public class BackupArchiveManagerImpl: BackupArchiveManager { case .success: break case .partialSuccess(let partialFailures): - errors.append(contentsOf: partialFailures.map { LoggableErrorAndProto(error: $0, wasFrameDropped: false) }) + errors.append(contentsOf: partialFailures) case .completeFailure(let error): - errors.append(LoggableErrorAndProto(error: error, wasFrameDropped: true)) + errors.append(error) throw BackupError() } @@ -947,7 +939,7 @@ public class BackupArchiveManagerImpl: BackupArchiveManager { ) } - var frameErrors = [LoggableErrorAndProto]() + var frameErrors = [any BackupArchive.LoggableError]() let result = Result(catching: { var hasMoreFrames = false var framesRestored: UInt64 = 0 @@ -964,11 +956,8 @@ public class BackupArchiveManagerImpl: BackupArchiveManager { throw OWSAssertionError("invalid empty header frame") case .protoDeserializationError(let error): // Fail if we fail to deserialize the header. - frameErrors.append(LoggableErrorAndProto( - error: BackupArchive.RestoreFrameError.restoreFrameError( - .invalidProtoData(.missingBackupInfoHeader), - ), - wasFrameDropped: true, + frameErrors.append(BackupArchive.RestoreFrameError.restoreFrameError( + .invalidProtoData(.missingBackupInfoHeader), )) throw error } @@ -976,12 +965,8 @@ public class BackupArchiveManagerImpl: BackupArchiveManager { logger.info("Importing with version \(backupInfo.version), timestamp \(backupInfo.backupTimeMs)") guard backupInfo.version == Constants.supportedBackupVersion else { - frameErrors.append(LoggableErrorAndProto( - error: BackupArchive.RestoreFrameError.restoreFrameError( - .invalidProtoData(.unsupportedBackupInfoVersion), - ), - wasFrameDropped: true, - protoFrame: backupInfo, + frameErrors.append(BackupArchive.RestoreFrameError.restoreFrameError( + .invalidProtoData(.unsupportedBackupInfoVersion), )) throw BackupImportError.unsupportedVersion } @@ -989,12 +974,8 @@ public class BackupArchiveManagerImpl: BackupArchiveManager { let mrbk = try BackupKey(contents: backupInfo.mediaRootBackupKey) localStorage.setMediaRootBackupKey(MediaRootBackupKey(backupKey: mrbk), tx: tx) } catch { - frameErrors.append(LoggableErrorAndProto( - error: BackupArchive.RestoreFrameError.restoreFrameError( - .invalidProtoData(.invalidMediaRootBackupKey), - ), - wasFrameDropped: true, - protoFrame: backupInfo, + frameErrors.append(BackupArchive.RestoreFrameError.restoreFrameError( + .invalidProtoData(.invalidMediaRootBackupKey), )) throw error } @@ -1113,11 +1094,8 @@ public class BackupArchiveManagerImpl: BackupArchiveManager { let frameItem = frame.item else { if hasMoreFrames { - frameErrors.append(LoggableErrorAndProto( - error: BackupArchive.UnrecognizedEnumError( - enumType: BackupProto_Frame.OneOf_Item.self, - ), - wasFrameDropped: true, + frameErrors.append(BackupArchive.UnrecognizedEnumError( + enumType: BackupProto_Frame.OneOf_Item.self, )) } return @@ -1178,12 +1156,12 @@ public class BackupArchiveManagerImpl: BackupArchiveManager { case .success: return case .unrecognizedEnum(let error): - frameErrors.append(LoggableErrorAndProto(error: error, wasFrameDropped: true, protoFrame: recipient)) + frameErrors.append(error) return case .partialRestore(let errors): - frameErrors.append(contentsOf: errors.map { LoggableErrorAndProto(error: $0, wasFrameDropped: false, protoFrame: recipient) }) + frameErrors.append(contentsOf: errors) case .failure(let errors): - frameErrors.append(contentsOf: errors.map { LoggableErrorAndProto(error: $0, wasFrameDropped: true, protoFrame: recipient) }) + frameErrors.append(contentsOf: errors) if BuildFlags.Backups.restoreFailOnAnyError { throw BackupError() } @@ -1197,12 +1175,12 @@ public class BackupArchiveManagerImpl: BackupArchiveManager { case .success: return case .unrecognizedEnum(let error): - frameErrors.append(LoggableErrorAndProto(error: error, wasFrameDropped: true, protoFrame: chat)) + frameErrors.append(error) return case .partialRestore(let errors): - frameErrors.append(contentsOf: errors.map { LoggableErrorAndProto(error: $0, wasFrameDropped: false, protoFrame: chat) }) + frameErrors.append(contentsOf: errors) case .failure(let errors): - frameErrors.append(contentsOf: errors.map { LoggableErrorAndProto(error: $0, wasFrameDropped: true, protoFrame: chat) }) + frameErrors.append(contentsOf: errors) if BuildFlags.Backups.restoreFailOnAnyError { throw BackupError() } @@ -1216,12 +1194,12 @@ public class BackupArchiveManagerImpl: BackupArchiveManager { case .success: return case .unrecognizedEnum(let error): - frameErrors.append(LoggableErrorAndProto(error: error, wasFrameDropped: true, protoFrame: chatItem)) + frameErrors.append(error) return case .partialRestore(let errors): - frameErrors.append(contentsOf: errors.map { LoggableErrorAndProto(error: $0, wasFrameDropped: false, protoFrame: chatItem) }) + frameErrors.append(contentsOf: errors) case .failure(let errors): - frameErrors.append(contentsOf: errors.map { LoggableErrorAndProto(error: $0, wasFrameDropped: true, protoFrame: chatItem) }) + frameErrors.append(contentsOf: errors) if BuildFlags.Backups.restoreFailOnAnyError { throw BackupError() } @@ -1237,12 +1215,12 @@ public class BackupArchiveManagerImpl: BackupArchiveManager { case .success: return case .unrecognizedEnum(let error): - frameErrors.append(LoggableErrorAndProto(error: error, wasFrameDropped: true, protoFrame: backupProtoAccountData)) + frameErrors.append(error) return case .partialRestore(let errors): - frameErrors.append(contentsOf: errors.map { LoggableErrorAndProto(error: $0, wasFrameDropped: false, protoFrame: backupProtoAccountData) }) + frameErrors.append(contentsOf: errors) case .failure(let errors): - frameErrors.append(contentsOf: errors.map { LoggableErrorAndProto(error: $0, wasFrameDropped: true, protoFrame: backupProtoAccountData) }) + frameErrors.append(contentsOf: errors) // We always fail if we fail to import account data, even in prod. throw BackupError() } @@ -1255,12 +1233,12 @@ public class BackupArchiveManagerImpl: BackupArchiveManager { case .success: return case .unrecognizedEnum(let error): - frameErrors.append(LoggableErrorAndProto(error: error, wasFrameDropped: true, protoFrame: backupProtoStickerPack)) + frameErrors.append(error) return case .partialRestore(let errors): - frameErrors.append(contentsOf: errors.map { LoggableErrorAndProto(error: $0, wasFrameDropped: false, protoFrame: backupProtoStickerPack) }) + frameErrors.append(contentsOf: errors) case .failure(let errors): - frameErrors.append(contentsOf: errors.map { LoggableErrorAndProto(error: $0, wasFrameDropped: true, protoFrame: backupProtoStickerPack) }) + frameErrors.append(contentsOf: errors) if BuildFlags.Backups.restoreFailOnAnyError { throw BackupError() } @@ -1274,12 +1252,12 @@ public class BackupArchiveManagerImpl: BackupArchiveManager { case .success: return case .unrecognizedEnum(let error): - frameErrors.append(LoggableErrorAndProto(error: error, wasFrameDropped: true, protoFrame: backupProtoAdHocCall)) + frameErrors.append(error) return case .partialRestore(let errors): - frameErrors.append(contentsOf: errors.map { LoggableErrorAndProto(error: $0, wasFrameDropped: false, protoFrame: backupProtoAdHocCall) }) + frameErrors.append(contentsOf: errors) case .failure(let errors): - frameErrors.append(contentsOf: errors.map { LoggableErrorAndProto(error: $0, wasFrameDropped: true, protoFrame: backupProtoAdHocCall) }) + frameErrors.append(contentsOf: errors) if BuildFlags.Backups.restoreFailOnAnyError { throw BackupError() } @@ -1432,32 +1410,28 @@ public class BackupArchiveManagerImpl: BackupArchiveManager { // MARK: - private func processErrors( - errors: [LoggableErrorAndProto], + errors: [any BackupArchive.LoggableError], didFail: Bool, ) { let collapsedErrors = BackupArchive.collapse(errors) - var maxLogLevel = -1 - var wasFrameDropped = false - collapsedErrors.forEach { collapsedError in + guard let firstError = collapsedErrors.first else { + return + } + + var maxLogLevel = firstError.logLevel + for collapsedError in collapsedErrors { collapsedError.log() - maxLogLevel = max(maxLogLevel, collapsedError.logLevel.rawValue) - if collapsedError.wasFrameDropped { - wasFrameDropped = true - } + maxLogLevel = max(maxLogLevel, collapsedError.logLevel) } - if wasFrameDropped { - // Log this specifically so we can do a naive exact text search in debug logs. - logger.error("Dropped frame(s) on backup export or import!!!") - } - // Only present errors if some error rises above warning. - // (But if one does, present _all_ errors). - if maxLogLevel > BackupArchive.LogLevel.warning.rawValue { + + if maxLogLevel > BackupArchive.LogLevel.warning { + // Do an async write, since in the case of an error we may roll back + // the transaction we're in. Task { await db.awaitableWrite { tx in - backupArchiveErrorPresenter.persistErrors(collapsedErrors, didFail: didFail, tx: tx) + backupArchiveErrorStore.setHasError(true, tx: tx) } } - } } @@ -1466,31 +1440,28 @@ public class BackupArchiveManagerImpl: BackupArchiveManager { backupEncryptionKey: MessageBackupKey, backupPurpose: MessageBackupPurpose, ) async throws { - let fileSize = (try? OWSFileSystem.fileSize(ofPath: fileUrl.path)) ?? 0 + let fileSize = try OWSFileSystem.fileSize(ofPath: fileUrl.path) do { - let result = try validateMessageBackup(key: backupEncryptionKey, purpose: backupPurpose, length: fileSize) { - return try FileHandle(forReadingFrom: fileUrl) - } + let result = try validateMessageBackup( + key: backupEncryptionKey, + purpose: backupPurpose, + length: fileSize, + makeStream: { () throws -> SignalInputStream in + return try FileHandle(forReadingFrom: fileUrl) + }, + ) if result.fields.count > 0 { - throw BackupValidationError.unknownFields(result.fields) + throw OWSAssertionError("Unknown fields during Backup validation! \(result.fields)") } + } catch let validationError as MessageBackupValidationError { + await db.awaitableWrite { tx in + backupArchiveErrorStore.setHasError(true, tx: tx) + } + + throw OWSAssertionError("Backup validation failed! \(validationError.errorMessage); unknown fields \(validationError.unknownFields.fields)") } catch { - switch error { - case let validationError as MessageBackupValidationError: - await backupArchiveErrorPresenter.persistValidationError(validationError) - logger.error("Backup validation failed \(validationError.errorMessage)") - throw BackupValidationError.validationFailed( - message: validationError.errorMessage, - unknownFields: validationError.unknownFields.fields, - ) - case SignalError.ioError(let description): - logger.error("Backup validation i/o error: \(description)") - throw BackupValidationError.ioError(description) - default: - logger.error("Backup validation unknown error: \(error)") - throw BackupValidationError.unknownError - } + throw OWSAssertionError("Backup validation failed! \(error)") } } diff --git a/SignalServiceKit/Environment/AppSetup.swift b/SignalServiceKit/Environment/AppSetup.swift index 9896dd7397..00faf5d817 100644 --- a/SignalServiceKit/Environment/AppSetup.swift +++ b/SignalServiceKit/Environment/AppSetup.swift @@ -172,7 +172,6 @@ extension AppSetup.GlobalsContinuation { public func initGlobals( appContext: AppContext, appReadiness: AppReadiness, - backupArchiveErrorPresenterFactory: BackupArchiveErrorPresenterFactory, deviceBatteryLevelManager: (any DeviceBatteryLevelManager)?, deviceSleepManager: (any DeviceSleepManager)?, paymentsEvents: PaymentsEvents, @@ -1398,10 +1397,6 @@ extension AppSetup.GlobalsContinuation { ) let backupThreadStore = BackupArchiveThreadStore(threadStore: threadStore) - let backupArchiveErrorPresenter = backupArchiveErrorPresenterFactory.build( - db: db, - tsAccountManager: tsAccountManager, - ) let backupArchiveAvatarFetcher = BackupArchiveAvatarFetcher( appReadiness: appReadiness, dateProvider: dateProvider, @@ -1492,7 +1487,7 @@ extension AppSetup.GlobalsContinuation { attachmentDownloadManager: attachmentDownloadManager, attachmentUploadManager: attachmentUploadManager, avatarFetcher: backupArchiveAvatarFetcher, - backupArchiveErrorPresenter: backupArchiveErrorPresenter, + backupArchiveErrorStore: BackupArchiveErrorStore(), backupAttachmentCoordinator: backupAttachmentCoordinator, backupAttachmentUploadEraStore: backupAttachmentUploadEraStore, backupNonceMetadataStore: backupNonceMetadataStore, @@ -1707,7 +1702,6 @@ extension AppSetup.GlobalsContinuation { authorMergeHelper: authorMergeHelper, avatarDefaultColorManager: avatarDefaultColorManager, backgroundMessageFetcherFactory: backgroundMessageFetcherFactory, - backupArchiveErrorPresenter: backupArchiveErrorPresenter, backupArchiveManager: backupArchiveManager, backupAttachmentDownloadProgress: backupAttachmentDownloadProgress, backupAttachmentDownloadStore: backupAttachmentDownloadStore, diff --git a/SignalServiceKit/Environment/DependenciesBridge.swift b/SignalServiceKit/Environment/DependenciesBridge.swift index 14104c98b7..2e136da345 100644 --- a/SignalServiceKit/Environment/DependenciesBridge.swift +++ b/SignalServiceKit/Environment/DependenciesBridge.swift @@ -66,7 +66,6 @@ public class DependenciesBridge { public let authorMergeHelper: AuthorMergeHelper public let avatarDefaultColorManager: AvatarDefaultColorManager public let backgroundMessageFetcherFactory: BackgroundMessageFetcherFactory - public let backupArchiveErrorPresenter: BackupArchiveErrorPresenter public let backupArchiveManager: BackupArchiveManager public let backupAttachmentDownloadProgress: BackupAttachmentDownloadProgress public let backupAttachmentDownloadStore: BackupAttachmentDownloadStore @@ -210,7 +209,6 @@ public class DependenciesBridge { authorMergeHelper: AuthorMergeHelper, avatarDefaultColorManager: AvatarDefaultColorManager, backgroundMessageFetcherFactory: BackgroundMessageFetcherFactory, - backupArchiveErrorPresenter: BackupArchiveErrorPresenter, backupArchiveManager: BackupArchiveManager, backupAttachmentDownloadProgress: BackupAttachmentDownloadProgress, backupAttachmentDownloadStore: BackupAttachmentDownloadStore, @@ -353,7 +351,6 @@ public class DependenciesBridge { self.authorMergeHelper = authorMergeHelper self.avatarDefaultColorManager = avatarDefaultColorManager self.backgroundMessageFetcherFactory = backgroundMessageFetcherFactory - self.backupArchiveErrorPresenter = backupArchiveErrorPresenter self.backupArchiveManager = backupArchiveManager self.backupAttachmentDownloadProgress = backupAttachmentDownloadProgress self.backupAttachmentDownloadStore = backupAttachmentDownloadStore diff --git a/SignalServiceKit/TestUtils/MockSSKEnvironment.swift b/SignalServiceKit/TestUtils/MockSSKEnvironment.swift index a3bdc0ed15..5cc1128bfe 100644 --- a/SignalServiceKit/TestUtils/MockSSKEnvironment.swift +++ b/SignalServiceKit/TestUtils/MockSSKEnvironment.swift @@ -73,7 +73,6 @@ public class MockSSKEnvironment { ).migrateDatabaseSchema().initGlobals( appContext: testAppContext, appReadiness: appReadiness, - backupArchiveErrorPresenterFactory: NoOpBackupArchiveErrorPresenterFactory(), deviceBatteryLevelManager: nil, deviceSleepManager: nil, paymentsEvents: PaymentsEventsNoop(), diff --git a/SignalShareExtension/ShareViewController.swift b/SignalShareExtension/ShareViewController.swift index 5f7fc66279..1cff7055e2 100644 --- a/SignalShareExtension/ShareViewController.swift +++ b/SignalShareExtension/ShareViewController.swift @@ -91,7 +91,6 @@ public class ShareViewController: OWSNavigationController, ShareViewDelegate, SA .initGlobals( appContext: appContext, appReadiness: appReadiness, - backupArchiveErrorPresenterFactory: NoOpBackupArchiveErrorPresenterFactory(), deviceBatteryLevelManager: nil, deviceSleepManager: nil, paymentsEvents: PaymentsEventsAppExtension(),