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

UndoRedo.Test/UndoRedoStackTests.cs

@@ -538,6 +538,66 @@
 		Assert.IsTrue(navigationProvider.WasCancelled, "Navigation should have been cancelled due to timeout");
 	}
 
+	[TestMethod]
+	public async Task UndoAsync_NavigationProviderThrows_ReturnsTrueWithUndoApplied()
+	{
+		// Arrange
+		UndoRedoService stack = CreateService();
+		stack.SetNavigationProvider(new ThrowingNavigationProvider());
+		int value = 0;
+		stack.Execute(new DelegateCommand("Increment", () => value++, () => value--, navigationContext: "editor"));
+
+		// Act
+		bool result = await stack.UndoAsync().ConfigureAwait(false);
+
+		// Assert
+		Assert.IsTrue(result, "UndoAsync should report the undo, which was applied before navigation failed");
+		Assert.AreEqual(0, value);
+		Assert.AreEqual(-1, stack.CurrentPosition);
+	}
+
+	[TestMethod]
+	public async Task RedoAsync_NavigationProviderThrows_ReturnsTrueWithRedoApplied()
+	{
+		// Arrange
+		UndoRedoService stack = CreateService();
+		stack.SetNavigationProvider(new ThrowingNavigationProvider());
+		int value = 0;
+		stack.Execute(new DelegateCommand("Increment", () => value++, () => value--, navigationContext: "editor"));
+		await stack.UndoAsync(navigateToChange: false).ConfigureAwait(false);
+
+		// Act
+		bool result = await stack.RedoAsync().ConfigureAwait(false);
+
+		// Assert
+		Assert.IsTrue(result, "RedoAsync should report the redo, which was applied before navigation failed");
+		Assert.AreEqual(1, value);
+		Assert.AreEqual(0, stack.CurrentPosition);
+	}
+
+	[TestMethod]
+	public async Task UndoToSaveBoundaryAsync_NavigationProviderThrows_ReturnsTrueAtBoundary()
+	{
+		// Arrange
+		UndoRedoService stack = CreateService();
+		stack.SetNavigationProvider(new ThrowingNavigationProvider());
+		int value = 0;
+		stack.Execute(new DelegateCommand("Increment", () => value++, () => value--, navigationContext: "editor"));
+		stack.MarkAsSaved();
+		stack.Execute(new DelegateCommand("Increment", () => value++, () => value--, navigationContext: "editor"));
+		stack.Execute(new DelegateCommand("Increment", () => value++, () => value--, navigationContext: "editor"));
+		SaveBoundary boundary = stack.SaveBoundaries[0];
+
+		// Act
+		bool result = await stack.UndoToSaveBoundaryAsync(boundary).ConfigureAwait(false);
+
+		// Assert
+		Assert.IsTrue(result, "UndoToSaveBoundaryAsync should report the undo, which was applied before navigation failed");
+		Assert.AreEqual(1, value);
+		Assert.AreEqual(0, stack.CurrentPosition);
+		Assert.IsFalse(stack.HasUnsavedChanges, "The stack should be back at the save boundary");
+	}
+
 	[TestMethod]
 	public void Execute_CommandThrowsException_DoesNotCorruptStack()
 	{
@@ -1049,6 +1109,14 @@
 		}
 	}
 
