What's wrong
NavigateSafelyAsync (UndoRedo/Services/UndoRedoService.cs:269-270) is meant to never throw. Its doc comment says a failure there "would tell the caller the undo or redo did not happen when it did, and a retry would then undo a second command", and #100 fixed exactly that. But the CancellationTokenSource setup sits outside the try:
using CancellationTokenSource cts = CancellationTokenSource.CreateLinkedTokenSource(cancellationToken);
cts.CancelAfter(_options.EffectiveNavigationTimeout); // <- can throw, not inside try
try
{
await _navigationProvider.NavigateToAsync(navigationContext!, cts.Token).ConfigureAwait(false);
}
catch (Exception) { }
CancelAfter(TimeSpan) throws ArgumentOutOfRangeException for any value above uint.MaxValue - 1 ms (~49.7 days) and for any negative value other than -1 ms. Nothing validates the timeout. UndoRedoOptions.DefaultNavigationTimeout and UndoRedoOptionsBuilder.WithNavigationTimeout(TimeSpan) both accept any TimeSpan, and TimeSpan.MaxValue is a natural way to say "no timeout".
Repro
var service = new UndoRedoService(/* stack/boundary managers */, navigationProvider,
new UndoRedoOptions(DefaultNavigationTimeout: TimeSpan.MaxValue));
int v = 0;
await service.ExecuteAsync(new DelegateCommand(() => v++, () => v--, navigationContext: "ctx"));
await service.ExecuteAsync(new DelegateCommand(() => v++, () => v--, navigationContext: "ctx"));
await service.UndoAsync();
- Observed:
ArgumentOutOfRangeException (Parameter 'delay') from line 270. At that point v == 1 and CurrentPosition == 0, so the undo was already applied. A caller that retries on the exception undoes a second command.
- Expected:
UndoAsync returns true. Navigation runs with no timeout (or is skipped), and in any case it must not surface an exception.
The same applies to RedoAsync and UndoToSaveBoundaryAsync, which also go through NavigateSafelyAsync.
Suggested fix
- Move the
CreateLinkedTokenSource/CancelAfter lines inside the try, so NavigateSafelyAsync really can't throw.
- Treat
Timeout.InfiniteTimeSpan, values ≤ 0 and values above the CancelAfter limit as "no timeout" and skip CancelAfter. Alternatively, validate the value in UndoRedoOptions/WithNavigationTimeout so it fails at configuration time rather than mid-undo.
- Add a regression test using
TimeSpan.MaxValue and a negative timeout.
What's wrong
NavigateSafelyAsync(UndoRedo/Services/UndoRedoService.cs:269-270) is meant to never throw. Its doc comment says a failure there "would tell the caller the undo or redo did not happen when it did, and a retry would then undo a second command", and #100 fixed exactly that. But theCancellationTokenSourcesetup sits outside thetry:CancelAfter(TimeSpan)throwsArgumentOutOfRangeExceptionfor any value aboveuint.MaxValue - 1ms (~49.7 days) and for any negative value other than-1 ms. Nothing validates the timeout.UndoRedoOptions.DefaultNavigationTimeoutandUndoRedoOptionsBuilder.WithNavigationTimeout(TimeSpan)both accept anyTimeSpan, andTimeSpan.MaxValueis a natural way to say "no timeout".Repro
ArgumentOutOfRangeException (Parameter 'delay')from line 270. At that pointv == 1andCurrentPosition == 0, so the undo was already applied. A caller that retries on the exception undoes a second command.UndoAsyncreturnstrue. Navigation runs with no timeout (or is skipped), and in any case it must not surface an exception.The same applies to
RedoAsyncandUndoToSaveBoundaryAsync, which also go throughNavigateSafelyAsync.Suggested fix
CreateLinkedTokenSource/CancelAfterlines inside thetry, soNavigateSafelyAsyncreally can't throw.Timeout.InfiniteTimeSpan, values ≤ 0 and values above theCancelAfterlimit as "no timeout" and skipCancelAfter. Alternatively, validate the value inUndoRedoOptions/WithNavigationTimeoutso it fails at configuration time rather than mid-undo.TimeSpan.MaxValueand a negative timeout.