crossmate

A collaborative crossword app for iOS
Log | Files | Refs | LICENSE

commit adef3fa3892c2cf2567ee8b79b181e13033b4ab6
parent 1f1777c8b879b7c0131a35e7347e831aa60f1e34
Author: Michael Camilleri <[email protected]>
Date:   Sun, 24 May 2026 06:06:27 +0900

Improve sophistication of try handling

The previous commit folded every `try? ctx.save()` into a single 'log to
the diagnostics log and continue' shape. That's right for cache writes
and background reconcile paths — CKSyncEngine will redeliver, or the
cache will get rebuilt — but it's wrong for two other categories the
audit had set aside.

- User-initiated mutations: A silent save here leaves in-memory state
  diverged from persisted state, so the user sees their action 'undone'
  after relaunch — or, worse, an invite/friend row that won't go away.
  These sites now throw instead of swallowing, and the call sites catch
  and surface via AnnouncementCenter.

- Sync-critical parsing: Three sites where a parse or fetch failure
  during record application silently drops an inbound sync update.
  CKSyncEngine advances its change token whenever the delegate returns
  from fetchedRecordZoneChanges, so re-throwing wouldn't trigger
  redelivery — making the drop visible is the only available
  remediation. SyncEngine.start now decodes the persisted
  CKSyncEngine.State.Serialization through a decodeEngineState helper
  that traces the cause before letting the engine cold-start.
  RecordSerializer.applyGameRecord gives the puzzleSource CKAsset read
  the same treatment, so a corrupted asset no longer leaves a GameEntity
  without playable content without any record of why.

Co-Authored-By: Claude Opus 4.7 <[email protected]>

Diffstat:
MCrossmate/CrossmateApp.swift | 32++++++++++++++++++++++++++++----
MCrossmate/Persistence/GameStore.swift | 18+++++++++---------
MCrossmate/Services/AppServices.swift | 61++++++++++++++++++++++++++++++++++++++++++-------------------
MCrossmate/Services/CloudService.swift | 4++--
MCrossmate/Sync/FriendController.swift | 10+++-------
MCrossmate/Sync/RecordApplier.swift | 56++++++++++++++++++++++++++++++++++++++++++++++++++++----
MCrossmate/Sync/RecordSerializer.swift | 25+++++++++++++++++++------
MCrossmate/Sync/SyncEngine.swift | 32+++++++++++++++++++++-----------
MCrossmate/Views/GameListView.swift | 44++++++++++++++++++++++++++++++++++++++------
9 files changed, 214 insertions(+), 68 deletions(-)

