Exception handling inside "async void" WPF command handlers

Question

I'm reviewing some WPF code of my colleagues, which is a library of UserControl-based components with a lot of async void event and command handlers. These methods currently do not implement any error handling internally.

The code in a nutshell:

<Window.CommandBindings>
    <CommandBinding
        Command="ApplicationCommands.New"
        Executed="NewCommand_Executed"/>
</Window.CommandBindings>

private async void NewCommand_Executed(object sender, ExecutedRoutedEventArgs e)
{
    // do some fake async work (and may throw if timeout < -1)
    var timeout = new Random(Environment.TickCount).Next(-100, 100);
    await Task.Delay(timeout);
}

Exceptions thrown but not observed inside NewCommand_Executed can only be handled on a global level (e.g., with AppDomain.CurrentDomain.UnhandledException). Apparently, this is not a good idea.

I could handle exceptions locally:

private async void NewCommand_Executed(object sender, ExecutedRoutedEventArgs e)
{
    try
    {
        // do some fake async work (throws if timeout < -1)
        var timeout = new Random(Environment.TickCount).Next(-100, 100);
        await Task.Delay(timeout);
    }
    catch (Exception ex)
    {
        // somehow log and report the error
        MessageBox.Show(ex.Message);
    }
}

However, in this case the host app's ViewModel would be unaware of errors inside NewCommand_Executed. Not an ideal solution either, plus the error reporting UI shouldn't always be a part of the library code.

Another approach is to handle them locally and fire a dedicated error event:

public class AsyncErrorEventArgs: EventArgs
{
    public object Sender { get; internal set; }
    public ExecutedRoutedEventArgs Args { get; internal set; }
    public ExceptionDispatchInfo ExceptionInfo { get; internal set; }
}

public delegate void AsyncErrorEventHandler(object sender, AsyncErrorEventArgs e);

public event AsyncErrorEventHandler AsyncErrorEvent;

private async void NewCommand_Executed(object sender, ExecutedRoutedEventArgs e)
{
    ExceptionDispatchInfo exceptionInfo = null;

    try
    {
        // do some fake async work (throws if timeout < -1)
        var timeout = new Random(Environment.TickCount).Next(-100, 100);
        await Task.Delay(timeout);
    }
    catch (Exception ex)
    {
        // capture the error
        exceptionInfo = ExceptionDispatchInfo.Capture(ex);
    }

    if (exceptionInfo != null && this.AsyncErrorEvent != null)
        this.AsyncErrorEvent(sender, new AsyncErrorEventArgs { 
            Sender = this, Args = e, ExceptionInfo = exceptionInfo });
}

I like the last one the most, but I'd appreciate any other suggestions as my experience with WPF is somewhat limited.

• Is there an established WPF pattern to propagate errors from async void command handlers to ViewModal?

• Is it generally a bad idea to do async work inside WPF command handlers, as perhaps they're intended for quick synchronous UI updates?

I'm asking this question in the context of WPF, but I think it may as well apply to async void event handlers in WinForms.

Answer 1

The issue here is that your UserControl library is not architected in a Typical MVVM way. Commonly, for non-trivial commands, your UserControl's code would not bind to commands directly, but instead would have properties that when set (through binding to a ViewModel) would trigger the action in the control. Then your ViewModel would bind to the application command, and set the appropriate properties. (Alternatively, your MVVM framework may have another message passing scenario that can be leveraged for interaction between the ViewModel and View).

As for Exceptions that are thrown inside the UI, I again feel that there is an architecture issue. If the UserControl is doing more than acting as a View, (i.e. running any kind of business logic that might cause unanticipated exceptions) then this should be separated into a View and a ViewModel. The ViewModel would run the logic and could either be instantiated by your other application ViewModels, or communicate via another method (as mentioned above).

If there are exceptions being thrown by the UserControl's layout / visualization code then this should (almost without exception) not be caught in any way by your ViewModel. This should, as you mentioned, only be handled for logging by a global level handler.

Lastly, if there truly are known 'exceptions' in the Control's code that your ViewModel needs to be notified about, I suggest catching the known exceptions and raising an event/command and setting a property. But again, this really shouldn't be used for exceptions, just anticipated 'error' states.

Answer 2

The propagation of exceptions about which the users are almost 100% unaware is not a good practice in my opinion. See this

I see the two options you really have since WPF doesn't provide any out of the box mechanisms of such the notifying of any problems:

1. The way you already offered with catching and firing the event.
2. Return the Task object from the async method (in your case, it seems that you will have to expose it through the property). The users will be able to check if there were any errors during the execution and attach a continuation task if they want. Inside the handler you can catch any exceptions and use TaskCompletionSource to set the result of the handler.

All in all you have to write some xml-comments for such a code, because that's not so easy to understand it. The most important thing is that you should never (almost) throw any exceptions from any secondary threads.