Skip to content

fix: suppress stale backup error toast #800

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
pwltr wants to merge 6 commits into
base: master
Choose a base branch
from
Open

fix: suppress stale backup error toast #800

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
aa91c56
fix: suppress stale backup error toast (#562)
pwltr Sep 25, 2026
72fff8e
chore: rename changelog fragment
pwltr Sep 25, 2026
38f2b2c
fix: recheck backup state before alerting
pwltr Sep 25, 2026
c58ffc3
fix: defer backup alert until observing
pwltr Sep 28, 2026
06824ec
fix: retry pending backups before foreground alerts
pwltr Oct 20, 2026
317736d
fix: bound foreground backup recovery window
pwltr Oct 20, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions Bitkit/AppScene.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1251,6 +1251,7 @@ struct AppScene: View {
isPaykitSceneActive = newPhase == .active
if newPhase != .inactive { isContactRefreshForeground = newPhase == .active }
Logger.info("Scene phase changed: \(newPhase)", context: "AppScene")
BackupService.shared.setAppActive(newPhase == .active)

let returnedFromBackground = foregroundReturnTracker.scenePhaseChanged(to: newPhase)

Expand Down
196 changes: 173 additions & 23 deletions Bitkit/Services/BackupService.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -114,12 +114,13 @@ class BackupService {
private var isObserving = false
private var isRestoring = false
private var isWiping = false
private var lastNotificationTime: UInt64 = 0
private var backupFailureNotificationGate = BackupFailureNotificationGate()

private let defaults: UserDefaults
private let backupData: ((BackupCategory) async throws -> Data)?
private let uploadBackup: (String, Data) async throws -> Void
private let waitForBackupDelay: () async throws -> Void
private let waitForForegroundRecoveryTimeout: () async throws -> Void
private let backupStatusesKey = "backupStatuses"
private var backupVersions: [BackupCategory: UInt64] = [:]

Expand Down Expand Up @@ -150,12 +151,16 @@ class BackupService {
},
waitForBackupDelay: @escaping () async throws -> Void = {
try await Task.sleep(nanoseconds: 5_000_000_000)
},
waitForForegroundRecoveryTimeout: @escaping () async throws -> Void = {
try await Task.sleep(for: .seconds(BackupService.backupFailureCheckInterval))
}
) {
self.defaults = defaults
self.backupData = backupData
self.uploadBackup = uploadBackup
self.waitForBackupDelay = waitForBackupDelay
self.waitForForegroundRecoveryTimeout = waitForForegroundRecoveryTimeout
let statuses = getAllBackupStatuses()
var clearedStatuses = statuses
for category in BackupCategory.allCases {
Expand Down Expand Up @@ -189,10 +194,76 @@ class BackupService {
}
}

func startObservingBackups() {
guard !shouldSkipBackup() else { return }
@discardableResult
func setAppActive(_ isActive: Bool) -> Task<Void, Never>? {
let shouldRecover = stateQueue.sync {
backupFailureNotificationGate.setActive(isActive)
return isActive && backupFailureNotificationGate.beginForegroundRecovery()
}

guard shouldRecover else { return nil }
return Task {
await waitForForegroundRetries()
self.stateQueue.sync {
self.backupFailureNotificationGate.endForegroundRecovery()
}
let isObserving = try? await ServiceQueue.background(.backup) {
self.isObserving
}
let shouldCheck = self.stateQueue.sync {
self.backupFailureNotificationGate.shouldCheckForFailedBackups(
isObserving: isObserving == true
)
}

guard shouldCheck else { return }
self.checkForFailedBackups()

@jvsena42 jvsena42 Oct 20, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Manual Test 1 still shows the stale failure toast on unlock, the same as master. On foreground this runs checkForFailedBackups() straight away. A category whose upload failed before the lock is still isRequired && !running, with required over 30 minutes old. Nothing retries it before the check, so connectivity restored while locked still produces "Data Backup Failure … Retrying in 1 minute." 1 ms after the scene turns active.

The failed category is retried only on the next cold launch. BLOCKTANK is the only category with a periodic retry.

Master shows the same toast (fired while the scene is still inactive, visible on unlock). The gate moves when the toast fires but doesn't change what the user sees, which is the scenario this PR sets out to fix.

Fix: before the foreground check, retry the categories that are required and not running, and check only after that attempt settles. Alternatively, skip the check for categories whose last failure is older than the current foreground session and let the retry report. BackupFailureNotificationGateTests would then need a case where the backup recovers on foreground.

Reproduction

On the iPhone 17 simulator, with a scratch-only fault hook that is not in this PR: VssBackupClient.putObject throws notConnectedToInternet while a flag file exists. LDK's VSS path is untouched.

  1. Backups clean, toggle Swipe Balance to Hide (SETTINGS fails), lock.
  2. After 32 minutes, remove the block while locked, then unlock.

PR c58ffc3:

[13:17:19.937 UTC] INFO: Scene phase changed: inactive - AppScene
[13:17:20.670 UTC] INFO: Scene phase changed: active - AppScene
[13:17:20.671 UTC] WARN: Backup failed for more than 30 minutes - BackupService [BackupService.swift: showBackupFailureNotification() line: 774]
[13:17:26.275 UTC] DEBUG: Backup starting for: 'BLOCKTANK' - BackupService
(no 'SETTINGS' line until the cold launch at 13:20:16, which retries and succeeds by 13:20:31)

master 5663543, same hook and steps:

[13:53:33.679 UTC] WARN: Backup failed for more than 30 minutes - BackupService [showBackupFailureNotification(currentTime:)]
[13:53:34.027 UTC] INFO: Scene phase changed: inactive - AppScene
[13:53:34.480 UTC] INFO: Scene phase changed: active - AppScene
[13:53:40.340 UTC] DEBUG: Backup starting for: 'BLOCKTANK'   (SETTINGS not retried)

@pwltr pwltr Oct 20, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 06824ec. Foregrounding now retries required, idle app-managed backups before checking for overdue failures. Failure notifications are suppressed until that recovery pass settles, and running or scheduled backups are not duplicated. LDK-managed backups remain with ldk-node.

Retries preserve the original pending timestamp and the existing ten-minute notification cooldown: a successful retry produces no stale toast, while a still-failing overdue backup remains eligible for an alert. Cancelled recovery also retains the pending requirement.

Added service-level regression coverage for recovery across all app-managed categories, continued failures and cooldown, concurrent/scheduled backups, cancellation, and restore/wipe suppression. All 30 focused backup tests pass on a separate simulator. These use isolated backup data and backdated pending timestamps; the actual device lock/unlock scenario still needs rerunning on this commit. PR scope and QA notes are updated.

@jvsena42 jvsena42 Oct 20, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

06824ec looks right in code:

  • setAppActive(true) opens a recovery window, awaits retryPendingBackups() for required idle categories, and only then runs checkForFailedBackups();
  • requiredAtStart keeps the original age, so a retry that still fails is reported.

I'm leaving this open until the lock/unlock re-run on the device, and I'm checking one edge (a retry that never returns) separately.

}
}

private func waitForForegroundRetries() async {
let (settled, finish) = AsyncStream<Void>.makeStream()
// Bound alert suppression, not the uploads: an FFI write may ignore cancellation.
Task {
await self.retryPendingBackups()
finish.finish()
}
let timeout = Task {
do {
try await self.waitForForegroundRecoveryTimeout()
try Task.checkCancellation()
finish.finish()
} catch {}
}
for await _ in settled {}
timeout.cancel()
}

private func retryPendingBackups() async {
let tasks = try? await ServiceQueue.background(.backup) { [self] in
guard isObserving, !shouldSkipBackup() else { return [Task<BackupOutcome, Never>]() }
return BackupCategory.allCases.compactMap { category -> Task<BackupOutcome, Never>? in
guard category != .lightningConnections else { return nil }
let status = getBackupStatus(category: category)
guard status.isRequired, !status.running,
backupJobs[category] == nil, runningBackupTasks[category] == nil
else { return nil }

let task = Task { await self.runBackup(category: category, isScheduled: true) }
runningBackupTasks[category] = task
return task
}
}
for task in tasks ?? [] {
_ = await task.value
}
}

@discardableResult
func startObservingBackups() -> Task<Void, Never>? {
guard !shouldSkipBackup() else { return nil }

return Task {
let shouldStart = try? await ServiceQueue.background(.backup) {
guard !self.isObserving else { return false }
self.isObserving = true
Expand Down Expand Up @@ -254,13 +325,19 @@ class BackupService {
var outcome = BackupOutcome.failed
var requiredAtStart: UInt64 = 0
var versionAtStart: UInt64 = 0
let backupTime = UInt64(Date().timeIntervalSince1970)
updateBackupStatus(category: category) { status in
requiredAtStart = max(status.required, status.synced + 1, UInt64(Date().timeIntervalSince1970))
// Retrying must not restart the age of a backup that is already overdue.
requiredAtStart = status.isRequired ? status.required : max(status.required, status.synced + 1, backupTime)
versionAtStart = self.backupVersions[category, default: 0]
return BackupItemStatus(synced: status.synced, required: requiredAtStart, running: true)
}

do {
try Task.checkCancellation()
if isScheduled, shouldSkipBackup() {
throw CancellationError()
}
if category == .wallet {
try await PaykitPaymentActivity.shared.waitUntilIdle()
if isScheduled {
Expand All @@ -270,12 +347,16 @@ class BackupService {
}
} else {
let data = try await getBackupDataBytes(category: category)
try Task.checkCancellation()
if isScheduled, shouldSkipBackup() {
throw CancellationError()
}
try await uploadBackup(category.rawValue, data)
}
updateBackupStatus(category: category) { status in
let changed = self.backupVersions[category, default: 0] != versionAtStart
return BackupItemStatus(
synced: changed ? min(requiredAtStart, status.required - 1) : requiredAtStart,
synced: changed ? min(requiredAtStart, status.required - 1) : max(requiredAtStart, backupTime),
required: status.required,
running: false
)
Expand All @@ -286,7 +367,7 @@ class BackupService {
updateBackupStatus(category: category) { status in
BackupItemStatus(
synced: status.synced,
required: category == .wallet ? status.required : status.synced,
required: status.required,
running: false
)
}
Expand All @@ -299,6 +380,7 @@ class BackupService {
Logger.warn("Wallet backup deferred after three changing snapshots", context: "BackupService")
} else {
Logger.error("Backup failed for: '\(category.rawValue)': \(error)", context: "BackupService")
checkForFailedBackups()
}
}
_ = try? await ServiceQueue.background(.backup) { self.runningBackupTasks.removeValue(forKey: category) }
Expand Down Expand Up @@ -396,7 +478,9 @@ class BackupService {
// VssBackupClient's existing custom backup namespace is explicitly wallet index 0.
let backupWallet = try await PaykitPaymentStateBackup.ActiveOnchainAttempt.Wallet.backupNamespace(index: 0)
try await PaykitPaymentProofService.shared.restoreBackup(paymentState, wallet: backupWallet)
if let usdt = paymentState.usdt { try await PaykitUsdtPaymentService.shared.restoreBackup(usdt) }
if let usdt = paymentState.usdt {
try await PaykitUsdtPaymentService.shared.restoreBackup(usdt)
}
}
try TransferStorage.shared.upsertList(payload.transfers)
await PrivatePaykitAddressReservationStore.shared.restoreBackup(payload.privatePaykitHighestReservedReceiveIndexByAddressType)
Expand Down Expand Up @@ -701,7 +785,9 @@ class BackupService {
}
guard getBackupStatus(category: category).isRequired && !shouldSkipBackup() else { break }
outcome = await triggerBackup(category: category, isScheduled: true)
if !outcome.canRetryPendingBackup || !getBackupStatus(category: category).isRequired { break }
if !outcome.canRetryPendingBackup || !getBackupStatus(category: category).isRequired {
break
}
}
_ = try? await ServiceQueue.background(.backup) { self.backupJobs.removeValue(forKey: category) }
updateBackupStatus(category: category) { status in
Expand All @@ -716,27 +802,36 @@ class BackupService {

private func checkForFailedBackups() {
let currentTime = UInt64(Date().timeIntervalSince1970)

let hasFailedBackups = BackupCategory.allCases.contains { category in
let status = getBackupStatus(category: category)
return status.isRequired &&
currentTime > status.required &&
(currentTime - status.required) > Self.failedBackupCheckTime
if hasFailedBackups(at: currentTime) {
showBackupFailureNotification()
}
}

if hasFailedBackups {
showBackupFailureNotification(currentTime: currentTime)
private func hasFailedBackups(at currentTime: UInt64) -> Bool {
BackupCategory.allCases.contains { category in
let status = getBackupStatus(category: category)
return BackupFailureNotificationGate.hasFailedBackup(
status,
at: currentTime,
failureAge: Self.failedBackupCheckTime
)
}
}

private func showBackupFailureNotification(currentTime: UInt64) {
private func showBackupFailureNotification() {
Task {
try? await ServiceQueue.background(.backup) {
if currentTime - self.lastNotificationTime < Self.failedBackupNotificationInterval {
return
guard self.isObserving, !self.shouldSkipBackup() else { return }
let currentTime = UInt64(Date().timeIntervalSince1970)
guard self.statusUpdateQueue.sync(execute: { self.hasFailedBackups(at: currentTime) }) else { return }

let shouldNotify = self.stateQueue.sync {
self.backupFailureNotificationGate.shouldNotify(
at: currentTime,
notificationInterval: Self.failedBackupNotificationInterval
)
}

self.lastNotificationTime = currentTime
guard shouldNotify else { return }

let backupCheckIntervalMinutes = Int(Self.backupFailureCheckInterval / 60)
self.backupFailureSubject.send(backupCheckIntervalMinutes)
Comment thread
pwltr marked this conversation as resolved.
Expand Down Expand Up @@ -855,7 +950,9 @@ class BackupService {
}

private func getBackupDataBytes(category: BackupCategory, usdt: String? = nil) async throws -> Data {
if let backupData { return try await backupData(category) }
if let backupData {
return try await backupData(category)
}
switch category {
case .settings:
let settingsDict = await SettingsViewModel.shared.getSettingsDictionary()
Expand Down Expand Up @@ -1106,7 +1203,9 @@ final class WalletBackupChangeTracker {
} catch WalletBackupSnapshotError.changed {
try Task.checkCancellation()
Logger.debug("Wallet snapshot changed during capture (attempt \(attempt)/3)", context: "BackupService")
if attempt == 3 { throw WalletBackupSnapshotError.changed }
if attempt == 3 {
throw WalletBackupSnapshotError.changed
}
}
}
throw WalletBackupSnapshotError.changed
Expand Down Expand Up @@ -1135,3 +1234,54 @@ actor WalletBackupWriter {
}
}
}

struct BackupFailureNotificationGate {
private var isActive = false
private var isRecovering = false
private var lastNotificationTime: UInt64?

mutating func setActive(_ isActive: Bool) {
self.isActive = isActive
}

mutating func beginForegroundRecovery() -> Bool {
guard !isRecovering else { return false }
isRecovering = true
return true
}

mutating func endForegroundRecovery() {
isRecovering = false
}

func shouldCheckForFailedBackups(isObserving: Bool) -> Bool {
isActive && isObserving
}

static func hasFailedBackup(
_ status: BackupItemStatus,
at currentTime: UInt64,
failureAge: UInt64
) -> Bool {
status.isRequired && !status.running && currentTime >= status.required &&
Comment thread
pwltr marked this conversation as resolved.
currentTime - status.required > failureAge
}

mutating func shouldNotify(
at currentTime: UInt64,
notificationInterval: UInt64
) -> Bool {
guard isActive, !isRecovering else { return false }

if let lastNotificationTime {
guard currentTime >= lastNotificationTime,
currentTime - lastNotificationTime >= notificationInterval
else {
return false
}
}

lastNotificationTime = currentTime
return true
}
}
Loading
Loading