Skip to content

Report every unloadable saved command as a failed load - #99

Merged
matt-edmondson merged 2 commits into
mainfrom
claude/undoredo-93-bad-command-type
Sep 27, 2026
Merged

matt-edmondson merged 2 commits into
mainfrom
claude/undoredo-93-bad-command-type

Conversation

@matt-edmondson

Copy link
Copy Markdown
Contributor

Fixes #93

What changed

LoadStateAsync promises to return false for data it cannot load. Three failures inside JsonUndoRedoSerializer.ConvertFromSerializableCommand threw raw exceptions instead, which escaped its JsonException/InvalidOperationException/NotSupportedException filter:

Saved type / data Before After
"Foo, =bad" FileLoadException false
"Foo, Bar, Version=abc" FileLoadException false
a type that implements ISerializableCommand but not ICommand InvalidCastException false
a command whose DeserializeData calls int.Parse("abc") FormatException false

The serializer now turns each of these into InvalidOperationException, with the original exception kept as the inner exception. This is the same treatment MissingMethodException already gets:

  • Type.GetType is wrapped in a small ResolveCommandType helper. It catches IOException, BadImageFormatException, ArgumentException and TypeLoadException. A type that simply isn't found still returns null and falls back to a placeholder, as before.
  • The ICommand check happens before the command is constructed.
  • DeserializeData is the command's own code, so any exception from it is wrapped. OperationCanceledException is let through. The CA1031 pragma follows the pattern already used in CompositeCommand and UndoRedoService.

Tests

  • JsonSerializer_DeserializeUnloadableCommand_ThrowsInvalidOperationException has one case per row of the table.
  • UndoRedoService_LoadStateUnloadableCommand_ReturnsFalseAndKeepsHistory runs the same four cases through LoadStateAsync and checks the live history is kept.

All 8 new cases failed on main with the exceptions listed in the issue. The full suite passes with the change (77/77), and the library builds for every target framework.

🤖 Generated with Claude Code

https://claude.ai/code/session_01N4m2HJkrsY88XBtTqTKAkE


Generated by Claude Code

LoadStateAsync promises to return false for data it cannot load, but a
malformed assembly-qualified type name threw FileLoadException, a type that
implements ISerializableCommand without ICommand threw InvalidCastException,
and a command whose DeserializeData rejected its data let that exception
escape. Translate each into InvalidOperationException with the original as
the inner exception, the way the missing-constructor case already is, and
check for ICommand before constructing the command.

Fixes #93

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01N4m2HJkrsY88XBtTqTKAkE
…d-command-type

# Conflicts:
#	UndoRedo.Test/SerializationTests.cs
@sonarqubecloud

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.

LoadStateAsync throws FileLoadException/InvalidCastException/FormatException on a bad command "type" or data instead of returning false

2 participants