From ba0a426b969f79aef0bdf046b8019ad90fcf888d Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 28 Sep 2026 03:25:33 +0000 Subject: [PATCH] Report a throwing or open-generic command constructor as a failed load [patch] JsonUndoRedoSerializer only translated MissingMethodException from Activator.CreateInstance, so a saved command whose constructor throws (TargetInvocationException) or whose type is an open generic (ArgumentException) escaped LoadStateAsync instead of returning false. Catch every non-cancellation exception there, as the DeserializeData block below already does, and add both cases to the load-failure tests. Fixes #102 Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01TXZMdofSRgS8oXS9RgTRhM --- UndoRedo.Test/SerializationTests.cs | 61 +++++++++++++++++++++ UndoRedo/Services/JsonUndoRedoSerializer.cs | 11 ++++ 2 files changed, 72 insertions(+) diff --git a/UndoRedo.Test/SerializationTests.cs b/UndoRedo.Test/SerializationTests.cs index 7e08b3f..7ad405d 100644 --- a/UndoRedo.Test/SerializationTests.cs +++ b/UndoRedo.Test/SerializationTests.cs @@ -572,6 +572,8 @@ public async Task UndoRedoService_SaveLoadState_ReconstructsCommandWithEmptyData private const string InvalidVersion = "invalid assembly version"; private const string NotACommand = "serializable type that is not a command"; private const string DataParseFailure = "command data its parser rejects"; + private const string ThrowingConstructor = "command whose constructor throws"; + private const string OpenGeneric = "open generic command type"; private static async Task SerializeWithCommandTypeAsync(string caseName) { @@ -581,6 +583,8 @@ private static async Task SerializeWithCommandTypeAsync(string caseName) InvalidVersion => "Foo, Bar, Version=abc", NotACommand => typeof(SerializableNonCommand).AssemblyQualifiedName!, DataParseFailure => typeof(IntParsingSerializableCommand).AssemblyQualifiedName!, + ThrowingConstructor => typeof(ThrowingConstructorSerializableCommand).AssemblyQualifiedName!, + OpenGeneric => typeof(GenericSerializableCommand<>).AssemblyQualifiedName!, _ => throw new ArgumentOutOfRangeException(nameof(caseName)), }; @@ -600,6 +604,8 @@ private static async Task SerializeWithCommandTypeAsync(string caseName) [DataRow(InvalidVersion)] [DataRow(NotACommand)] [DataRow(DataParseFailure)] + [DataRow(ThrowingConstructor)] + [DataRow(OpenGeneric)] public async Task JsonSerializer_DeserializeUnloadableCommand_ThrowsInvalidOperationException(string caseName) { // Arrange @@ -617,6 +623,8 @@ await Assert.ThrowsExactlyAsync( [DataRow(InvalidVersion)] [DataRow(NotACommand)] [DataRow(DataParseFailure)] + [DataRow(ThrowingConstructor)] + [DataRow(OpenGeneric)] public async Task UndoRedoService_LoadStateUnloadableCommand_ReturnsFalseAndKeepsHistory(string caseName) { // Arrange @@ -673,6 +681,59 @@ public override void Undo() } #pragma warning restore CA1812 +#pragma warning disable CA1812 // Instantiated by reflection during deserialization + private sealed class ThrowingConstructorSerializableCommand : BaseCommand, ISerializableCommand + { + public ThrowingConstructorSerializableCommand() : base(ChangeType.Modify, ["test"]) => + throw new InvalidDataException("Needs context the loader cannot supply"); + + public override string Description => "Throwing constructor"; + + public override void Execute() + { + // Test implementation + } + + public override void Undo() + { + // Test implementation + } + + public string SerializeData() => string.Empty; + + public void DeserializeData(string data) + { + // Nothing to restore + } + } + + private sealed class GenericSerializableCommand : BaseCommand, ISerializableCommand + { + public GenericSerializableCommand() : base(ChangeType.Modify, ["test"]) + { + } + + public override string Description => $"Generic command of {typeof(T).Name}"; + + public override void Execute() + { + // Test implementation + } + + public override void Undo() + { + // Test implementation + } + + public string SerializeData() => string.Empty; + + public void DeserializeData(string data) + { + // Nothing to restore + } + } +#pragma warning restore CA1812 + private sealed class EmptyDataSerializableCommand : BaseCommand, ISerializableCommand { public EmptyDataSerializableCommand() : base(ChangeType.Modify, ["test"]) diff --git a/UndoRedo/Services/JsonUndoRedoSerializer.cs b/UndoRedo/Services/JsonUndoRedoSerializer.cs index cd57415..89b39bf 100644 --- a/UndoRedo/Services/JsonUndoRedoSerializer.cs +++ b/UndoRedo/Services/JsonUndoRedoSerializer.cs @@ -171,6 +171,17 @@ private static ICommand ConvertFromSerializableCommand(SerializableCommand seria $"Cannot reconstruct command type '{commandType.FullName}': {nameof(ISerializableCommand)} implementations must have a public parameterless constructor for DeserializeData to populate.", ex); } +#pragma warning disable CA1031 // Do not catch general exception types + catch (Exception ex) when (ex is not OperationCanceledException) +#pragma warning restore CA1031 // Do not catch general exception types + { + // The constructor itself can throw (surfacing as TargetInvocationException or + // TypeInitializationException), and an open generic type cannot be constructed at all + // (ArgumentException). Report these through the deserialization contract as well. + throw new InvalidOperationException( + $"Cannot reconstruct command type '{commandType.FullName}': its public parameterless constructor could not create an instance.", + ex); + } try {