Skip to content

Stop a navigation failure from surfacing after undo/redo is applied - #100

Merged
matt-edmondson merged 1 commit into
mainfrom
claude/undoredo-89-navigation-exception
Sep 27, 2026
Merged

matt-edmondson merged 1 commit into
mainfrom
claude/undoredo-89-navigation-exception

Conversation

@matt-edmondson

Copy link
Copy Markdown
Contributor

Fixes #89

What was wrong

UndoAsync, RedoAsync and UndoToSaveBoundaryAsync apply the change, move the position and raise the event, and only then navigate. Around that navigation call they caught only OperationCanceledException. If the navigation provider threw anything else, such as an InvalidOperationException because the editor was closed, the exception reached the caller even though the undo had already happened. Under the #75 contract, a caller that sees an exception assumes the stack did not move. It would retry and undo a second command.

Change

  • The three navigation call sites now go through one private helper, NavigateSafelyAsync. It keeps the existing checks (EnableNavigation, a provider is set, the context is non-empty) and the linked timeout.
  • The helper treats every navigation failure as non-critical, the same way it already treated timeouts and cancellation. Once the state has changed, it never throws.
  • The public API is unchanged. I didn't add a failure event, because that would mean adding a member to IUndoRedoService.

Tests

  • Added three regression tests, one each for UndoAsync, RedoAsync and UndoToSaveBoundaryAsync. Each uses a navigation provider that throws, and checks the return value, the command's effect and the resulting position.
  • With the service change reverted, all three tests fail. With it applied, they pass.
  • The full suite passes locally: 93 of 93.

🤖 Generated with Claude Code

https://claude.ai/code/session_01TX4nouJvSXNs7eSvDP13qU


Generated by Claude Code

…ied [patch]

UndoAsync, RedoAsync and UndoToSaveBoundaryAsync applied the change, moved
the position and raised the event before navigating, but only caught
OperationCanceledException from the navigation provider. Any other
provider exception reached the caller after the undo had succeeded, so a
caller following the #75 contract would retry and undo a second command.

The three call sites now share NavigateSafelyAsync, which treats every
navigation failure as non-critical, as it already did for timeouts.

Fixes #89

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01TX4nouJvSXNs7eSvDP13qU
@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.

UndoAsync/RedoAsync throw a navigation-provider exception after the undo/redo has already been applied, so a retry undoes a second command

1 participant