From 2cd4e44960f5a27a37468df1b34b3b31866fbb6f Mon Sep 17 00:00:00 2001 From: Sabrina Tardio Date: Wed, 13 Dec 2023 12:38:04 +0100 Subject: [PATCH 1/6] add alerts for errors --- ...cSettingsViewController+SyncDelegate.swift | 58 ++++++++++++++++--- DuckDuckGo/SyncSettingsViewController.swift | 12 ++-- DuckDuckGo/UserText.swift | 7 +++ DuckDuckGo/en.lproj/Localizable.strings | 21 +++++++ 4 files changed, 84 insertions(+), 14 deletions(-) diff --git a/DuckDuckGo/SyncSettingsViewController+SyncDelegate.swift b/DuckDuckGo/SyncSettingsViewController+SyncDelegate.swift index 981cfe0a9e..eca048bd78 100644 --- a/DuckDuckGo/SyncSettingsViewController+SyncDelegate.swift +++ b/DuckDuckGo/SyncSettingsViewController+SyncDelegate.swift @@ -45,7 +45,7 @@ extension SyncSettingsViewController: SyncManagementViewModelDelegate { let devices = try await syncService.updateDeviceName(name) mapDevices(devices) } catch { - handleError(error) + handleError(SyncError.unableToUpdateDeviceName, error: error) } } } @@ -61,15 +61,27 @@ extension SyncSettingsViewController: SyncManagementViewModelDelegate { self.refreshDevices() navigationController?.topViewController?.dismiss(animated: true, completion: showRecoveryPDF) } catch { - handleError(error) + handleError(SyncError.unableToSync, error: error) } } } @MainActor - func handleError(_ error: Error) { - // Work out how to handle this properly later - assertionFailure(error.localizedDescription) + func handleError(_ type: SyncError, error: Error) { + self.dismissPresentedViewController() + let alertController = UIAlertController( + title: type.title, + message: type.description + "\n" + error.localizedDescription, + preferredStyle: .alert) + + let okAction = UIAlertAction(title: UserText.syncPausedAlertOkButton, style: .default, handler: nil) + alertController.addAction(okAction) + + DispatchQueue.main.asyncAfter(deadline: .now() + 0.5) { + self.dismissPresentedViewController { [weak self] in + self?.navigationController?.topViewController?.present(alertController, animated: true, completion: nil) + } + } } func showSyncWithAnotherDevice() { @@ -163,7 +175,7 @@ extension SyncSettingsViewController: SyncManagementViewModelDelegate { AppUserDefaults().isSyncBookmarksPaused = false AppUserDefaults().isSyncCredentialsPaused = false } catch { - self.handleError(error) + self.handleError(SyncError.unableToTurnSyncOff, error: error) } continuation.resume(returning: true) } @@ -188,7 +200,7 @@ extension SyncSettingsViewController: SyncManagementViewModelDelegate { AppUserDefaults().isSyncBookmarksPaused = false AppUserDefaults().isSyncCredentialsPaused = false } catch { - self.handleError(error) + self.handleError(SyncError.unableToDeleteData, error: error) } continuation.resume(returning: true) } @@ -224,7 +236,7 @@ extension SyncSettingsViewController: SyncManagementViewModelDelegate { try await syncService.disconnect(deviceId: device.id) refreshDevices() } catch { - handleError(error) + handleError(SyncError.unableToRemoveDevice, error: error) } } } @@ -261,3 +273,33 @@ private class PortraitNavigationController: UINavigationController { } } + +enum SyncError { + case unableToSync + case unableToGetDevices + case unableToUpdateDeviceName + case unableToTurnSyncOff + case unableToDeleteData + case unableToRemoveDevice + + var title: String { + return UserText.syncErrorAlertTitle + } + + var description: String { + switch self { + case .unableToSync: + return UserText.unableToSyncDescription + case .unableToGetDevices: + return UserText.unableToGetDevicesDescription + case .unableToUpdateDeviceName: + return UserText.unableToUpdateDeviceNameDescription + case .unableToTurnSyncOff: + return UserText.unableToTurnSyncOffDescription + case .unableToDeleteData: + return UserText.unableToDeleteDataDescription + case .unableToRemoveDevice: + return UserText.unableToRemoveDeviceDescription + } + } +} diff --git a/DuckDuckGo/SyncSettingsViewController.swift b/DuckDuckGo/SyncSettingsViewController.swift index 2fec903a79..1db37bbd83 100644 --- a/DuckDuckGo/SyncSettingsViewController.swift +++ b/DuckDuckGo/SyncSettingsViewController.swift @@ -171,10 +171,10 @@ class SyncSettingsViewController: UIHostingController { } } - func dismissPresentedViewController() { + func dismissPresentedViewController(completion: (() -> Void)? = nil) { guard let presentedViewController = navigationController?.presentedViewController, !(presentedViewController is UIHostingController) else { return } - presentedViewController.dismiss(animated: true, completion: nil) + presentedViewController.dismiss(animated: true, completion: completion) endConnectMode() } @@ -190,7 +190,7 @@ class SyncSettingsViewController: UIHostingController { let devices = try await syncService.fetchDevices() mapDevices(devices) } catch { - handleError(error) + handleError(SyncError.unableToGetDevices, error: error) } } } @@ -223,7 +223,7 @@ extension SyncSettingsViewController: ScanOrPasteCodeViewModelDelegate { self.startPolling() return self.connector?.code } catch { - self.handleError(error) + self.handleError(SyncError.unableToSync, error: error) return nil } } @@ -249,7 +249,7 @@ extension SyncSettingsViewController: ScanOrPasteCodeViewModelDelegate { return } } catch { - handleError(error) + handleError(SyncError.unableToSync, error: error) } } } @@ -292,7 +292,7 @@ extension SyncSettingsViewController: ScanOrPasteCodeViewModelDelegate { } } catch { - handleError(error) + handleError(SyncError.unableToSync, error: error) } return false } diff --git a/DuckDuckGo/UserText.swift b/DuckDuckGo/UserText.swift index bc1454043b..a57714c114 100644 --- a/DuckDuckGo/UserText.swift +++ b/DuckDuckGo/UserText.swift @@ -860,6 +860,13 @@ But if you *do* want a peek under the hood, you can find more information about static let syncCredentialsPausedAlertDescription = NSLocalizedString("alert.sync-credentials-paused-description", value: "You have exceeded the passwords sync limit. Try deleting some passwords. Until this is resolved your passwords will not be backed up.", comment: "Description for alert shown when sync credentials paused for too many items") public static let syncPausedAlertOkButton = NSLocalizedString("alert.sync-paused-alert-ok-button", value: "OK", comment: "Confirmation button in alert") public static let syncPausedAlertLearnMoreButton = NSLocalizedString("alert.sync-paused-alert-learn-more-button", value: "Learn More", comment: "Learn more button in alert") + public static let syncErrorAlertTitle = NSLocalizedString("alert.sync-error", value: "Sync Error", comment: "Title for sync error alert") + public static let unableToSyncDescription = NSLocalizedString("alert.unable-to-sync-description", value: "Unable to sync.", comment: "Description for unable to sync error") + public static let unableToGetDevicesDescription = NSLocalizedString("alert.unable-to-get-devices-description", value: "Unable to retrieve the list of connected devices.", comment: "Description for unable to get devices error") + public static let unableToUpdateDeviceNameDescription = NSLocalizedString("alert.unable-to-update-device-name-description", value: "Unable to update the name of the device.", comment: "Description for unable to update device name error") + public static let unableToTurnSyncOffDescription = NSLocalizedString("alert.unable-to-turn-sync-off-description", value: "Unable to turn sync off.", comment: "Description for unable to turn sync off error") + public static let unableToDeleteDataDescription = NSLocalizedString("alert.unable-to-delete-data-description", value: "Unable to delete data on the server.", comment: "Description for unable to delete data error") + public static let unableToRemoveDeviceDescription = NSLocalizedString("alert.unable-to-remove-device-description", value: "Unable to remove the specified device from the synchronized devices.", comment: "Description for unable to remove device error") static let preemptiveCrashTitle = NSLocalizedString("error.preemptive-crash.title", value: "App issue detected", comment: "Alert title") static let preemptiveCrashBody = NSLocalizedString("error.preemptive-crash.body", value: "Looks like there's an issue with the app and it needs to close. Please reopen to continue.", comment: "Alert message") diff --git a/DuckDuckGo/en.lproj/Localizable.strings b/DuckDuckGo/en.lproj/Localizable.strings index b7a81a8f05..3533e4887a 100644 --- a/DuckDuckGo/en.lproj/Localizable.strings +++ b/DuckDuckGo/en.lproj/Localizable.strings @@ -139,6 +139,9 @@ /* Title for alert shown when sync credentials paused for too many items */ "alert.sync-credentials-paused-title" = "Passwords Sync is Paused"; +/* Title for sync error alert */ +"alert.sync-error" = "Sync Error"; + /* Learn more button in alert */ "alert.sync-paused-alert-learn-more-button" = "Learn More"; @@ -160,6 +163,24 @@ /* Save Favorite action */ "alert.title.save.favorite" = "Save Favorite"; +/* Description for unable to delete data error */ +"alert.unable-to-delete-data-description" = "Unable to delete data on the server."; + +/* Description for unable to get devices error */ +"alert.unable-to-get-devices-description" = "Unable to retrieve the list of connected devices."; + +/* Description for unable to remove device error */ +"alert.unable-to-remove-device-description" = "Unable to remove the specified device from the synchronized devices."; + +/* Description for unable to sync error */ +"alert.unable-to-sync-description" = "Unable to sync."; + +/* Description for unable to turn sync off error */ +"alert.unable-to-turn-sync-off-description" = "Unable to turn sync off."; + +/* Description for unable to update device name error */ +"alert.unable-to-update-device-name-description" = "Unable to update the name of the device."; + /* Shown on authentication screen */ "app.authentication.unlock" = "Unlock DuckDuckGo."; From d3db9a1c8076ebbb67cb44f0f18af14b62dfa560 Mon Sep 17 00:00:00 2001 From: Sabrina Tardio Date: Wed, 13 Dec 2023 12:39:44 +0100 Subject: [PATCH 2/6] add comment --- DuckDuckGo/SyncSettingsViewController+SyncDelegate.swift | 1 + 1 file changed, 1 insertion(+) diff --git a/DuckDuckGo/SyncSettingsViewController+SyncDelegate.swift b/DuckDuckGo/SyncSettingsViewController+SyncDelegate.swift index eca048bd78..eea08fdcf5 100644 --- a/DuckDuckGo/SyncSettingsViewController+SyncDelegate.swift +++ b/DuckDuckGo/SyncSettingsViewController+SyncDelegate.swift @@ -77,6 +77,7 @@ extension SyncSettingsViewController: SyncManagementViewModelDelegate { let okAction = UIAlertAction(title: UserText.syncPausedAlertOkButton, style: .default, handler: nil) alertController.addAction(okAction) + // Give time to the is syncing view to appear DispatchQueue.main.asyncAfter(deadline: .now() + 0.5) { self.dismissPresentedViewController { [weak self] in self?.navigationController?.topViewController?.present(alertController, animated: true, completion: nil) From 8a18fe7167b8b5b5b3d0672a0cba5ed38faadaa4 Mon Sep 17 00:00:00 2001 From: Sabrina Tardio Date: Wed, 13 Dec 2023 13:04:14 +0100 Subject: [PATCH 3/6] remove unwanted code --- DuckDuckGo/SyncSettingsViewController+SyncDelegate.swift | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/DuckDuckGo/SyncSettingsViewController+SyncDelegate.swift b/DuckDuckGo/SyncSettingsViewController+SyncDelegate.swift index eea08fdcf5..b66c47dfc2 100644 --- a/DuckDuckGo/SyncSettingsViewController+SyncDelegate.swift +++ b/DuckDuckGo/SyncSettingsViewController+SyncDelegate.swift @@ -68,7 +68,6 @@ extension SyncSettingsViewController: SyncManagementViewModelDelegate { @MainActor func handleError(_ type: SyncError, error: Error) { - self.dismissPresentedViewController() let alertController = UIAlertController( title: type.title, message: type.description + "\n" + error.localizedDescription, @@ -78,9 +77,9 @@ extension SyncSettingsViewController: SyncManagementViewModelDelegate { alertController.addAction(okAction) // Give time to the is syncing view to appear - DispatchQueue.main.asyncAfter(deadline: .now() + 0.5) { + DispatchQueue.main.asyncAfter(deadline: .now() + 1) { self.dismissPresentedViewController { [weak self] in - self?.navigationController?.topViewController?.present(alertController, animated: true, completion: nil) + self?.present(alertController, animated: true, completion: nil) } } } From e08fe9f2460230f5f9689b3c0d3ebcc979ff9a4f Mon Sep 17 00:00:00 2001 From: Sabrina Tardio Date: Wed, 13 Dec 2023 14:14:05 +0100 Subject: [PATCH 4/6] manage synced enabled errors --- ...cSettingsViewController+SyncDelegate.swift | 20 +++++++++++++------ DuckDuckGo/SyncSettingsViewController.swift | 8 ++++++-- 2 files changed, 20 insertions(+), 8 deletions(-) diff --git a/DuckDuckGo/SyncSettingsViewController+SyncDelegate.swift b/DuckDuckGo/SyncSettingsViewController+SyncDelegate.swift index b66c47dfc2..e2ec2ae698 100644 --- a/DuckDuckGo/SyncSettingsViewController+SyncDelegate.swift +++ b/DuckDuckGo/SyncSettingsViewController+SyncDelegate.swift @@ -76,8 +76,14 @@ extension SyncSettingsViewController: SyncManagementViewModelDelegate { let okAction = UIAlertAction(title: UserText.syncPausedAlertOkButton, style: .default, handler: nil) alertController.addAction(okAction) - // Give time to the is syncing view to appear - DispatchQueue.main.asyncAfter(deadline: .now() + 1) { + if type == .unableToSync { + // Give time to the is syncing view to appear + DispatchQueue.main.asyncAfter(deadline: .now() + 1) { + self.dismissPresentedViewController { [weak self] in + self?.present(alertController, animated: true, completion: nil) + } + } + } else { self.dismissPresentedViewController { [weak self] in self?.present(alertController, animated: true, completion: nil) } @@ -170,14 +176,15 @@ extension SyncSettingsViewController: SyncManagementViewModelDelegate { alert.addAction(title: UserText.syncTurnOffConfirmAction, style: .destructive) { Task { @MainActor in do { - self.rootView.model.isSyncEnabled = false try await self.syncService.disconnect() + self.rootView.model.isSyncEnabled = false AppUserDefaults().isSyncBookmarksPaused = false AppUserDefaults().isSyncCredentialsPaused = false + continuation.resume(returning: true) } catch { self.handleError(SyncError.unableToTurnSyncOff, error: error) + continuation.resume(returning: false) } - continuation.resume(returning: true) } } self.present(alert, animated: true) @@ -195,14 +202,15 @@ extension SyncSettingsViewController: SyncManagementViewModelDelegate { alert.addAction(title: UserText.syncDeleteAllConfirmAction, style: .destructive) { Task { @MainActor in do { - self.rootView.model.isSyncEnabled = false try await self.syncService.deleteAccount() + self.rootView.model.isSyncEnabled = false AppUserDefaults().isSyncBookmarksPaused = false AppUserDefaults().isSyncCredentialsPaused = false + continuation.resume(returning: true) } catch { self.handleError(SyncError.unableToDeleteData, error: error) + continuation.resume(returning: false) } - continuation.resume(returning: true) } } self.present(alert, animated: true) diff --git a/DuckDuckGo/SyncSettingsViewController.swift b/DuckDuckGo/SyncSettingsViewController.swift index 1db37bbd83..0044c1c3cc 100644 --- a/DuckDuckGo/SyncSettingsViewController.swift +++ b/DuckDuckGo/SyncSettingsViewController.swift @@ -173,7 +173,10 @@ class SyncSettingsViewController: UIHostingController { func dismissPresentedViewController(completion: (() -> Void)? = nil) { guard let presentedViewController = navigationController?.presentedViewController, - !(presentedViewController is UIHostingController) else { return } + !(presentedViewController is UIHostingController) else { + completion?() + return + } presentedViewController.dismiss(animated: true, completion: completion) endConnectMode() } @@ -190,7 +193,8 @@ class SyncSettingsViewController: UIHostingController { let devices = try await syncService.fetchDevices() mapDevices(devices) } catch { - handleError(SyncError.unableToGetDevices, error: error) + // Not displaying error since there is the spinner and it is called every few seconds +// assertionFailure(error.localizedDescription) } } } From 696c615fb240e160a3d88b7fdac9d17c0017b149 Mon Sep 17 00:00:00 2001 From: Sabrina Tardio Date: Wed, 13 Dec 2023 14:20:28 +0100 Subject: [PATCH 5/6] log device error --- DuckDuckGo/SyncSettingsViewController.swift | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/DuckDuckGo/SyncSettingsViewController.swift b/DuckDuckGo/SyncSettingsViewController.swift index 0044c1c3cc..ec039fb9a1 100644 --- a/DuckDuckGo/SyncSettingsViewController.swift +++ b/DuckDuckGo/SyncSettingsViewController.swift @@ -22,6 +22,7 @@ import Core import Combine import SyncUI import DDGSync +import Common @MainActor class SyncSettingsViewController: UIHostingController { @@ -194,7 +195,7 @@ class SyncSettingsViewController: UIHostingController { mapDevices(devices) } catch { // Not displaying error since there is the spinner and it is called every few seconds -// assertionFailure(error.localizedDescription) + os_log(error.localizedDescription, log: .syncLog, type: .error) } } } From d620deb9c20f4e6b88cf7f02a7aa20fe14234bb4 Mon Sep 17 00:00:00 2001 From: Sabrina Tardio Date: Wed, 13 Dec 2023 14:24:31 +0100 Subject: [PATCH 6/6] remove trailing space --- DuckDuckGo/SyncSettingsViewController.swift | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/DuckDuckGo/SyncSettingsViewController.swift b/DuckDuckGo/SyncSettingsViewController.swift index ec039fb9a1..50ddc47dec 100644 --- a/DuckDuckGo/SyncSettingsViewController.swift +++ b/DuckDuckGo/SyncSettingsViewController.swift @@ -174,7 +174,7 @@ class SyncSettingsViewController: UIHostingController { func dismissPresentedViewController(completion: (() -> Void)? = nil) { guard let presentedViewController = navigationController?.presentedViewController, - !(presentedViewController is UIHostingController) else { + !(presentedViewController is UIHostingController) else { completion?() return }