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.
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:
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.
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.