StephenCleary / Mvvm

MVVM helpers, including calculated properties and asynchronous notification tasks.
MIT License
134 stars 38 forks source link

CancelCommand WrapDelegate needs try catch #9

Closed pdinnissen closed 7 years ago

pdinnissen commented 7 years ago

I get an unhandled TaskCancellationException when cancelling a Command.

I believe you need to add a try catch block to WrapDelegate:

public static Func<object, Task> WrapDelegate(Func<object, CancellationToken, Task> executeAsync)
        {
            return async () =>
            {
                using (StartOperation())
                {
                    try
                    {
                        await executeAsync(CancellationToken);
                    }
                    catch
                    {
                    }
                }
            };
        }
StephenCleary commented 7 years ago

For asynchronous commands, any exceptions that escape the command delegate are raised on the UI context. This is done deliberately, and matches the behavior of synchronous commands.

If you want to prevent this exception from being raised on the UI context, you should catch it within the delegate you pass to WrapDelegate.

pdinnissen commented 7 years ago

Even for the TaskCancellationException of the task through CancelCommand?

I see the logic, but I feel that I'm just going to write a wrapper try/catch anyways. How is this any different than the try catch in NotifyTask?

StephenCleary commented 7 years ago

I can see where catching an OperationCanceledException for that specific CancellationToken would be desired behavior. I'll re-open this.

StephenCleary commented 7 years ago

Fixed in 1.0.0-eta-03.

Added a catch for OperationCanceledException. The CancellationToken is not checked, since end-user delegates may be using a linked cancellation token.