Skip to content

Archive a settings file that fails to deserialize instead of deleting it [patch] - #337

Merged
matt-edmondson merged 1 commit into
mainfrom
fix/archive-unreadable-settings-314
Oct 5, 2026
Merged

matt-edmondson merged 1 commit into
mainfrom
fix/archive-unreadable-settings-314

Conversation

@matt-edmondson

Copy link
Copy Markdown
Contributor

Fixes #314

What was wrong

When AppData<T>.LoadOrCreate hit a JsonException, it deleted the main file and retried. A successful save removes the .bk file, so in practice that file was the only copy of the user's data. Two ordinary events triggered this, and nothing was logged or left on disk afterwards:

  • a trailing comma left by a hand edit
  • a model change in an app update, such as a renamed enum member

Change

  • The counter loop from TryRestoreFrom is now AppData.Archive(filePath, label), which returns the path it archived to. TryRestoreFrom uses it with no label, so its behaviour is unchanged.
  • LoadOrCreate archives the unreadable file to <file>.corrupt.<yyyyMMdd_HHmmss> instead of deleting it. The retry still finds the main file missing and falls back to the temp file, then the backup, then defaults.
  • The misleading "delete and try load a backup" comment is corrected, and so is the TryRestoreFrom comment that described the old delete.
  • This PR does not expose the archived path as an event or property, which the issue listed as optional. The triage comment suggested that come in a separate change.

Tests

  • New test TestLoadOrCreateArchivesAnUnreadableFileInsteadOfDeletingIt follows the issue's repro: save real data, break the JSON, then LoadOrCreate. It asserts the defaults load and exactly one .corrupt.* archive holds the original content. With the delete restored, it fails at Assert.AreEqual(1, archived.Length).
  • Full suite: 109/109 passed locally, including TestLoadOrCreateHandlesCorruptFile and the recovery tests.

🤖 Generated with Claude Code

https://claude.ai/code/session_01DCzUAHg8icJNNp7WtR51Um


Generated by Claude Code

… it [patch]

LoadOrCreate deleted the main file on a JsonException and retried, but a
successful save removes the backup, so that file was usually the only copy
of the user's data and a hand edit or a model change in an app update
silently wiped it. Move it aside to <file>.corrupt.<timestamp> with the same
counter loop TryRestoreFrom uses, now shared as AppData.Archive.

Fixes #314

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DCzUAHg8icJNNp7WtR51Um
@sonarqubecloud

sonarqubecloud Bot commented Oct 5, 2026

Copy link
Copy Markdown

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.

A settings file that fails to deserialize is deleted outright and replaced with defaults, with no backup left to recover from

2 participants