signalapp/Signal-iOS
signalapp/Signal-iOS
1
0
Fork 0
Code Issues Pull Requests Actions 1 Packages Projects Releases 4 Wiki Activity

Improve cancellation/error handling around some QuickRestore operations

Browse Source
This commit is contained in:
Pete Walters 2025-08-07 11:57:53 -05:00 committed by GitHub
parent fb74d4151f
commit 8c5bccfcf0
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
11 changed files with 274 additions and 38 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

92
Signal/DeviceTransfer/OutgoingDeviceRestorePresenter.swift
View File

@ -156,7 +156,8 @@ class OutgoingDeviceRestorePresenter: OutgoingDeviceRestoreInitialPresenter {
}
guard await viewModel.confirmTransfer() else {
// TODO: [Backups] - Display an error
// Silently fail here. The confirmTransfer UI will notify the user of
// success/failure (e.g. FaceID UI)
return
}
@ -171,8 +172,8 @@ class OutgoingDeviceRestorePresenter: OutgoingDeviceRestoreInitialPresenter {
await displayRestoreMessage(isBackup: false, presentingViewController: presentingViewController)
case .deviceTransfer:
guard let peerConnectionData = restoreMethodData.peerConnectionData else {
// TODO: [Backups] - Display an error
throw OWSAssertionError("Missing transfer connection data")
Logger.error("Missing transfer connection data")
throw DeviceRestoreError.invalidRestoreData
}
// Push the status sheet if this is a transfer
@ -182,20 +183,91 @@ class OutgoingDeviceRestorePresenter: OutgoingDeviceRestoreInitialPresenter {
)
await viewModel.waitForDeviceConnection(peerConnectionData: peerConnectionData)
Task { @MainActor in
Task { @MainActor [weak self] in
// TODO: [Backups] - DeviceTransferService does a db.write
// internally, and this should be updated to an actor/async aware
// in a followup piece of work (and possibly once the old device transfer
// flow is removed)
try viewModel.startTransfer(peerConnectionData: peerConnectionData)
do {
try viewModel.startTransfer(peerConnectionData: peerConnectionData)
} catch {
Logger.warn("Device transfer failed: \(error)")
await self?.handleError(
DeviceRestoreError.unknownError,
presentingViewController: presentingViewController
)
}
}
let success = await viewModel.waitForTransferCompletion()
if !success {
await handleError(
DeviceRestoreError.restoreCancelled,
presentingViewController: presentingViewController
)
} else {
await displayTransferComplete(presentingViewController: presentingViewController)
}
await viewModel.waitForTransferCompletion()
await displayTransferComplete(presentingViewController: presentingViewController)
}
} catch {
// TODO: [Backups] - Display an error
Logger.error("error: \(error)")
switch error {
case let restoreError as DeviceRestoreError:
await handleError(restoreError, presentingViewController: presentingViewController)
default:
Logger.error("Unexpected device transfer error: \(error)")
}
}
}
@MainActor
func handleError(_ error: DeviceRestoreError, presentingViewController: UIViewController?) async {
guard let presentingViewController else {
Logger.warn("Cannot display transfer error")
return
}
let (title, body) = switch error {
case .invalidRestoreData: (
OWSLocalizedString(
"OUTGOING_DEVICE_REGISTRATION_FAILED_RESTORE_TITLE",
comment: "Title of prompt notifying restore failed."
),
OWSLocalizedString(
"OUTGOING_DEVICE_REGISTRATION_FAILED_RESTORE_BODY",
comment: "Body of prompt notifying restore failed."
)
)
case .restoreCancelled: (
OWSLocalizedString(
"OUTGOING_DEVICE_REGISTRATION_CANCELLED_RESTORE_TITLE",
comment: "Title of prompt notifying restore was cancelled."
),
OWSLocalizedString(
"OUTGOING_DEVICE_REGISTRATION_CANCELLED_RESTORE_BODY",
comment: "Body of prompt notifying restore was cancelled."
)
)
case .unknownError: (
OWSLocalizedString(
"OUTGOING_DEVICE_REGISTRATION_UNKNOWN_ERROR_TITLE",
comment: "Title of prompt notifying restore failed for unknown reasons."
),
OWSLocalizedString(
"OUTGOING_DEVICE_REGISTRATION_UNKNOWN_ERROR_BODY",
comment: "Body of prompt notifying restore failed for unknown reasons."
)
)
}
let sheet = HeroSheetViewController(
hero: .image(UIImage(resource: .checkCircle)),
title: title,
body: body,
primaryButton: .init(title: CommonStrings.okayButton, action: { _ in
presentingViewController.dismiss(animated: true)
})
)
sheet.modalPresentationStyle = .formSheet
await presentingViewController.awaitableDismiss(animated: true)
await presentingViewController.awaitablePresent(sheet, animated: true)
}
}