+	private sealed class ThrowingNavigationProvider : INavigationProvider
+	{
+		public Task<bool> NavigateToAsync(string context, CancellationToken cancellationToken = default) =>
+			throw new InvalidOperationException("editor closed");
+
+		public bool IsValidContext(string context) => true;
+	}
+
 	private sealed class SlowNavigationProvider : INavigationProvider
 	{
 		public bool WasCancelled { get; private set; }

UndoRedo/Services/UndoRedoService.cs

@@ -165,19 +165,9 @@
 		_stackManager.MovePrevious();
 		CommandUndone?.Invoke(this, new CommandUndoneEventArgs(command, _stackManager.CurrentPosition));
 
-		if (navigateToChange && _options.EnableNavigation && _navigationProvider != null && !string.IsNullOrEmpty(command.NavigationContext))
+		if (navigateToChange)
 		{
-			using CancellationTokenSource cts = CancellationTokenSource.CreateLinkedTokenSource(cancellationToken);
-			cts.CancelAfter(_options.EffectiveNavigationTimeout);
-
-			try
-			{
-				await _navigationProvider.NavigateToAsync(command.NavigationContext!, cts.Token).ConfigureAwait(false);
-			}
-			catch (OperationCanceledException)
-			{
-				// Navigation timeout or cancellation - not critical
-			}
+			await NavigateSafelyAsync(command.NavigationContext, cancellationToken).ConfigureAwait(false);
 		}
 
 		return true;
@@ -201,19 +191,9 @@
 		_stackManager.MoveNext();
 		CommandRedone?.Invoke(this, new CommandRedoneEventArgs(command, _stackManager.CurrentPosition));
 
-		if (navigateToChange && _options.EnableNavigation && _navigationProvider != null && !string.IsNullOrEmpty(command.NavigationContext))
+		if (navigateToChange)
 		{
-			using CancellationTokenSource cts = CancellationTokenSource.CreateLinkedTokenSource(cancellationToken);
-			cts.CancelAfter(_options.EffectiveNavigationTimeout);
-
-			try
-			{
-				await _navigationProvider.NavigateToAsync(command.NavigationContext!, cts.Token).ConfigureAwait(false);
-			}
-			catch (OperationCanceledException)
-			{
-				// Navigation timeout or cancellation - not critical
-			}
+			await NavigateSafelyAsync(command.NavigationContext, cancellationToken).ConfigureAwait(false);
 		}
 
 		return true;
@@ -266,25 +246,41 @@
 			CommandUndone?.Invoke(this, new CommandUndoneEventArgs(command, _stackManager.CurrentPosition));
 		}
 
-		if (navigateToLastChange && lastCommand != null && _options.EnableNavigation && _navigationProvider != null &&
-			!string.IsNullOrEmpty(lastCommand.NavigationContext))
+		if (navigateToLastChange && lastCommand != null)
 		{
-			using CancellationTokenSource cts = CancellationTokenSource.CreateLinkedTokenSource(cancellationToken);
-			cts.CancelAfter(_options.EffectiveNavigationTimeout);
-
-			try
-			{
-				await _navigationProvider.NavigateToAsync(lastCommand.NavigationContext!, cts.Token).ConfigureAwait(false);
-			}
-			catch (OperationCanceledException)
-			{
-				// Navigation timeout or cancellation - not critical
-			}
+			await NavigateSafelyAsync(lastCommand.NavigationContext, cancellationToken).ConfigureAwait(false);
 		}
 
 		return true;
 	}
 
+	/// <summary>
+	/// Navigates to where a change was made, after the undo or redo has already been applied.
+	/// Navigation is best effort: any failure is swallowed, because an exception here would tell the
+	/// caller the undo or redo did not happen when it did, and a retry would then undo a second command.
+	/// </summary>
+	private async Task NavigateSafelyAsync(string? navigationContext, CancellationToken cancellationToken)
+	{
+		if (!_options.EnableNavigation || _navigationProvider == null || string.IsNullOrEmpty(navigationContext))
+		{
+			return;
+		}
+
+		using CancellationTokenSource cts = CancellationTokenSource.CreateLinkedTokenSource(cancellationToken);
+		cts.CancelAfter(_options.EffectiveNavigationTimeout);
+
+		try
+		{
+			await _navigationProvider.NavigateToAsync(navigationContext!, cts.Token).ConfigureAwait(false);
+		}
+#pragma warning disable CA1031 // Do not catch general exception types
+		catch (Exception)
+		{
+			// Navigation timeout, cancellation or provider failure - not critical
+		}
+#pragma warning restore CA1031 // Do not catch general exception types
+	}
+
 	/// <inheritdoc />
 	public IEnumerable<ChangeVisualization> GetChangeVisualizations(int maxItems = 50)
 	{