Skip to content

Report a throwing or open-generic command constructor as a failed load [patch] - #116

Merged
matt-edmondson merged 1 commit into
mainfrom
claude/undoredo-102-constructor-exceptions
Sep 28, 2026
Merged

matt-edmondson merged 1 commit into
mainfrom
claude/undoredo-102-constructor-exceptions

Conversation

@matt-edmondson

Copy link
Copy Markdown
Contributor

Fixes #102

What was wrong

JsonUndoRedoSerializer.ConvertFromSerializableCommand only translated MissingMethodException from Activator.CreateInstance into the InvalidOperationException that LoadStateAsync turns into false. Two other constructor failures escaped to the caller instead of the documented failed-load result:

  • a parameterless constructor that throws, which surfaces as TargetInvocationException
  • a saved type name that resolves to an open generic, which surfaces as ArgumentException

Change

  • Around Activator.CreateInstance, keep the specific MissingMethodException message and add a catch for every other non-cancellation exception. The catch wraps the exception in InvalidOperationException, the same pattern the DeserializeData block just below already uses. This also covers TypeInitializationException and MemberAccessException.
  • Add ThrowingConstructor and OpenGeneric cases to the LoadStateAsync throws FileLoadException/InvalidCastException/FormatException on a bad command "type" or data instead of returning false #93 data-driven tests, for both the serializer (JsonSerializer_DeserializeUnloadableCommand_ThrowsInvalidOperationException) and the service (UndoRedoService_LoadStateUnloadableCommand_ReturnsFalseAndKeepsHistory, which returns false and keeps the live stack).

Testing

  • On main, all 4 new cases fail with the exceptions the issue reports.
  • With the fix, dotnet test UndoRedo.Test passes 104/104.

🤖 Generated with Claude Code

https://claude.ai/code/session_01TXZMdofSRgS8oXS9RgTRhM


Generated by Claude Code

…d [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 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01TXZMdofSRgS8oXS9RgTRhM
@sonarqubecloud

Copy link
Copy Markdown

@matt-edmondson
matt-edmondson merged commit 6c367b2 into main Sep 28, 2026
12 checks passed
@matt-edmondson
matt-edmondson deleted the claude/undoredo-102-constructor-exceptions branch September 28, 2026 08:21
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 instead of returning false when a saved command's constructor throws or its type is an open generic

2 participants