fix: NAS operation timeouts, fast-path warm-up, 30-min stale threshold
Three changes that together eliminate the "Checking…" stall and excessive NAS rescanning without requiring a database migration: NASManifestCache: extend stale threshold from 5 min to 30 min. directoryCount is now updated incrementally after every upload, and PHPhotoLibraryChangeObserver keeps LocalPhotoIndex current — a full NAS roundtrip every 5 min was redundant and forced the expensive reconcile path far more often than necessary. BackupStatusService: add withTimeout(_:work:) using a racing ThrowingTaskGroup so NAS operations (SMB connect, manifest download, directory listing) fail fast instead of hanging indefinitely when the NAS is slow or unreachable. Timeouts: 15s for connect, 12s for manifest download, 12s for directory listings. On timeout, BackupError.timeout is caught separately to preserve cached counts instead of marking connectionState = .offline. BackupStatusService: call LocalPhotoIndex.shared.loadOrBuild() at the start of performRefresh before checking localIndexCount. Without this, AppDelegate's concurrent loadOrBuild() task races with the first onActive() trigger; if it loses, localIndexCount == 0 and the fast path is bypassed, forcing a full NAS roundtrip on every cold launch. Co-Authored-By: Kutesir <kutesir@provoc.ug> Co-Authored-By: Sentry <sentry@provoc.ug>
This commit is contained in:
@@ -100,6 +100,11 @@ final class BackupStatusService: NSObject, ObservableObject {
|
|||||||
return
|
return
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// Ensure LocalPhotoIndex is loaded from disk before reconcile.
|
||||||
|
// loadOrBuild() is a fast no-op when already in memory; on cold launch it
|
||||||
|
// reads the cached JSON so the fast path (skip NAS roundtrip) can fire.
|
||||||
|
await LocalPhotoIndex.shared.loadOrBuild()
|
||||||
|
|
||||||
lastFullRefresh = Date()
|
lastFullRefresh = Date()
|
||||||
isRefreshing = true
|
isRefreshing = true
|
||||||
refreshError = nil
|
refreshError = nil
|
||||||
@@ -110,6 +115,13 @@ final class BackupStatusService: NSObject, ObservableObject {
|
|||||||
log.info("Refresh completed — phone=\(self.snapshot.phoneTotal) safe=\(self.snapshot.alreadySafe)")
|
log.info("Refresh completed — phone=\(self.snapshot.phoneTotal) safe=\(self.snapshot.alreadySafe)")
|
||||||
} catch is CancellationError {
|
} catch is CancellationError {
|
||||||
log.info("Refresh cancelled")
|
log.info("Refresh cancelled")
|
||||||
|
} catch BackupError.timeout {
|
||||||
|
// NAS did not respond within the timeout window.
|
||||||
|
// Retain cached counts so the UI shows useful data instead of zeroes.
|
||||||
|
log.warning("Refresh timed out — retaining cached state")
|
||||||
|
refreshError = "NAS not responding — using last known state"
|
||||||
|
snapshot.lastCheckedAt = Date()
|
||||||
|
saveSnapshot()
|
||||||
} catch {
|
} catch {
|
||||||
log.error("Refresh failed: \(error.localizedDescription)")
|
log.error("Refresh failed: \(error.localizedDescription)")
|
||||||
refreshError = error.localizedDescription
|
refreshError = error.localizedDescription
|
||||||
@@ -179,12 +191,19 @@ final class BackupStatusService: NSObject, ObservableObject {
|
|||||||
// ── Step 2: NAS connection ────────────────────────────────────────
|
// ── Step 2: NAS connection ────────────────────────────────────────
|
||||||
let transfer: any NASTransferProtocol = conn.nasProtocol == .smb ? SMBService() : SFTPService()
|
let transfer: any NASTransferProtocol = conn.nasProtocol == .smb ? SMBService() : SFTPService()
|
||||||
do {
|
do {
|
||||||
try await transfer.connect(
|
try await Self.withTimeout(15) {
|
||||||
to: conn.host, port: conn.port,
|
try await transfer.connect(
|
||||||
username: conn.username, password: conn.password
|
to: conn.host, port: conn.port,
|
||||||
)
|
username: conn.username, password: conn.password
|
||||||
|
)
|
||||||
|
}
|
||||||
} catch {
|
} catch {
|
||||||
let reason = (error as? BackupError).flatMap { $0.errorDescription } ?? error.localizedDescription
|
let reason: String
|
||||||
|
if case BackupError.timeout = error {
|
||||||
|
reason = "NAS not responding — using last known state"
|
||||||
|
} else {
|
||||||
|
reason = (error as? BackupError).flatMap { $0.errorDescription } ?? error.localizedDescription
|
||||||
|
}
|
||||||
log.warning("NAS connect failed: \(reason, privacy: .public) — retaining cached counts")
|
log.warning("NAS connect failed: \(reason, privacy: .public) — retaining cached counts")
|
||||||
refreshError = reason
|
refreshError = reason
|
||||||
|
|
||||||
@@ -262,7 +281,7 @@ final class BackupStatusService: NSObject, ObservableObject {
|
|||||||
) async -> ManifestIndex {
|
) async -> ManifestIndex {
|
||||||
log.info("buildManifestIndex: remotePath=\(conn.remotePath, privacy: .public) manifestFile=\(path, privacy: .public)")
|
log.info("buildManifestIndex: remotePath=\(conn.remotePath, privacy: .public) manifestFile=\(path, privacy: .public)")
|
||||||
do {
|
do {
|
||||||
let data = try await transfer.downloadData(at: path)
|
let data = try await Self.withTimeout(12) { try await transfer.downloadData(at: path) }
|
||||||
log.info("buildManifestIndex: manifest downloaded — \(data.count) bytes")
|
log.info("buildManifestIndex: manifest downloaded — \(data.count) bytes")
|
||||||
// Decode and build index off main actor — JSONDecoder is not cheap for large manifests.
|
// Decode and build index off main actor — JSONDecoder is not cheap for large manifests.
|
||||||
let decoded: (ManifestIndex, BackupManifest)? = await Task.detached(priority: .utility) {
|
let decoded: (ManifestIndex, BackupManifest)? = await Task.detached(priority: .utility) {
|
||||||
@@ -279,7 +298,7 @@ final class BackupStatusService: NSObject, ObservableObject {
|
|||||||
// created. Scan the directory to get the real file count for the NAS Archive
|
// created. Scan the directory to get the real file count for the NAS Archive
|
||||||
// display — but do NOT add directory files as orphan manifest entries.
|
// display — but do NOT add directory files as orphan manifest entries.
|
||||||
if index.totalCount <= 1 {
|
if index.totalCount <= 1 {
|
||||||
let items = (try? await transfer.listDirectory(at: conn.remotePath)) ?? []
|
let items = (try? await Self.withTimeout(12) { try await transfer.listDirectory(at: conn.remotePath) }) ?? []
|
||||||
let dirCount = items.filter {
|
let dirCount = items.filter {
|
||||||
!$0.isDirectory && !$0.name.hasPrefix(".") && $0.name != BackupManifest.remoteFilename
|
!$0.isDirectory && !$0.name.hasPrefix(".") && $0.name != BackupManifest.remoteFilename
|
||||||
}.count
|
}.count
|
||||||
@@ -297,7 +316,7 @@ final class BackupStatusService: NSObject, ObservableObject {
|
|||||||
}
|
}
|
||||||
|
|
||||||
// Bootstrap: list NAS directory and build filename-only index in background.
|
// Bootstrap: list NAS directory and build filename-only index in background.
|
||||||
let items = (try? await transfer.listDirectory(at: conn.remotePath)) ?? []
|
let items = (try? await Self.withTimeout(12) { try await transfer.listDirectory(at: conn.remotePath) }) ?? []
|
||||||
log.info("buildManifestIndex: bootstrap from \(items.count) items in \(conn.remotePath, privacy: .public)")
|
log.info("buildManifestIndex: bootstrap from \(items.count) items in \(conn.remotePath, privacy: .public)")
|
||||||
let bootstrapIndex = await Task.detached(priority: .utility) {
|
let bootstrapIndex = await Task.detached(priority: .utility) {
|
||||||
ManifestIndex(nasListing: items)
|
ManifestIndex(nasListing: items)
|
||||||
@@ -424,6 +443,27 @@ final class BackupStatusService: NSObject, ObservableObject {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// MARK: — Timeout helper
|
||||||
|
|
||||||
|
/// Runs `work` and throws `BackupError.timeout` if it doesn't finish within `seconds`.
|
||||||
|
/// Uses a racing task group so the timeout fires even when the underlying SMB/SFTP
|
||||||
|
/// stack doesn't propagate Swift cooperative cancellation.
|
||||||
|
private nonisolated static func withTimeout<T: Sendable>(
|
||||||
|
_ seconds: TimeInterval,
|
||||||
|
work: @Sendable @escaping () async throws -> T
|
||||||
|
) async throws -> T {
|
||||||
|
try await withThrowingTaskGroup(of: T.self) { group in
|
||||||
|
group.addTask(priority: .userInitiated) { try await work() }
|
||||||
|
group.addTask(priority: .utility) {
|
||||||
|
try await Task.sleep(nanoseconds: UInt64(seconds * 1_000_000_000))
|
||||||
|
throw BackupError.timeout
|
||||||
|
}
|
||||||
|
defer { group.cancelAll() }
|
||||||
|
guard let result = try await group.next() else { throw BackupError.timeout }
|
||||||
|
return result
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
// MARK: — Cache
|
// MARK: — Cache
|
||||||
|
|
||||||
private func loadCachedSnapshot() {
|
private func loadCachedSnapshot() {
|
||||||
|
|||||||
@@ -14,7 +14,10 @@ actor NASManifestCache {
|
|||||||
|
|
||||||
private var cached: CachedEntry?
|
private var cached: CachedEntry?
|
||||||
private let diskURL: URL
|
private let diskURL: URL
|
||||||
private static let staleInterval: TimeInterval = 5 * 60
|
// 30-minute window: directoryCount is updated incrementally after every upload,
|
||||||
|
// and PHPhotoLibraryChangeObserver keeps LocalPhotoIndex current. A full NAS
|
||||||
|
// roundtrip every 5 minutes was unnecessary and caused "Checking…" stalls.
|
||||||
|
private static let staleInterval: TimeInterval = 30 * 60
|
||||||
|
|
||||||
private struct CachedEntry: Codable {
|
private struct CachedEntry: Codable {
|
||||||
var manifest: BackupManifest
|
var manifest: BackupManifest
|
||||||
|
|||||||
Reference in New Issue
Block a user