diff --git a/Crossmate/CrossmateApp.swift b/Crossmate/CrossmateApp.swift @@ -27,7 +27,20 @@ struct CrossmateApp: App { .environment(\.syncEngine, services.syncEngine) .environment(services.nytAuth) .environment(\.nytPuzzleFetcher, services.nytFetcher) - .environment(\.resetDatabase, { await services.cloudService.resetAllData() }) + .environment(\.resetDatabase, { + do { + try await services.cloudService.resetAllData() + } catch { + services.announcements.post(Announcement( + id: "reset-database-error", + scope: .global, + severity: .error, + title: "Couldn't Reset Database", + body: error.localizedDescription, + dismissal: .manual + )) + } + }) .environment(\.inviteFriend, { gameID, friendAuthorID in try await services.inviteFriend(gameID: gameID, friendAuthorID: friendAuthorID) }) @@ -397,9 +410,20 @@ private struct PuzzleDisplayView: View { shareController: shareController, roster: roster, onComplete: { - store.markCompleted(id: gameID) - Task { - await services.sendCompletionPings(gameID: gameID, resigned: false) + do { + try store.markCompleted(id: gameID) + Task { + await services.sendCompletionPings(gameID: gameID, resigned: false) + } + } catch { + services.announcements.post(Announcement( + id: "mark-completed-error-\(gameID.uuidString)", + scope: .game(gameID), + severity: .error, + title: "Couldn't Save Completion", + body: error.localizedDescription, + dismissal: .manual + )) } }, onResign: { diff --git a/Crossmate/Persistence/GameStore.swift b/Crossmate/Persistence/GameStore.swift @@ -529,18 +529,18 @@ final class GameStore { /// Marks a game as completed after a normal win. No-ops if already marked. /// Triggers a buffer flush so the completion snapshot is created promptly /// rather than waiting for the next keystroke or app-background event. - func markCompleted(id: UUID) { + func markCompleted(id: UUID) throws { let request = NSFetchRequest<GameEntity>(entityName: "GameEntity") request.predicate = NSPredicate(format: "id == %@", id as CVarArg) request.fetchLimit = 1 - guard let entity = try? context.fetch(request).first, + guard let entity = try context.fetch(request).first, entity.completedAt == nil else { return } entity.completedAt = Date() // A win: stamp the solver so the Game record can distinguish wins from // resignations and drive the directed `.win` ping's body. entity.completedBy = authorIDProvider() entity.hasPendingSave = true - saveContext("markCompleted") + try context.save() if let ckName = entity.ckRecordName { onGameUpdated(ckName) } @@ -551,23 +551,23 @@ final class GameStore { /// Deletes every game (and its cascaded moves, snapshots, and cells) plus /// the sync-state row. Used by the diagnostics reset button. - func resetAllData() { - for entity in (try? context.fetch(NSFetchRequest<GameEntity>(entityName: "GameEntity"))) ?? [] { + func resetAllData() throws { + for entity in try context.fetch(NSFetchRequest<GameEntity>(entityName: "GameEntity")) { context.delete(entity) } - for entity in (try? context.fetch(NSFetchRequest<SyncStateEntity>(entityName: "SyncStateEntity"))) ?? [] { + for entity in try context.fetch(NSFetchRequest<SyncStateEntity>(entityName: "SyncStateEntity")) { context.delete(entity) } // Friend zones themselves are removed by CloudService.resetAllData's // wholesale private-zone delete / shared-zone leave; clear the local // friendship + invite rows so a reset is a clean slate. - for entity in (try? context.fetch(NSFetchRequest<FriendEntity>(entityName: "FriendEntity"))) ?? [] { + for entity in try context.fetch(NSFetchRequest<FriendEntity>(entityName: "FriendEntity")) { context.delete(entity) } - for entity in (try? context.fetch(NSFetchRequest<InviteEntity>(entityName: "InviteEntity"))) ?? [] { + for entity in try context.fetch(NSFetchRequest<InviteEntity>(entityName: "InviteEntity")) { context.delete(entity) } - saveContext("resetLocalState") + try context.save() currentGame = nil currentMutator = nil currentEntity = nil diff --git a/Crossmate/Services/AppServices.swift b/Crossmate/Services/AppServices.swift @@ -343,7 +343,18 @@ final class AppServices { // so a freshly-synced game and its stale invite don't show side // by side. `applyInvitePings` GCs the same row, but only when a // ping is next fetched. - self.removePendingInvite(forGameID: gameID) + do { + try self.removePendingInvite(forGameID: gameID) + } catch { + self.announcements.post(Announcement( + id: "remove-pending-invite-error-\(gameID.uuidString)", + scope: .global, + severity: .error, + title: "Couldn't Clear Invite", + body: error.localizedDescription, + dismissal: .manual + )) + } } // A sibling device consumed (deleted) a directed ping; withdraw any @@ -1184,21 +1195,17 @@ final class AppServices { /// garbage-collection over every pending invite, but only when a ping is /// fetched — hooking zone arrival closes the window where a just-synced /// game and its stale invite show side by side in the library. - func removePendingInvite(forGameID gameID: UUID) { + func removePendingInvite(forGameID gameID: UUID) throws { let ctx = persistence.viewContext let req = NSFetchRequest<InviteEntity>(entityName: "InviteEntity") req.predicate = NSPredicate( format: "gameID == %@ AND status == %@", gameID as CVarArg, "pending" ) - let invites = (try? ctx.fetch(req)) ?? [] + let invites = try ctx.fetch(req) guard !invites.isEmpty else { return } for invite in invites { ctx.delete(invite) } if ctx.hasChanges { - do { - try ctx.save() - } catch { - eventLog.note("AppServices: removePendingInvite save failed — \(error)", level: "error") - } + try ctx.save() } } @@ -1214,11 +1221,19 @@ final class AppServices { do { try await cloudService.acceptShare(url: url) } catch let error as CKError where error.code == .unknownItem { - await deleteInviteAndPing(pingRecordName: pingRecordName) - syncMonitor.note("accept invite: removed stale invite \(pingRecordName)") + // Stale share: the row needs to go away too, but the next + // `applyInvitePings` will GC it if this cleanup itself fails. + // The user-visible signal here is `.unavailable`, so don't let a + // cleanup error clobber it — log and continue. + do { + try await deleteInviteAndPing(pingRecordName: pingRecordName) + syncMonitor.note("accept invite: removed stale invite \(pingRecordName)") + } catch { + syncMonitor.note("accept invite: stale-invite cleanup failed for \(pingRecordName) — \(error)") + } throw InviteAcceptanceError.unavailable } - await deleteInviteAndPing(pingRecordName: pingRecordName) + try await deleteInviteAndPing(pingRecordName: pingRecordName) } /// Accepts the pending invite for `gameID`, if one is still recorded @@ -1245,11 +1260,11 @@ final class AppServices { try await acceptInvite(shareURL: shareURL, pingRecordName: pingRecordName) } - private func deleteInviteAndPing(pingRecordName: String) async { + private func deleteInviteAndPing(pingRecordName: String) async throws { let ctx = persistence.viewContext let req = NSFetchRequest<InviteEntity>(entityName: "InviteEntity") req.predicate = NSPredicate(format: "pingRecordName == %@", pingRecordName) - for invite in (try? ctx.fetch(req)) ?? [] { + for invite in try ctx.fetch(req) { if let inviterAuthorID = invite.inviterAuthorID { await friendController.deleteInvitePing( fromFriendAuthorID: inviterAuthorID, @@ -1259,11 +1274,7 @@ final class AppServices { ctx.delete(invite) } if ctx.hasChanges { - do { - try ctx.save() - } catch { - eventLog.note("AppServices: deleteInviteAndPing save failed — \(error)", level: "error") - } + try ctx.save() } } @@ -1272,7 +1283,19 @@ final class AppServices { /// their pending invites. Games we *own* that they joined are untouched. /// Surfaced via `\.blockFriend`. func blockFriend(authorID: String) async { - await friendController.blockAndTeardown(friendAuthorID: authorID) + do { + try await friendController.blockAndTeardown(friendAuthorID: authorID) + } catch { + announcements.post(Announcement( + id: "block-friend-error-\(authorID)", + scope: .global, + severity: .error, + title: "Couldn't Block Collaborator", + body: error.localizedDescription, + dismissal: .manual + )) + return + } let ctx = persistence.container.newBackgroundContext() let gameIDsToLeave: [UUID] = ctx.performAndWait { diff --git a/Crossmate/Services/CloudService.swift b/Crossmate/Services/CloudService.swift @@ -96,14 +96,14 @@ final class CloudService { } } - func resetAllData() async { + func resetAllData() async throws { await syncEngine.resetSyncState() async let privateCleanup: Void = deleteAllPrivateZones() async let sharedCleanup: Void = leaveAllSharedZones() _ = await (privateCleanup, sharedCleanup) - store.resetAllData() + try store.resetAllData() UserDefaults.standard.removeObject(forKey: "gamePlayerColors") syncMonitor.note("Database reset — all games and sync state cleared") } diff --git a/Crossmate/Sync/FriendController.swift b/Crossmate/Sync/FriendController.swift @@ -257,21 +257,17 @@ final class FriendController { /// participant leaves by deleting the zone-wide share. The `FriendEntity` /// row is kept (as a blocked tombstone) so future `.invite` Pings are /// suppressed and the zone stays out of `knownZones`. - func blockAndTeardown(friendAuthorID: String) async { + func blockAndTeardown(friendAuthorID: String) async throws { let ctx = persistence.viewContext let req = NSFetchRequest<FriendEntity>(entityName: "FriendEntity") req.predicate = NSPredicate(format: "authorID == %@", friendAuthorID) req.fetchLimit = 1 - guard let friend = try? ctx.fetch(req).first else { return } + guard let friend = try ctx.fetch(req).first else { return } let scope = friend.databaseScope let zoneName = friend.friendZoneName let ownerName = friend.friendZoneOwnerName friend.isBlocked = true - do { - try ctx.save() - } catch { - eventLog?.note("FriendController: blockAndTeardown save failed — \(error)", level: "error") - } + try ctx.save() // Make the block authoritative across the user's own devices. Written // before the channel teardown so the durable fact survives even a diff --git a/Crossmate/Sync/RecordApplier.swift b/Crossmate/Sync/RecordApplier.swift @@ -228,11 +228,28 @@ extension SyncEngine { let gameReq = NSFetchRequest<GameEntity>(entityName: "GameEntity") gameReq.predicate = NSPredicate(format: "id == %@", gameID as CVarArg) gameReq.fetchLimit = 1 - guard let game = try? ctx.fetch(gameReq).first else { return } + let game: GameEntity? + do { + game = try ctx.fetch(gameReq).first + } catch { + // CKSyncEngine commits the batch when the delegate returns + // (see fetchedRecordZoneChanges save-failure note), so re-throwing + // won't redeliver — surface the failure to console instead of + // silently leaving the cell cache stale for this game. + Self.logSyncError("replayCellCache game fetch", gameID: gameID, error: error) + return + } + guard let game else { return } let movesReq = NSFetchRequest<MovesEntity>(entityName: "MovesEntity") movesReq.predicate = NSPredicate(format: "game == %@", game) - let movesEntities = (try? ctx.fetch(movesReq)) ?? [] + let movesEntities: [MovesEntity] + do { + movesEntities = try ctx.fetch(movesReq) + } catch { + Self.logSyncError("replayCellCache moves fetch", gameID: gameID, error: error) + return + } let values: [MovesValue] = movesEntities.compactMap { Self.movesValue(from: $0) } let gridState = GridStateMerger.merge(values) @@ -298,19 +315,50 @@ extension SyncEngine { let gameReq = NSFetchRequest<GameEntity>(entityName: "GameEntity") gameReq.predicate = NSPredicate(format: "id == %@", gameID as CVarArg) gameReq.fetchLimit = 1 - guard let game = try? ctx.fetch(gameReq).first else { + let game: GameEntity? + do { + game = try ctx.fetch(gameReq).first + } catch { + // Batch is committed at the engine layer; can't redeliver, so + // surface the dropped snapshot to console. Empty snapshot + // means authorDeltas will see no transitions for this game. + logSyncError("gridSnapshots game fetch", gameID: gameID, error: error) + result[gameID] = [:] + continue + } + guard let game else { result[gameID] = [:] continue } let movesReq = NSFetchRequest<MovesEntity>(entityName: "MovesEntity") movesReq.predicate = NSPredicate(format: "game == %@", game) - let movesEntities = (try? ctx.fetch(movesReq)) ?? [] + let movesEntities: [MovesEntity] + do { + movesEntities = try ctx.fetch(movesReq) + } catch { + logSyncError("gridSnapshots moves fetch", gameID: gameID, error: error) + result[gameID] = [:] + continue + } let values: [MovesValue] = movesEntities.compactMap { movesValue(from: $0) } result[gameID] = GridStateMerger.mergeWithProvenance(values) } return result } + /// Sync-context error surfacing — mirrors the `print(...)` format used by + /// the surrounding save-failure handlers. The engine's change token has + /// already advanced by the time these helpers run inside performAndWait, + /// so the only available remediation is making the drop visible. + nonisolated static func logSyncError(_ label: String, gameID: UUID, error: Error) { + let nsError = error as NSError + print( + "SyncEngine: \(label) failed for \(gameID.uuidString) " + + "— domain=\(nsError.domain) code=\(nsError.code) " + + "\(nsError.localizedDescription)" + ) + } + /// Aggregates the merged-grid diff per `(gameID, writerAuthorID)`. Counts /// empty→letter transitions as `added` and letter→empty as `cleared`; /// letter→different-letter (one author rewriting over another's cell) diff --git a/Crossmate/Sync/RecordSerializer.swift b/Crossmate/Sync/RecordSerializer.swift @@ -484,12 +484,25 @@ enum RecordSerializer { } if let asset = record["puzzleSource"] as? CKAsset, - let fileURL = asset.fileURL, - let source = try? String(contentsOf: fileURL, encoding: .utf8) { - entity.puzzleSource = source - if let xd = try? XD.parse(source) { - entity.puzzleCmVersion = Int64(XD.currentCmVersion) - entity.populateCachedSummaryFields(from: Puzzle(xd: xd)) + let fileURL = asset.fileURL { + do { + let source = try String(contentsOf: fileURL, encoding: .utf8) + entity.puzzleSource = source + if let xd = try? XD.parse(source) { + entity.puzzleCmVersion = Int64(XD.currentCmVersion) + entity.populateCachedSummaryFields(from: Puzzle(xd: xd)) + } + } catch { + // CKSyncEngine has already committed this batch by the time + // the delegate returns, so re-throwing wouldn't redeliver. + // Surface the dropped puzzle source instead of silently + // leaving the entity without playable content. + let nsError = error as NSError + print( + "RecordSerializer: puzzleSource asset read failed for \(recordName) " + + "— domain=\(nsError.domain) code=\(nsError.code) " + + "\(nsError.localizedDescription)" + ) } } diff --git a/Crossmate/Sync/SyncEngine.swift b/Crossmate/Sync/SyncEngine.swift @@ -187,29 +187,25 @@ actor SyncEngine { /// wiring callbacks. Idempotent — extra calls are no-ops so a race between /// `services.start()` and a scene-active foreground sync can't double- /// initialise the engines or re-fire subscription setup. - func start() { + func start() async { guard privateEngine == nil, sharedEngine == nil else { return } let bgCtx = persistence.container.newBackgroundContext() - let privateState: CKSyncEngine.State.Serialization? = bgCtx.performAndWait { - guard let data = SyncStateEntity.current(in: bgCtx).ckPrivateEngineState else { - return nil - } - return try? JSONDecoder().decode(CKSyncEngine.State.Serialization.self, from: data) + let privateData: Data? = bgCtx.performAndWait { + SyncStateEntity.current(in: bgCtx).ckPrivateEngineState } + let privateState = await decodeEngineState(privateData, label: "private") privateEngine = CKSyncEngine(CKSyncEngine.Configuration( database: container.privateCloudDatabase, stateSerialization: privateState, delegate: self )) - let sharedState: CKSyncEngine.State.Serialization? = bgCtx.performAndWait { - guard let data = SyncStateEntity.current(in: bgCtx).ckSharedEngineState else { - return nil - } - return try? JSONDecoder().decode(CKSyncEngine.State.Serialization.self, from: data) + let sharedData: Data? = bgCtx.performAndWait { + SyncStateEntity.current(in: bgCtx).ckSharedEngineState } + let sharedState = await decodeEngineState(sharedData, label: "shared") sharedEngine = CKSyncEngine(CKSyncEngine.Configuration( database: container.sharedCloudDatabase, stateSerialization: sharedState, @@ -742,6 +738,20 @@ actor SyncEngine { await tracer(message) } + /// Decodes a persisted `CKSyncEngine.State.Serialization` payload. + /// On failure, traces the cold-start cause so it isn't silent in the + /// diagnostics log — the engine will rebuild change tokens and resend + /// pending changes from scratch, which is visible-to-the-user behavior. + private func decodeEngineState(_ data: Data?, label: String) async -> CKSyncEngine.State.Serialization? { + guard let data else { return nil } + do { + return try JSONDecoder().decode(CKSyncEngine.State.Serialization.self, from: data) + } catch { + await trace("\(label) engine state decode failed (\(data.count) bytes) — cold-starting sync: \(describe(error))") + return nil + } + } + private func saveEngineState( _ serialization: CKSyncEngine.State.Serialization, isPrivate: Bool diff --git a/Crossmate/Views/GameListView.swift b/Crossmate/Views/GameListView.swift @@ -10,7 +10,6 @@ struct GameListView: View { @Binding var navigationPath: NavigationPath @Environment(\.managedObjectContext) private var viewContext - @Environment(EventLog.self) private var eventLog @Environment(\.dynamicTypeSize) private var dynamicTypeSize @Environment(\.horizontalSizeClass) private var horizontalSizeClass @FetchRequest( @@ -101,9 +100,20 @@ struct GameListView: View { )) { Button("Resign", role: .destructive) { if let target = resignTarget { - try? store.resignGame(id: target.id) - let id = target.id - Task { await sendResignPings?(id) } + do { + try store.resignGame(id: target.id) + let id = target.id + Task { await sendResignPings?(id) } + } catch { + announcements.post(Announcement( + id: Self.destructiveActionErrorID, + scope: .global, + severity: .error, + title: "Couldn't Resign Puzzle", + body: error.localizedDescription, + dismissal: .manual + )) + } } } Button("Cancel", role: .cancel) {} @@ -133,7 +143,18 @@ struct GameListView: View { )) { Button("Delete", role: .destructive) { if let target = deleteTarget { - try? store.deleteGame(id: target.id) + do { + try store.deleteGame(id: target.id) + } catch { + announcements.post(Announcement( + id: Self.destructiveActionErrorID, + scope: .global, + severity: .error, + title: "Couldn't Delete Puzzle", + body: error.localizedDescription, + dismissal: .manual + )) + } } } Button("Cancel", role: .cancel) {} @@ -485,12 +506,23 @@ struct GameListView: View { /// failure replaces the prior one rather than stacking. private static let inviteErrorID = "invite-accept-error" + /// Single-slot id for game-list destructive-action failures (decline, + /// resign, delete) — a fresh failure replaces the prior one. + private static let destructiveActionErrorID = "game-list-destructive-action-error" + private func decline(_ invite: InviteEntity) { invite.status = "declined" do { try viewContext.save() } catch { - eventLog.note("GameListView: decline invite save failed — \(error)", level: "error") + announcements.post(Announcement( + id: Self.destructiveActionErrorID, + scope: .global, + severity: .error, + title: "Couldn't Decline Invite", + body: error.localizedDescription, + dismissal: .manual + )) } }