56
Signal/DeviceTransfer/OutgoingDeviceRestoreViewModel.swift
View File

@ -7,6 +7,12 @@ import Foundation
import SignalServiceKit
import MultipeerConnectivity
enum DeviceRestoreError: Error {
case invalidRestoreData
case restoreCancelled
case unknownError
}
class OutgoingDeviceRestoreViewModel: ObservableObject, DeviceTransferServiceObserver {
struct RestoreMethodData {
@ -35,7 +41,7 @@ class OutgoingDeviceRestoreViewModel: ObservableObject, DeviceTransferServiceObs
)?> = AtomicValue(nil, lock: .init())
private var finishTransferContinuation: AtomicValue<
CheckedContinuation<Void, Never>?
CheckedContinuation<Bool, Never>?
> = AtomicValue(nil, lock: .init())
init(
@ -46,6 +52,10 @@ class OutgoingDeviceRestoreViewModel: ObservableObject, DeviceTransferServiceObs
self.deviceTransferService = deviceTransferService
self.quickRestoreManager = quickRestoreManager
self.provisioningURL = deviceProvisioningURL
transferStatusViewModel.onCancel = { [weak self] in
self?.cancelTransfer()
}
}
func confirmTransfer() async -> Bool {
@ -57,9 +67,16 @@ class OutgoingDeviceRestoreViewModel: ObservableObject, DeviceTransferServiceObs
/// 2. Outgoing device will wait for the restore method choice from the other device.
/// 3. Confirm the returned choice is 'device transfer' or fail.
/// 4. Parse out the MPC connection information returned in the restore method choice, and return this connection data
func waitForRestoreMethodResponse() async throws -> RestoreMethodData {
let restoreMethodToken = try await quickRestoreManager.register(deviceProvisioningUrl: provisioningURL)
let restoreMethod = try await quickRestoreManager.waitForRestoreMethodChoice(restoreMethodToken: restoreMethodToken)
func waitForRestoreMethodResponse() async throws(DeviceRestoreError) -> RestoreMethodData {
let restoreMethod: QuickRestoreManager.RestoreMethodType
do {
let token = try await quickRestoreManager.register(deviceProvisioningUrl: provisioningURL)
restoreMethod = try await quickRestoreManager.waitForRestoreMethodChoice(restoreMethodToken: token)
} catch {
Logger.error("Failed to wait for restore method choice: \(error)")
throw DeviceRestoreError.invalidRestoreData
}
guard case let .deviceTransfer(transferData) = restoreMethod else {
return RestoreMethodData(restoreMethod: restoreMethod, peerConnectionData: nil)
}
@ -68,7 +85,8 @@ class OutgoingDeviceRestoreViewModel: ObservableObject, DeviceTransferServiceObs
let urlString = String(data: stringData, encoding: .utf8),
let transferURL = URL(string: urlString)
else {
throw OWSAssertionError("Attempting to restore using a method other than device transfer")
Logger.error("Attempting to restore using a method other than device transfer")
throw DeviceRestoreError.invalidRestoreData
}
do {
@ -81,8 +99,8 @@ class OutgoingDeviceRestoreViewModel: ObservableObject, DeviceTransferServiceObs
)
)
} catch {
Logger.error("Failed to register device via URL: \(error)")
throw error
Logger.error("Failed to parse transfer URL: \(error)")
throw DeviceRestoreError.invalidRestoreData
}
}
@ -120,14 +138,20 @@ class OutgoingDeviceRestoreViewModel: ObservableObject, DeviceTransferServiceObs
}
}
func waitForTransferCompletion() async {
func waitForTransferCompletion() async -> Bool {
return await withCheckedContinuation { continuation in
self.finishTransferContinuation.update {
switch transferStatusViewModel.state {
case .done, .error:
case .done:
// If the transfer is finished, just return
continuation.resume()
continuation.resume(returning: true)
return
case .cancelled:
continuation.resume(returning: false)
return
case .error(let error):
Logger.warn("Transfer failed: \(error)")
continuation.resume(returning: false)
case .connecting, .idle, .starting, .transferring:
break
}
@ -136,6 +160,15 @@ class OutgoingDeviceRestoreViewModel: ObservableObject, DeviceTransferServiceObs
}
}
private func cancelTransfer() {
stopListeningForTransfer()
transferStatusViewModel.state = .cancelled
deviceTransferService.cancelTransferFromOldDevice()
deviceTransferService.stopTransfer()
let continuation = finishTransferContinuation.swap(nil)
continuation?.resume(returning: false)
}
private func stopListeningForTransfer() {
deviceTransferService.removeObserver(self)
deviceTransferService.stopListeningForNewDevices()
@ -169,10 +202,11 @@ class OutgoingDeviceRestoreViewModel: ObservableObject, DeviceTransferServiceObs
finishTransferContinuation.update { continuation in
if let error {
transferStatusViewModel.state = .error(error)
continuation?.resume(returning: false)
} else {
transferStatusViewModel.state = .done
continuation?.resume(returning: true)
}
continuation?.resume()
continuation = nil
}
}

52
Signal/DeviceTransfer/TransferStatusState.swift
View File

@ -12,7 +12,8 @@ enum TransferState {
case connecting
case transferring(Double)
case done
case error(Error)
case cancelled
case error(DeviceTransferService.Error)
}
class TransferStatusViewModel: ObservableObject {
@ -20,34 +21,66 @@ class TransferStatusViewModel: ObservableObject {
enum Indefinite {
case starting
case connecting
case cancelling
func title(isNewDevice: Bool) -> String {
switch self {
case .starting:
if isNewDevice {
"Waiting to connect to old iPhone…" // TODO: Localize
OWSLocalizedString(
"DEVICE_TRANSFER_STATUS_NEW_DEVICE_STARTING",
comment: "Status message on new device when transfer is starting."
)
} else {
"Waiting to connect to new iPhone…" // TODO: Localize
OWSLocalizedString(
"DEVICE_TRANSFER_STATUS_OLD_DEVICE_STARTING",
comment: "Status message on old device when transfer is starting."
)
}
case .connecting:
if isNewDevice {
"Connecting to old iPhone…" // TODO: Localize
OWSLocalizedString(
"DEVICE_TRANSFER_STATUS_NEW_DEVICE_CONNECTING",
comment: "Status message on new device when connecting to old device."
)
} else {
"Connecting to new iPhone…" // TODO: Localize
OWSLocalizedString(
"DEVICE_TRANSFER_STATUS_OLD_DEVICE_CONNECTING",
comment: "Status message on new device when connecting to new device."
)
}
case .cancelling:
if isNewDevice {
OWSLocalizedString(
"DEVICE_TRANSFER_STATUS_NEW_DEVICE_CANCELLING",
comment: "Status message on new device when cancelling transfer."
)
} else {
OWSLocalizedString(
"DEVICE_TRANSFER_STATUS_OLD_DEVICE_CANCELLING",
comment: "Status message on old device when cancelling transfer."
)
}
}
}
func message(isNewDevice: Bool) -> String {
if isNewDevice {
"Bring your old device nearby, and make sure Wi-Fi and Bluetooth are enabled." // TODO: Localize
OWSLocalizedString(
"DEVICE_TRANSFER_STATUS_NEW_DEVICE_CONNECTING_MESSAGE",
comment: "Description message on new device displayed during device transfer."
)
} else {
"Bring your new device nearby, and make sure Wi-Fi and Bluetooth are enabled." // TODO: Localize
OWSLocalizedString(
"DEVICE_TRANSFER_STATUS_OLD_DEVICE_CONNECTING_MESSAGE",
comment: "Description message on old device displayed during device transfer."
)
}
}
}
case indefinite(Indefinite)
case transferring(Double)
case error(Error)
}
@Published var viewState: ViewState = .indefinite(.starting)
@ -63,7 +96,10 @@ class TransferStatusViewModel: ObservableObject {
self.progressDidUpdate(currentProgress: progress)
case .done:
viewState = .transferring(1)
case .error(_):
case .cancelled:
viewState = .indefinite(.cancelling)
case .error(let error):
viewState = .error(error)
return
}
}

11
Signal/Provisioning/QuickRestoreManager.swift
View File

@ -80,8 +80,15 @@ public class QuickRestoreManager {
case .disabled, .disabling: nil
}
let lastBackupTime = backupSettingsStore.lastBackupDate(tx: tx)?.ows_millisecondsSince1970
let lastBackupSizeBytes = backupSettingsStore.lastBackupSizeBytes(tx: tx)
let lastBackupTime: UInt64?
let lastBackupSizeBytes: UInt64?
if backupTier != nil {
lastBackupTime = backupSettingsStore.lastBackupDate(tx: tx)?.ows_millisecondsSince1970
lastBackupSizeBytes = backupSettingsStore.lastBackupSizeBytes(tx: tx)
} else {
lastBackupTime = nil
lastBackupSizeBytes = nil
}
return (
localIdentifiers,

5
Signal/Registration/RegistrationCoodinatorShims.swift
View File

@ -109,6 +109,7 @@ protocol _RegistrationCoordinator_DeviceTransferServiceShim {
func addObserver(_ observer: DeviceTransferServiceObserver)
func removeObserver(_ observer: DeviceTransferServiceObserver)
func stopAcceptingTransfersFromOldDevices()
func cancelTransferFromOldDevice()
}
class _RegistrationCoordinator_DeviceTransferServiceWrapper: _RegistrationCoordinator_DeviceTransferServiceShim {
@ -132,6 +133,10 @@ class _RegistrationCoordinator_DeviceTransferServiceWrapper: _RegistrationCoordi
func stopAcceptingTransfersFromOldDevices() {
deviceTransferService.stopAcceptingTransfersFromOldDevices()
}
func cancelTransferFromOldDevice() {
deviceTransferService.cancelTransferFromOldDevice()
}
}
// MARK: - ExperienceManager

2
Signal/Registration/RegistrationCoordinatorImpl.swift
View File

@ -538,6 +538,8 @@ public class RegistrationCoordinatorImpl: RegistrationCoordinator {
}
public func resetRestoreMode() -> Guarantee<RegistrationStep> {
inMemoryState.registrationMessage = nil
inMemoryState.accountEntropyPool = nil
db.write { tx in
self.updatePersistedState(tx) {
$0.shouldSkipRegistrationSplash = false

2
Signal/Registration/UserInterface/RegistrationNavigationController.swift
View File

@ -734,7 +734,7 @@ extension RegistrationNavigationController: RegistrationQuickRestoreQRCodePresen
extension RegistrationNavigationController: RegistrationTransferStatusPresenter {
func cancelTransfer() {
let guarantee = coordinator.resetRestoreMethodChoice()
let guarantee = coordinator.resetRestoreMode()
pushNextController(guarantee)
}
}

5
Signal/Registration/UserInterface/RegistrationTransferStatusState.swift
View File

@ -21,6 +21,7 @@ public class RegistrationTransferStatusState: DeviceTransferServiceObserver, Equ
set {
transferStatusViewModel.onCancel = { [weak self] in
self?.stopAcceptingTransfers()
self?.cancelTransfer()
newValue()
}
}
@ -60,6 +61,10 @@ public class RegistrationTransferStatusState: DeviceTransferServiceObserver, Equ
)
}
private func cancelTransfer() {
deviceTransferService.cancelTransferFromOldDevice()
}
public func stopAcceptingTransfers() {
deviceTransferService.removeObserver(self)
deviceTransferService.stopAcceptingTransfersFromOldDevices()

31
Signal/Registration/UserInterface/RegistrationTransferStatusViewController.swift
View File

@ -66,15 +66,12 @@ class RegistrationTransferStatusViewController: HostingController<TransferStatus
do {
try await state.start()
} catch {
Logger.error("ERROR: \(error)")
// TODO: [Backups] - Display an error to the user
}
}
}
override func viewDidDisappear(_ animated: Bool) {
state.onCancel()
super.viewDidDisappear(animated)
}
}
struct TransferStatusView: View {
@ -99,12 +96,18 @@ struct TransferStatusView: View {
.font(.body)
.foregroundStyle(Color.Signal.secondaryLabel)
case .transferring(let progress):
Text(verbatim: "Transferring") // TODO: Localize
Text(OWSLocalizedString(
"DEVICE_TRANSFER_STATUS_NEW_DEVICE_TRANSFERRING",
comment: "Title for a progress view displayed during device transfer."
))
.font(.title2.weight(.semibold))
.foregroundStyle(Color.Signal.label)
.padding(.top, 44)
.padding(.bottom, 2)
Text(verbatim: "Keep both devices near each other. Do not turn off either device and keep Signal open. Transfers are end-to-end encrypted.") // TODO: Localize
Text(OWSLocalizedString(
"DEVICE_TRANSFER_STATUS_NEW_DEVICE_TRANSFERRING_DESCRIPTION",
comment: "Description in the progress view displayed during device transfer."
))
.font(.body)
.foregroundStyle(Color.Signal.secondaryLabel)
@ -118,6 +121,22 @@ struct TransferStatusView: View {
Text(viewModel.progressEstimateLabel)
.foregroundStyle(Color.Signal.secondaryLabel)
Spacer()
case .error(_):
Text(OWSLocalizedString(
"DEVICE_TRANSFER_STATUS_NEW_DEVICE_TRANSFER_FAILED_TITLE",
comment: "Title for a progress view displayed after failure of device transfer."
))
.font(.title2.weight(.semibold))
.foregroundStyle(Color.Signal.label)
.padding(.top, 44)
.padding(.bottom, 2)
Text(OWSLocalizedString(
"DEVICE_TRANSFER_STATUS_NEW_DEVICE_TRANSFER_FAILED_BODY",
comment: "Description in the progress view displayed after failure of device transfer."
))
.font(.body)
.foregroundStyle(Color.Signal.secondaryLabel)
Spacer()
}
Spacer()

2
Signal/test/Registration/RegistrationCoordinatorTestShims.swift
View File

@ -76,6 +76,8 @@ public class _RegistrationCoordinator_DeviceTransferServiceMock: _RegistrationCo
public func removeObserver(_ observer: any Signal.DeviceTransferServiceObserver) { }
public func stopAcceptingTransfersFromOldDevices() { }
public func cancelTransferFromOldDevice() { }
}
public class _RegistrationCoordinator_ExperienceManagerMock: _RegistrationCoordinator_ExperienceManagerShim {

54
Signal/translations/en.lproj/Localizable.strings
View File

@ -2542,6 +2542,42 @@
/* The title for the action sheet asking the user to scan the QR code to transfer */
"DEVICE_TRANSFER_SCANNING_TITLE" = "Hold Your New Device Up to the Camera";
/* Status message on new device when cancelling transfer. */
"DEVICE_TRANSFER_STATUS_NEW_DEVICE_CANCELLING" = "Cancelling connection to old iPhone…";
/* Status message on new device when connecting to old device. */
"DEVICE_TRANSFER_STATUS_NEW_DEVICE_CONNECTING" = "Connecting to old iPhone…";
/* Description message on new device displayed during device transfer. */
"DEVICE_TRANSFER_STATUS_NEW_DEVICE_CONNECTING_MESSAGE" = "Bring your old device nearby, and make sure Wi-Fi and Bluetooth are enabled.";
/* Status message on new device when transfer is starting. */
"DEVICE_TRANSFER_STATUS_NEW_DEVICE_STARTING" = "Waiting to connect to old iPhone…";
/* Description in the progress view displayed after failure of device transfer. */
"DEVICE_TRANSFER_STATUS_NEW_DEVICE_TRANSFER_FAILED_BODY" = "Transfer to new device failed. Bring your old device nearby, and please try again.";
/* Title for a progress view displayed after failure of device transfer. */
"DEVICE_TRANSFER_STATUS_NEW_DEVICE_TRANSFER_FAILED_TITLE" = "Transfer Failed";
/* Title for a progress view displayed during device transfer. */
"DEVICE_TRANSFER_STATUS_NEW_DEVICE_TRANSFERRING" = "Transferring";
/* Description in the progress view displayed during device transfer. */
"DEVICE_TRANSFER_STATUS_NEW_DEVICE_TRANSFERRING_DESCRIPTION" = "Keep both devices near each other. Do not turn off either device and keep Signal open. Transfers are end-to-end encrypted.";
/* Status message on old device when cancelling transfer. */
"DEVICE_TRANSFER_STATUS_OLD_DEVICE_CANCELLING" = "Cancelling connection to new iPhone…";
/* Status message on new device when connecting to new device. */
"DEVICE_TRANSFER_STATUS_OLD_DEVICE_CONNECTING" = "Connecting to new iPhone…";
/* Description message on old device displayed during device transfer. */
"DEVICE_TRANSFER_STATUS_OLD_DEVICE_CONNECTING_MESSAGE" = "Bring your new device nearby, and make sure Wi-Fi and Bluetooth are enabled.";
/* Status message on old device when transfer is starting. */
"DEVICE_TRANSFER_STATUS_OLD_DEVICE_STARTING" = "Waiting to connect to new iPhone…";
/* The explanation on the action sheet that shows transfer progress */
"DEVICE_TRANSFER_TRANSFERRING_EXPLANATION" = "Keep both devices near each other. Do not turn off either device and keep Signal open.";
@ -5671,12 +5707,30 @@
/* Label warning the user that they should update Signal to continue using payments. */
"OUTDATED_PAYMENT_CLIENT_REMINDER_TEXT" = "Update Signal to continue using payments. Your balance may not be up-to-date.";
/* Body of prompt notifying restore was cancelled. */
"OUTGOING_DEVICE_REGISTRATION_CANCELLED_RESTORE_BODY" = "Restore was cancelled. Please try again.";
/* Title of prompt notifying restore was cancelled. */
"OUTGOING_DEVICE_REGISTRATION_CANCELLED_RESTORE_TITLE" = "Restore cancelled";
/* Body of prompt notifying registration without restore completed on the new device. */
"OUTGOING_DEVICE_REGISTRATION_COMPLETE_BODY" = "Your Signal account has been activated on your other device. Signal is now inactive on this device.";
/* Title of prompt notifying registration without restore completed on the new device. */
"OUTGOING_DEVICE_REGISTRATION_COMPLETE_TITLE" = "Registration complete";
/* Body of prompt notifying restore failed. */
"OUTGOING_DEVICE_REGISTRATION_FAILED_RESTORE_BODY" = "Restore message from other device contained invalid or expired data. Please try again.";
/* Title of prompt notifying restore failed. */
"OUTGOING_DEVICE_REGISTRATION_FAILED_RESTORE_TITLE" = "Restore failed";
/* Body of prompt notifying restore failed for unknown reasons. */
"OUTGOING_DEVICE_REGISTRATION_UNKNOWN_ERROR_BODY" = "Unknown error, please try again.";
/* Title of prompt notifying restore failed for unknown reasons. */
"OUTGOING_DEVICE_REGISTRATION_UNKNOWN_ERROR_TITLE" = "Unknown error";
/* Body of prompt notifying device restore started on the new device. */
"OUTGOING_DEVICE_RESTORE_COMPLETE_BODY" = "Your Signal account and messages have started transferring to your other device. Signal is now inactive on this device.";