fix: resolve duplicate-file failures, alreadySafe reset, and backup restart loop
BackupEngine: after all retries fail, call fileExists() before recording a failure. If the file is on NAS (uploaded earlier but fileExists silently threw on the pre-check), count it as skipped and add it to manifestEntries — this ensures writeManifest includes every safe asset and prevents alreadySafe from dropping to 1 after the post-backup reconcile. BackupView: remove the immediate refreshAndWait from onChange(.failed). That call raced with refreshAfterBackup's writeManifest, cancelled the manifest- correct refresh, then reconciled against a stale 1-entry NAS manifest. Auto-resolve (.failed → .completed when needBackup == 0) is now owned by the coordinator so it runs after writeManifest finishes. AutoBackupCoordinator: add backupTriggerArmed flag (set by onActive/ handleNASReachable, cleared by handleReconciliationComplete). Post-backup reconciles from refreshAfterBackup are no longer able to re-trigger auto- backup — only an explicit app-open or LAN-join can arm the trigger. Also added auto-resolve: calls engine.resolveSuccess() when job is .failed but needBackup == 0, which fires the .completed green-flash path in BackupView. Co-Authored-By: Kutesir <kutesir@provoc.ug> Co-Authored-By: Sentry <sentry@provoc.ug>
This commit is contained in:
@@ -153,18 +153,11 @@ struct BackupView: View {
|
|||||||
try? await Task.sleep(nanoseconds: 500_000_000)
|
try? await Task.sleep(nanoseconds: 500_000_000)
|
||||||
statusService.refresh(force: true)
|
statusService.refresh(force: true)
|
||||||
}
|
}
|
||||||
} else if case .failed = status {
|
|
||||||
// Reconcile — if all files landed on NAS despite the failure, auto-resolve to .completed
|
|
||||||
Task {
|
|
||||||
await statusService.refreshAndWait(force: true)
|
|
||||||
if statusService.snapshot.needBackup == 0 {
|
|
||||||
engine.resolveSuccess()
|
|
||||||
withAnimation(.spring(response: 0.3, dampingFraction: 0.8)) { ctaFlashGreen = true }
|
|
||||||
try? await Task.sleep(nanoseconds: 2_500_000_000)
|
|
||||||
withAnimation(.spring(response: 0.5, dampingFraction: 0.8)) { ctaFlashGreen = false }
|
|
||||||
}
|
|
||||||
}
|
|
||||||
}
|
}
|
||||||
|
// .failed auto-resolve is owned by AutoBackupCoordinator.handleReconciliationComplete.
|
||||||
|
// It waits for refreshAfterBackup's writeManifest to finish before reconciling, then
|
||||||
|
// calls engine.resolveSuccess() when needBackup == 0, which re-triggers .completed above.
|
||||||
|
// Calling refreshAndWait here would race with writeManifest and read a stale NAS manifest.
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
@@ -33,6 +33,11 @@ final class AutoBackupCoordinator: ObservableObject {
|
|||||||
private var lastAutoCheckAt: Date = .distantPast
|
private var lastAutoCheckAt: Date = .distantPast
|
||||||
private var recheckTimer: AnyCancellable?
|
private var recheckTimer: AnyCancellable?
|
||||||
|
|
||||||
|
// One-shot flag armed by onActive()/handleNASReachable() and consumed by
|
||||||
|
// handleReconciliationComplete(). Prevents the post-backup refresh cycle from
|
||||||
|
// re-triggering auto-backup — only an explicit app-open or LAN-join should do that.
|
||||||
|
private var backupTriggerArmed = false
|
||||||
|
|
||||||
private let engine = BackupEngine.shared
|
private let engine = BackupEngine.shared
|
||||||
private let statusService = BackupStatusService.shared
|
private let statusService = BackupStatusService.shared
|
||||||
private let store = ConnectionStore.shared
|
private let store = ConnectionStore.shared
|
||||||
@@ -62,6 +67,7 @@ final class AutoBackupCoordinator: ObservableObject {
|
|||||||
Task { @MainActor [weak self] in
|
Task { @MainActor [weak self] in
|
||||||
self?.recheckTimer = nil
|
self?.recheckTimer = nil
|
||||||
self?.lastAutoCheckAt = .distantPast
|
self?.lastAutoCheckAt = .distantPast
|
||||||
|
self?.backupTriggerArmed = false
|
||||||
log.debug("App resigned — recheck timer stopped, debounce reset")
|
log.debug("App resigned — recheck timer stopped, debounce reset")
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
@@ -120,6 +126,7 @@ final class AutoBackupCoordinator: ObservableObject {
|
|||||||
}
|
}
|
||||||
|
|
||||||
lastAutoCheckAt = Date()
|
lastAutoCheckAt = Date()
|
||||||
|
backupTriggerArmed = true
|
||||||
phase = .checking
|
phase = .checking
|
||||||
startPeriodicRecheckIfNeeded()
|
startPeriodicRecheckIfNeeded()
|
||||||
log.info("onActive: trigger=\(lanTriggered ? "LAN" : "appOpen") — status check starting")
|
log.info("onActive: trigger=\(lanTriggered ? "LAN" : "appOpen") — status check starting")
|
||||||
@@ -134,13 +141,25 @@ final class AutoBackupCoordinator: ObservableObject {
|
|||||||
// No destination configured yet
|
// No destination configured yet
|
||||||
guard let conn = store.savedConnection, conn.remotePath != "/" else {
|
guard let conn = store.savedConnection, conn.remotePath != "/" else {
|
||||||
phase = .idle
|
phase = .idle
|
||||||
|
backupTriggerArmed = false
|
||||||
return
|
return
|
||||||
}
|
}
|
||||||
|
|
||||||
// Nothing to back up — clear any stale state
|
// Auto-resolve: engine reported failure but reconcile shows all assets are safe.
|
||||||
|
// This fires after refreshAfterBackup's writeManifest completes and updates the manifest,
|
||||||
|
// at which point needBackup correctly reflects what's still missing.
|
||||||
|
if case .failed = engine.job.status {
|
||||||
|
if snapshot.needBackup == 0 {
|
||||||
|
engine.resolveSuccess()
|
||||||
|
log.info("Auto-resolved .failed → .completed (needBackup == 0 after reconcile)")
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// Nothing to back up
|
||||||
guard snapshot.needBackup > 0 else {
|
guard snapshot.needBackup > 0 else {
|
||||||
phase = .idle
|
phase = .idle
|
||||||
pendingAtDisconnect = 0
|
pendingAtDisconnect = 0
|
||||||
|
backupTriggerArmed = false
|
||||||
return
|
return
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -148,23 +167,35 @@ final class AutoBackupCoordinator: ObservableObject {
|
|||||||
if snapshot.connectionState == .offline {
|
if snapshot.connectionState == .offline {
|
||||||
pendingAtDisconnect = snapshot.needBackup
|
pendingAtDisconnect = snapshot.needBackup
|
||||||
phase = .disconnected
|
phase = .disconnected
|
||||||
|
backupTriggerArmed = false
|
||||||
log.info("NAS offline — \(snapshot.needBackup) items queued")
|
log.info("NAS offline — \(snapshot.needBackup) items queued")
|
||||||
return
|
return
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// Only fire auto-backup when explicitly armed by onActive() or handleNASReachable().
|
||||||
|
// Post-backup reconciles (from refreshAfterBackup) must not re-trigger — that's the
|
||||||
|
// source of the restart loop when alreadySafe is temporarily wrong.
|
||||||
|
guard backupTriggerArmed else {
|
||||||
|
phase = .idle
|
||||||
|
log.info("handleReconciliationComplete: trigger not armed — skipping auto-backup")
|
||||||
|
return
|
||||||
|
}
|
||||||
|
|
||||||
// Gate auto-backup on trigger source:
|
// Gate auto-backup on trigger source:
|
||||||
// LAN join → autoBackupEnabled (lanTriggerEnabled guards the SSID check in LANMonitor)
|
// LAN join → autoBackupEnabled
|
||||||
// App open → autoBackupOnOpen (off by default; requires explicit opt-in)
|
// App open → autoBackupOnOpen (off by default)
|
||||||
let autoAllowed = lastTriggerWasLAN
|
let autoAllowed = lastTriggerWasLAN
|
||||||
? store.autoBackupEnabled
|
? store.autoBackupEnabled
|
||||||
: store.autoBackupOnOpen
|
: store.autoBackupOnOpen
|
||||||
|
|
||||||
guard autoAllowed else {
|
guard autoAllowed else {
|
||||||
phase = .idle
|
phase = .idle
|
||||||
|
backupTriggerArmed = false
|
||||||
log.info("Auto-backup gated — trigger=\(self.lastTriggerWasLAN ? "LAN" : "appOpen") enabled=\(autoAllowed)")
|
log.info("Auto-backup gated — trigger=\(self.lastTriggerWasLAN ? "LAN" : "appOpen") enabled=\(autoAllowed)")
|
||||||
return
|
return
|
||||||
}
|
}
|
||||||
|
|
||||||
|
backupTriggerArmed = false
|
||||||
triggerAutoBackupIfNeeded(conn: conn)
|
triggerAutoBackupIfNeeded(conn: conn)
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
@@ -306,17 +306,38 @@ final class BackupEngine: ObservableObject {
|
|||||||
}
|
}
|
||||||
|
|
||||||
if !uploadSucceeded, let error = lastUploadError {
|
if !uploadSucceeded, let error = lastUploadError {
|
||||||
logger.error("Upload permanently failed for \(asset.filename, privacy: .public): \(error.localizedDescription, privacy: .public)")
|
// Before recording a failure, verify the file isn't already on NAS.
|
||||||
job.fileFailed()
|
// fileExists() may have returned nil earlier due to a transient SMB error;
|
||||||
failed += 1
|
// if it's actually there, the asset is safe — count it as skipped, not failed.
|
||||||
if assetIdx < queueItems.count {
|
if (try? await transfer.fileExists(at: remotePath)) == true {
|
||||||
queueItems[assetIdx].status = .failed
|
job.fileCompleted(skipped: true)
|
||||||
queueItems[assetIdx].completedAt = Date()
|
skipped += 1
|
||||||
queueItems[assetIdx].error = BackupQueueItem.UploadError(
|
if assetIdx < queueItems.count {
|
||||||
code: (error as NSError).domain,
|
queueItems[assetIdx].status = .skipped
|
||||||
message: error.localizedDescription,
|
queueItems[assetIdx].completedAt = Date()
|
||||||
timestamp: Date()
|
}
|
||||||
)
|
manifestEntries.append(ManifestEntry(
|
||||||
|
localIdentifier: asset.localIdentifier,
|
||||||
|
filename: asset.filename,
|
||||||
|
creationDate: asset.creationDate,
|
||||||
|
fileSize: 0,
|
||||||
|
remotePath: remotePath,
|
||||||
|
uploadedAt: Date()
|
||||||
|
))
|
||||||
|
logger.info("Upload failed but file confirmed on NAS — marking safe: \(asset.filename, privacy: .public)")
|
||||||
|
} else {
|
||||||
|
logger.error("Upload permanently failed for \(asset.filename, privacy: .public): \(error.localizedDescription, privacy: .public)")
|
||||||
|
job.fileFailed()
|
||||||
|
failed += 1
|
||||||
|
if assetIdx < queueItems.count {
|
||||||
|
queueItems[assetIdx].status = .failed
|
||||||
|
queueItems[assetIdx].completedAt = Date()
|
||||||
|
queueItems[assetIdx].error = BackupQueueItem.UploadError(
|
||||||
|
code: (error as NSError).domain,
|
||||||
|
message: error.localizedDescription,
|
||||||
|
timestamp: Date()
|
||||||
|
)
|
||||||
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user