LoadStateAsync throws instead of returning false when a saved command's constructor throws or its type is an open generic

[matt-edmondson]

What's wrong

The fix for #93 (PR #99) made LoadStateAsync report false for every unloadable saved command. Two exceptions from Activator.CreateInstance still get through:

• JsonUndoRedoSerializer.ConvertFromSerializableCommand (UndoRedo/Services/JsonUndoRedoSerializer.cs:~158-170) catches only MissingMethodException around Activator.CreateInstance(commandType).
• UndoRedoService.LoadStateAsync (UndoRedo/Services/UndoRedoService.cs:351) catches only JsonException, InvalidOperationException and NotSupportedException.

So these escape to the caller:

• TargetInvocationException, when the parameterless constructor itself throws
• ArgumentException, when the saved type name resolves to an open generic (ContainsGenericParameters)

TypeInitializationException and MemberAccessException would escape the same way.

Repro (verified against the built library)

public class ThrowingCtorCmd : BaseCommand, ISerializableCommand
{
    public ThrowingCtorCmd() : base(ChangeType.Modify, []) => throw new InvalidDataException("needs context");
    public override string Description => "t";
    public override void Execute() { }
    public override void Undo() { }
    public string SerializeData() => "1";
    public void DeserializeData(string d) { }
}

Hand-craft saved state with one command whose "type" is the assembly-qualified name of ThrowingCtorCmd, or of an open GenericCmd<> with the same shape, and pass it to LoadStateAsync.

• Actual: TargetInvocationException (or ArgumentException: Cannot create an instance of GenericCmd1[T] because Type.ContainsGenericParameters is true) escapes LoadStateAsync`.
• Expected: false, and the live history is kept. This is what the LoadStateAsync throws FileLoadException/InvalidCastException/FormatException on a bad command "type" or data instead of returning false #93 fix promises, and what already happens for an abstract type or one with only a private constructor.

Why it matters

Command constructors that depend on services, configuration or static state are common. Opening a file saved by another build, or after such a dependency changes, crashes the caller instead of producing the documented "failed load" result.

Suggested fix

Around Activator.CreateInstance, catch MissingMethodException or TargetInvocationException or ArgumentException or MemberAccessException or TypeInitializationException, or everything except OperationCanceledException, as the DeserializeData block just below already does. Wrap the exception in InvalidOperationException.

Acceptance: add a throwing constructor and an open generic type to the #93 data-driven load-failure tests. Both return false and leave the stack unchanged.

[matt-edmondson]

Triage

• Category: Bug. This is a gap left by the LoadStateAsync throws FileLoadException/InvalidCastException/FormatException on a bad command "type" or data instead of returning false #93 fix (PR Report every unloadable saved command as a failed load #99).
• Priority: Medium. The caller crashes instead of getting the documented false, but only for saved state whose command type cannot be constructed.
• Area / suggested assignee: JsonUndoRedoSerializer.ConvertFromSerializableCommand and UndoRedoService.LoadStateAsync. Owner: @matt-edmondson
• Duplicates / in progress: it follows on from LoadStateAsync throws FileLoadException/InvalidCastException/FormatException on a bad command "type" or data instead of returning false #93 and is not a duplicate. Open PRs Stop a navigation failure from surfacing after undo/redo is applied #100 (UndoAsync/RedoAsync throw a navigation-provider exception after the undo/redo has already been applied, so a retry undoes a second command #89) and Resolve a held save boundary to its live position before undoing to it #101 (UndoToSaveBoundaryAsync undoes to the wrong position when given a SaveBoundary obtained before the stack was trimmed #88) change undo/navigation and save boundaries, not load. Neither covers this issue, but Stop a navigation failure from surfacing after undo/redo is applied #100 touches UndoRedoService, so expect a small merge overlap.
• Next step: widen the catch around Activator.CreateInstance to match the DeserializeData block, and add both cases to the LoadStateAsync throws FileLoadException/InvalidCastException/FormatException on a bad command "type" or data instead of returning false #93 data-driven tests.

---

Generated by Claude Code