From b73d7b5c89554ca7747db98bc2f18dc1b8168217 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 8 Oct 2026 12:29:04 +0000 Subject: [PATCH 1/2] Recover from a saved value a semantic type rejects, and retry Get() after a failed load [patch] A value that is well-formed JSON but fails a semantic type's validation surfaced as ArgumentException, which LoadOrCreate did not catch, so it escaped and the file was left to fail again on every launch. Route ArgumentException, FormatException and NotSupportedException from deserialization to the same archive-and-retry path as JsonException. InternalState was a Lazy in ExecutionAndPublication mode, which caches the exception, so one failed load made every Get() throw for the rest of the process. It is now PublicationOnly, so the next Get() tries again. Fixes #315 Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01J9ABDLXQgwyGEboxA8DpmQ --- AppDataStorage.Test/AppDataTests.cs | 51 +++++++++++++++++++++++++++++ AppDataStorage/AppData.cs | 13 ++++++-- 2 files changed, 61 insertions(+), 3 deletions(-) diff --git a/AppDataStorage.Test/AppDataTests.cs b/AppDataStorage.Test/AppDataTests.cs index 8e5b12a..ad9a95b 100644 --- a/AppDataStorage.Test/AppDataTests.cs +++ b/AppDataStorage.Test/AppDataTests.cs @@ -459,6 +459,57 @@ public void TestLoadOrCreateArchivesAnUnreadableFileInsteadOfDeletingIt() Assert.AreEqual(unreadable, AppData.FileSystem.File.ReadAllText(archived[0]), "The archive must hold the original content."); } + [TestMethod] + public void TestLoadOrCreateRecoversFromAValueASemanticTypeRejects() + { + // Well-formed JSON whose path the semantic type rejects: the converter throws + // ArgumentException rather than JsonException. + const string rejected = "{\"Data\":\"keep\",\"Path\":\"not/absolute\"}"; + using SemanticPathAppData probe = new(); + AbsoluteFilePath filePath = probe.FilePath; + AppData.EnsureDirectoryExists(filePath); + AppData.FileSystem.File.WriteAllText(filePath, rejected); + + SemanticPathAppData appData = SemanticPathAppData.LoadOrCreate(); + + Assert.AreEqual("d", appData.Data, "Data should be default if a value could not be read."); + Assert.IsNull(appData.Path, "Path should be default if a value could not be read."); + string[] archived = AppData.FileSystem.Directory.GetFiles(filePath.AbsoluteDirectoryPath.ToString(), $"{Path.GetFileName(filePath.ToString())}.corrupt.*"); + Assert.AreEqual(1, archived.Length, "The rejected file must be archived, not left to fail again."); + Assert.AreEqual(rejected, AppData.FileSystem.File.ReadAllText(archived[0]), "The archive must hold the original content."); + } + + [TestMethod] + public void TestGetRetriesAfterAFailedLoadInsteadOfRethrowingIt() + { + FlakyAppData.FailConstruction = true; + // new() wraps the constructor's exception in a TargetInvocationException. + Assert.Throws(FlakyAppData.Get); + + FlakyAppData.FailConstruction = false; + Assert.IsNotNull(FlakyAppData.Get(), "A failed load must not poison Get() for the rest of the process."); + } + + internal sealed class SemanticPathAppData : AppData + { + public string Data { get; set; } = "d"; + public AbsoluteDirectoryPath? Path { get; set; } + } + + [System.Diagnostics.CodeAnalysis.SuppressMessage("Performance", "CA1812:Avoid uninstantiated internal classes", Justification = "Instantiated by AppData.LoadOrCreate through the new() constraint.")] + internal sealed class FlakyAppData : AppData + { + internal static bool FailConstruction { get; set; } + + public FlakyAppData() + { + if (FailConstruction) + { + throw new InvalidOperationException("Simulated load failure."); + } + } + } + [TestMethod] public void TestLoadOrCreateHandlesNullJsonFile() { diff --git a/AppDataStorage/AppData.cs b/AppDataStorage/AppData.cs index e440925..d04c5a3 100644 --- a/AppDataStorage/AppData.cs +++ b/AppDataStorage/AppData.cs @@ -425,7 +425,12 @@ public static void ResetFileSystem() /// /// Gets the internal state of the app data. /// - internal static Lazy InternalState { get; } = new(LoadOrCreate); + /// + /// Publication-only, so a load that throws is retried by the next rather than + /// cached and rethrown for the rest of the process. takes the lock, + /// so racing first calls still load one at a time, and only one result is ever published. + /// + internal static Lazy InternalState { get; } = new(LoadOrCreate, LazyThreadSafetyMode.PublicationOnly); /// /// Gets or sets the last save time of the app data. @@ -592,10 +597,12 @@ public static T LoadOrCreate(RelativeDirectoryPath? subdirectory, FileName? file newAppData.FileNameOverride = fileName; return newAppData; } - catch (JsonException) + catch (Exception ex) when (ex is JsonException or ArgumentException or FormatException or NotSupportedException) { // The file could not be read as T, whether from corruption, a hand edit or a model - // change in an app update. It is usually the only copy of the user's data, since a + // change in an app update. Well-formed JSON can fail too: a converter passes on the + // exception a semantic type throws for a value it rejects, such as a Windows path + // read on Linux. It is usually the only copy of the user's data, since a // successful save removes the backup, so it is archived rather than deleted. The // retry then finds it missing and falls back to a temp or backup file, or defaults. _ = AppData.Archive(newAppData.FilePath, ".corrupt"); From 16adeb2c8e96183d247b9ee186b008a1df63e5f8 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 8 Oct 2026 12:40:44 +0000 Subject: [PATCH 2/2] Use Assert.HasCount for the archive count in the new test Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01J9ABDLXQgwyGEboxA8DpmQ --- AppDataStorage.Test/AppDataTests.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/AppDataStorage.Test/AppDataTests.cs b/AppDataStorage.Test/AppDataTests.cs index ad9a95b..0ca0a08 100644 --- a/AppDataStorage.Test/AppDataTests.cs +++ b/AppDataStorage.Test/AppDataTests.cs @@ -475,7 +475,7 @@ public void TestLoadOrCreateRecoversFromAValueASemanticTypeRejects() Assert.AreEqual("d", appData.Data, "Data should be default if a value could not be read."); Assert.IsNull(appData.Path, "Path should be default if a value could not be read."); string[] archived = AppData.FileSystem.Directory.GetFiles(filePath.AbsoluteDirectoryPath.ToString(), $"{Path.GetFileName(filePath.ToString())}.corrupt.*"); - Assert.AreEqual(1, archived.Length, "The rejected file must be archived, not left to fail again."); + Assert.HasCount(1, archived, "The rejected file must be archived, not left to fail again."); Assert.AreEqual(rejected, AppData.FileSystem.File.ReadAllText(archived[0]), "The archive must hold the original content."); }