mirror of
https://github.com/awizemann/scarf.git
synced 2026-05-10 10:36:35 +00:00
fix(ios-rootmodel): surface store failures (A.3 + A.4 bundled)
Bundled because the fixes are coherent — they all add the same mechanism (`lastError` + `os.Logger`) to the same model. A.3 — Distinguish "no servers" from "Keychain unreachable": - `RootModel.connect(to:)` previously used `try?` on `keyStore.load(for:)`. A biometric cancel or device-locked Keychain read returned nil → the app dropped the user into fresh onboarding, destroying the existing server's host/user/port. Now we catch the throw, log via os.Logger, set `lastError`, and stay on `.serverList`. The user sees a banner + Dismiss button instead of being kicked back to onboarding. - `RootModel.load()` now logs the corrupted-blob path via os.Logger and sets `lastError` before falling through to onboarding (recovery is necessary, but the user gets context now). A.4 — Surface delete failures in `forget()` and `disconnect()`: - Both used `try?` on every store delete. On partial failure the in-memory dict was wiped while orphan Keychain entries lingered. Now each delete is `do/catch` with logging, failures collected into `lastError`. The in-memory state is reloaded from disk so it tracks what's actually persisted (covers the partial-failure case). ServerListView gains an inline error banner above the list that reads `model.lastError`, with a Dismiss button calling `clearLastError()`. Verified: iOS build succeeds. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -1,6 +1,7 @@
|
|||||||
import SwiftUI
|
import SwiftUI
|
||||||
import ScarfCore
|
import ScarfCore
|
||||||
import ScarfIOS
|
import ScarfIOS
|
||||||
|
import os
|
||||||
|
|
||||||
/// App entry point. Renders a single `WindowGroup` whose root decides
|
/// App entry point. Renders a single `WindowGroup` whose root decides
|
||||||
/// between onboarding and the connected-app surface based on whether
|
/// between onboarding and the connected-app surface based on whether
|
||||||
@@ -116,19 +117,37 @@ final class RootModel {
|
|||||||
/// having to re-query stores on every re-render.
|
/// having to re-query stores on every re-render.
|
||||||
private(set) var servers: [ServerID: IOSServerConfig] = [:]
|
private(set) var servers: [ServerID: IOSServerConfig] = [:]
|
||||||
|
|
||||||
|
/// Most recent non-fatal failure surfaced from RootModel operations
|
||||||
|
/// (load, connect, forget). The ServerListView renders a banner above
|
||||||
|
/// the list when this is non-nil with a Retry/Dismiss affordance.
|
||||||
|
/// `nil` after a successful op so stale errors don't linger.
|
||||||
|
var lastError: String?
|
||||||
|
|
||||||
private let keyStore: any SSHKeyStore
|
private let keyStore: any SSHKeyStore
|
||||||
private let configStore: any IOSServerConfigStore
|
private let configStore: any IOSServerConfigStore
|
||||||
|
|
||||||
|
private static let logger = Logger(
|
||||||
|
subsystem: "com.scarf.ios",
|
||||||
|
category: "RootModel"
|
||||||
|
)
|
||||||
|
|
||||||
init(keyStore: any SSHKeyStore, configStore: any IOSServerConfigStore) {
|
init(keyStore: any SSHKeyStore, configStore: any IOSServerConfigStore) {
|
||||||
self.keyStore = keyStore
|
self.keyStore = keyStore
|
||||||
self.configStore = configStore
|
self.configStore = configStore
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// Clear the surfaced error. Called by the ServerListView banner's
|
||||||
|
/// Dismiss button.
|
||||||
|
func clearLastError() {
|
||||||
|
lastError = nil
|
||||||
|
}
|
||||||
|
|
||||||
/// Load configured servers from disk and pick an initial state.
|
/// Load configured servers from disk and pick an initial state.
|
||||||
func load() async {
|
func load() async {
|
||||||
do {
|
do {
|
||||||
let all = try await configStore.listAll()
|
let all = try await configStore.listAll()
|
||||||
servers = all
|
servers = all
|
||||||
|
lastError = nil
|
||||||
if all.isEmpty {
|
if all.isEmpty {
|
||||||
// Fresh install or user forgot every server → go
|
// Fresh install or user forgot every server → go
|
||||||
// straight to onboarding with a new ID reserved so
|
// straight to onboarding with a new ID reserved so
|
||||||
@@ -138,7 +157,14 @@ final class RootModel {
|
|||||||
state = .serverList
|
state = .serverList
|
||||||
}
|
}
|
||||||
} catch {
|
} catch {
|
||||||
|
// configStore is UserDefaults-backed; failures here are
|
||||||
|
// exceptional (corrupted v2 blob, JSONDecoder error). Surface
|
||||||
|
// the error to the user but recover into onboarding so they
|
||||||
|
// aren't permanently locked out of the app — the state is
|
||||||
|
// unsalvageable, the user needs to re-onboard anyway.
|
||||||
|
Self.logger.error("RootModel.load failed: \(error.localizedDescription, privacy: .public)")
|
||||||
servers = [:]
|
servers = [:]
|
||||||
|
lastError = "Couldn't load saved servers (\(error.localizedDescription)). Starting fresh."
|
||||||
state = .onboarding(forNewServer: ServerID())
|
state = .onboarding(forNewServer: ServerID())
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
@@ -179,18 +205,33 @@ final class RootModel {
|
|||||||
/// Tap a server row → connect. Loads fresh from disk to catch any
|
/// Tap a server row → connect. Loads fresh from disk to catch any
|
||||||
/// edits made through the Mac app (or future multi-device scenarios).
|
/// edits made through the Mac app (or future multi-device scenarios).
|
||||||
func connect(to id: ServerID) async {
|
func connect(to id: ServerID) async {
|
||||||
|
do {
|
||||||
var diskConfig: IOSServerConfig? = servers[id]
|
var diskConfig: IOSServerConfig? = servers[id]
|
||||||
if diskConfig == nil {
|
if diskConfig == nil {
|
||||||
diskConfig = try? await configStore.load(id: id)
|
diskConfig = try await configStore.load(id: id)
|
||||||
}
|
}
|
||||||
let diskKey: SSHKeyBundle? = try? await keyStore.load(for: id)
|
let diskKey: SSHKeyBundle? = try await keyStore.load(for: id)
|
||||||
guard let config = diskConfig, let key = diskKey else {
|
guard let config = diskConfig, let key = diskKey else {
|
||||||
// Missing key → force re-onboarding under this ID so the
|
// Genuine "no row" / "no key" — preserve the pre-A.3
|
||||||
// user can regenerate without losing host/user/port.
|
// behaviour: re-onboard under this ID so the user keeps
|
||||||
|
// host/user/port and just regenerates the key.
|
||||||
state = .onboarding(forNewServer: id)
|
state = .onboarding(forNewServer: id)
|
||||||
return
|
return
|
||||||
}
|
}
|
||||||
|
lastError = nil
|
||||||
state = .connected(id, config, key)
|
state = .connected(id, config, key)
|
||||||
|
} catch {
|
||||||
|
// Transient Keychain errors (biometric cancel, device
|
||||||
|
// locked, OS-level Keychain corruption) used to drop the
|
||||||
|
// user into fresh onboarding — destroying useful state.
|
||||||
|
// Now we keep them on the server list with a banner so
|
||||||
|
// they can retry once the Keychain is reachable again.
|
||||||
|
Self.logger.error(
|
||||||
|
"RootModel.connect failed for \(id, privacy: .public): \(error.localizedDescription, privacy: .public)"
|
||||||
|
)
|
||||||
|
lastError = "Couldn't unlock server credentials: \(error.localizedDescription)"
|
||||||
|
state = .serverList
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
/// Soft disconnect: return to the server list without wiping
|
/// Soft disconnect: return to the server list without wiping
|
||||||
@@ -211,20 +252,60 @@ final class RootModel {
|
|||||||
|
|
||||||
/// Hard forget: wipe the specified server's key + config, refresh
|
/// Hard forget: wipe the specified server's key + config, refresh
|
||||||
/// the list, transition to serverList (or onboarding if empty).
|
/// the list, transition to serverList (or onboarding if empty).
|
||||||
|
/// Per-store failures are captured in `lastError` so a partial
|
||||||
|
/// forget surfaces a banner instead of silently leaving orphans.
|
||||||
func forget(id: ServerID) async {
|
func forget(id: ServerID) async {
|
||||||
try? await keyStore.delete(for: id)
|
var failures: [String] = []
|
||||||
try? await configStore.delete(id: id)
|
do {
|
||||||
|
try await keyStore.delete(for: id)
|
||||||
|
} catch {
|
||||||
|
Self.logger.error(
|
||||||
|
"RootModel.forget keyStore.delete failed for \(id, privacy: .public): \(error.localizedDescription, privacy: .public)"
|
||||||
|
)
|
||||||
|
failures.append("Keychain: \(error.localizedDescription)")
|
||||||
|
}
|
||||||
|
do {
|
||||||
|
try await configStore.delete(id: id)
|
||||||
|
} catch {
|
||||||
|
Self.logger.error(
|
||||||
|
"RootModel.forget configStore.delete failed for \(id, privacy: .public): \(error.localizedDescription, privacy: .public)"
|
||||||
|
)
|
||||||
|
failures.append("Config: \(error.localizedDescription)")
|
||||||
|
}
|
||||||
|
// Reload from disk so in-memory state reflects what's actually
|
||||||
|
// persisted — covers the partial-failure case where Keychain
|
||||||
|
// succeeded but config didn't (or vice versa).
|
||||||
servers = (try? await configStore.listAll()) ?? [:]
|
servers = (try? await configStore.listAll()) ?? [:]
|
||||||
|
if failures.isEmpty {
|
||||||
|
lastError = nil
|
||||||
|
} else {
|
||||||
|
lastError = "Couldn't fully forget server: " + failures.joined(separator: "; ")
|
||||||
|
}
|
||||||
state = servers.isEmpty ? .onboarding(forNewServer: ServerID()) : .serverList
|
state = servers.isEmpty ? .onboarding(forNewServer: ServerID()) : .serverList
|
||||||
}
|
}
|
||||||
|
|
||||||
/// Legacy v1 "Disconnect" that wipes EVERYTHING. Kept for back-compat
|
/// Legacy v1 "Disconnect" that wipes EVERYTHING. Kept for back-compat
|
||||||
/// with any caller that still hits the no-arg path (there shouldn't
|
/// with any caller that still hits the no-arg path (there shouldn't
|
||||||
/// be any after 3.5 lands, but the protocol still supports it).
|
/// be any after 3.5 lands, but the protocol still supports it).
|
||||||
|
/// Same partial-failure semantics as `forget(id:)`.
|
||||||
func disconnect() async {
|
func disconnect() async {
|
||||||
try? await keyStore.delete()
|
var failures: [String] = []
|
||||||
try? await configStore.delete()
|
do {
|
||||||
servers = [:]
|
try await keyStore.delete()
|
||||||
|
} catch {
|
||||||
|
Self.logger.error("RootModel.disconnect keyStore.delete failed: \(error.localizedDescription, privacy: .public)")
|
||||||
|
failures.append("Keychain: \(error.localizedDescription)")
|
||||||
|
}
|
||||||
|
do {
|
||||||
|
try await configStore.delete()
|
||||||
|
} catch {
|
||||||
|
Self.logger.error("RootModel.disconnect configStore.delete failed: \(error.localizedDescription, privacy: .public)")
|
||||||
|
failures.append("Config: \(error.localizedDescription)")
|
||||||
|
}
|
||||||
|
servers = (try? await configStore.listAll()) ?? [:]
|
||||||
|
if !failures.isEmpty {
|
||||||
|
lastError = "Couldn't fully sign out: " + failures.joined(separator: "; ")
|
||||||
|
}
|
||||||
state = .onboarding(forNewServer: ServerID())
|
state = .onboarding(forNewServer: ServerID())
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -19,6 +19,26 @@ struct ServerListView: View {
|
|||||||
var body: some View {
|
var body: some View {
|
||||||
NavigationStack {
|
NavigationStack {
|
||||||
List {
|
List {
|
||||||
|
if let err = model.lastError {
|
||||||
|
Section {
|
||||||
|
VStack(alignment: .leading, spacing: 8) {
|
||||||
|
Label("Something went wrong", systemImage: "exclamationmark.triangle.fill")
|
||||||
|
.foregroundStyle(.orange)
|
||||||
|
.font(.headline)
|
||||||
|
Text(err)
|
||||||
|
.font(.callout)
|
||||||
|
.foregroundStyle(.secondary)
|
||||||
|
.fixedSize(horizontal: false, vertical: true)
|
||||||
|
HStack(spacing: 12) {
|
||||||
|
Button("Dismiss") { model.clearLastError() }
|
||||||
|
.buttonStyle(.bordered)
|
||||||
|
.controlSize(.small)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
.padding(.vertical, 4)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
Section {
|
Section {
|
||||||
ForEach(sortedServers, id: \.id) { row in
|
ForEach(sortedServers, id: \.id) { row in
|
||||||
ServerListRow(row: row) {
|
ServerListRow(row: row) {
|
||||||
|
|||||||
Reference in New Issue
Block a user