Skip to content

fix(sync): propagate settings decode errors instead of silently skipping#1143

Merged
datlechin merged 1 commit into
mainfrom
fix/sync-settings-decode-strict
May 9, 2026
Merged

fix(sync): propagate settings decode errors instead of silently skipping#1143
datlechin merged 1 commit into
mainfrom
fix/sync-settings-decode-strict

Conversation

@datlechin

Copy link
Copy Markdown
Member

Summary

  • SyncCoordinator.applySettingsData(_:for:) is now throws and lets decode failures propagate as SyncDecodeError.decodeFailure(field:underlying:). The eight category branches (general, appearance, editor, data grid, history, tabs, keyboard, AI) drop the per-branch if let settings = try? decoder.decode(...) { ... } pattern in favor of a single try inside a shared do/catch.
  • applyRemoteSettings(_:) wraps the call in do/catch and logs the failing record name + category + error, then skips the record.

Removes 32 lines of if let try? boilerplate while making the failure path observable.

Why this matters

This is bug B3 from the full-app audit. Same anti-pattern as #1141 (B2), one layer up. A settings record written by a newer schema version would deserialize to nil, the if body would be skipped, and the user would think their settings synced when they hadn't: no log, no error, no UI signal. After this PR the failure is logged with the failing category name, and the record stays unapplied so a future build can pick it up.

Test plan

  • Manual: write a CloudKit settings record with a malformed settingsJson blob for one of the eight categories, run a pull, confirm the log line Skipping remote settings ... (general): Sync record decode failed for general: ... and that local settings are unchanged.
  • Manual: confirm a normal pull with valid settings still applies them and advances state.
  • swiftlint --strict clean.

@chatgpt-codex-connector

Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.

@datlechin datlechin merged commit a9772cb into main May 9, 2026
2 checks passed
@datlechin datlechin deleted the fix/sync-settings-decode-strict branch May 9, 2026 15:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant