[Question] Global exception handling & IAsyncRelayCommand

[ShahinDohan]

Forgive me if I'm overlooking something, but I can't seem to figure out how to catch exceptions thrown from a AsyncRelayCommand handler in my global exception handlers. Is this even possible?

With RelayCommand it works fine, but with AsyncRelayCommand I can only see the exception in the Visual Studio output window.

MainWindow.xaml

<Grid>
    <StackPanel VerticalAlignment="Center" HorizontalAlignment="Center">
        <Button Content="Regular command" Command="{Binding DoCommand}" />
        <Button Content="Async command" Command="{Binding DoAsyncCommand}" Margin="0,15,0,0" />
    </StackPanel>
</Grid>

MainWindowViewModel.cs

public class MainWindowViewModel : ObservableObject
{
    public MainWindowViewModel()
    {
        DoAsyncCommand = new AsyncRelayCommand(OnDoAsync);
        DoCommand = new RelayCommand(OnDo);
    }

    private void OnDo()
    {
        throw new Exception("OnDo exception!");
    }

    private async Task OnDoAsync()
    {
        await Task.Delay(0);
        throw new Exception("OnDoAsync exception!");
    }

    public IAsyncRelayCommand DoAsyncCommand { get; }
    public IRelayCommand DoCommand { get; }
}

App.xaml.cs

public partial class App : Application
{
    protected override void OnStartup(StartupEventArgs e)
    {
        SetupExceptionHandlers();
        base.OnStartup(e);
    }

    private static void SetupExceptionHandlers()
    {
        AppDomain.CurrentDomain.UnhandledException += (s, a) =>
        {
            HandleException((Exception) a.ExceptionObject);
        };

        Current.DispatcherUnhandledException += (s, a) =>
        {
            HandleException(a.Exception);
            a.Handled = true;
        };

        TaskScheduler.UnobservedTaskException += (s, a) =>
        {
            HandleException(a.Exception);
            a.SetObserved();
        };
    }

    private static void HandleException(Exception exception)
    {
        MessageBox.Show(exception.GetBaseException().Message, "Unhandled Exception", MessageBoxButton.OK, MessageBoxImage.Error);
    }
}

When I was using Caliburn.Micro framework a few years back, I had a similar issue when I switched from IHandle to IHandleWithTask for async support when using the event aggregator (similar to messenger), and I was able to fix it by adding an additional Caliburn-specific exception handler like this:

Coroutine.Completed += (s, a) =>
{
    if (a.Error != null)
    {
        HandleException(a.Error);
    }
};

Is there something similar that I have to do with MVVM Toolkit?

Thank you very much! 🙏

EDIT: I'm using WPF

[ghost]

Hello ShahinDohan, thank you for your interest in Windows Community Toolkit!

I have automatically added a "needs triage" label to help get things started. Our team will analyze and investigate the issue, and escalate it to the relevant team if possible.. Other community members may also answer the question and provide feedback 🙌

[Rosuavio]

Any ideas @Sergio0694?

[Sergio0694]

Hi @ShahinDohan, the reason why faulted async commands are not bubbling up is because we're explicitly ignoring exceptions there to avoid crashing the entire application in case a commands fails to execute but the exception isn't handled.
This is done in ObservableObject.SetPropertyAndNotifyOnCompletion, here:

https://github.com/windows-toolkit/WindowsCommunityToolkit/blob/dc5a00be1f69639806de57dff24b5ec17249f109/Microsoft.Toolkit.Mvvm/ComponentModel/ObservableObject.cs#L482-L489

This method is called internally by AsyncRelayCommand (and generic version) when executing the delegate.
We don't have a "global" handler of sorts, but you should be able to add a handler to individual commands like so:

AsyncRelayCommand command = ...; // Assume we have a command here

command.PropertyChanged += (s, e) =>
{
    if (e.PropertyName == nameof(AsyncRelayCommand.ExecutionTask) &&
        ((AsyncRelayCommand)s).ExecutionTask is Task task &&
        task.Exception is AggregateException exception)
    {
        // Log your exception here
    }
};

As in, the ExecutionTask property raises a notification event when a task is set or when it completes, so you can subscribe to it and check whether the completed task (if any) has thrown any exceptions, and if so, log them. If this is something that is considered particularly useful I guess we could add a dedicated event for this in a future release 🙂

Let me know what you think!

cc. @michael-hawker

[ShahinDohan]

@Sergio0694 Thank you Sergio for the explanation! I tried your solution and it worked, but to be honest it's quite ugly to have that for every async command 😄

I don't know how useful others would find this, but to me it sounds like a very important feature to have. I would never expect uncaught exceptions to be swallowed. The user would never know something went wrong unless we explicitly catch exceptions in the command handler and show a message box or at least log the error, and even then developers could easily forget to do that.

I would definitely appreciate a dedicated event for this if possible 🙏 Thank you!

[Sergio0694]

Happy to help! 🙂

"I would definitely appreciate a dedicated event for this if possible"

Just to be clear, I was referring to an instance event on the command type(s), not a global static one.
Would that still work for you?

Alternatively, just hypothetically speaking, would you rather have commands throw exceptions directly?

[ShahinDohan]

Oh, sorry I thought it would be a global static one. I don't think an instance event would be too helpful since I can just catch the exception in my async command handler, but I'd like the exception to propagate if not handled.

Alternatively, just hypothetically speaking, would you rather have commands throw exceptions directly?

Well since the regular RelayCommand does exactly that, I expected the async one to work the same way. Maybe there's a technical reason why that's not a good idea, but yes I would personally prefer that it just threw the exceptions directly.

I think it's the developers responsibility to prevent the application from crashing by handling the exception in the global exception handlers, like what I'm doing as well.

My 2 cents 🙂

[Sergio0694]

Yeah, that's a fair point. The reason why SetPropertyAndNotifyOnCompletion (and therefore AsyncRelayCommand too, as it uses that behind the scenes) doesn't just throw the exceptions is because it awaits the task in an async void method, which means that unlike with RelayCommand, the entire app would just crash. As in, you wouldn't get the nice stacktrace pointing to the command beind executed directly following eg. a button press, but instead you'd see some random crash in a continuation method coming from somewhere else in the library (and at a later point in time than the action that triggered it), which would be confusing/unintuitive for many users. Different issue if we used an async Task method - then we wouldn't crash the app but the exception would just be swallowed there and still you wouldn't just see it as you'd expect. There's a number of different approaches here and it's just difficult to decide on "the best one" (if there's such a thing) that works for everyone 🙂

Eg. you might prefer a single static event, whereas others might prefer individual instance events for every command.

[ShahinDohan]

Yeah I figured there was some technical reason for all of this. Unfortunately I don't know what would work for everyone, perhaps others can share their opinions on the matter, but personally I think it's nice to have some way to globally catch these exceptions so they don't just silently disappear.

Caliburn.Micro's Coroutine solution was quite nice, and it always showed useful stack traces as far as I remember. Perhaps something inspired by that could be done in MVVM Toolkit?

For now I'll just have to remember to add a try-catch in every AsyncRelayCommand handler, and use my awesome MVVM DialogService to show the error in the UI and log it.

Curious to hear what others think!

[Rosuavio]

@Sergio0694 Thanks for the explanation, I found it quite informative.

[virzak]

A few months after switching to this library from MvvmLight, I ran into this exact issue.

Unfortunately, discovering this came by the way of my client reporting a functionality behaving "weird". Previously Application.Current.DispatcherUnhandledException += Application_DispatcherUnhandledException; was a global handler which handled exceptions and displayed them in a message box. Now the client wasn't even aware of the exception.

This must be mentioned in the AsyncRelayCommand section of the migration guide with a huge warning.

Ideally migrating should have an option to keep a global error handler.

Since there is no global handler currently available, I'll be using a method to generate all AsyncRelayCommands to keep instantiation to a single line. This doesn't seem like the worst solution, since the complex code is confined to the AsyncCommandCreate helper method. Perhaps a future release could be provide a better solution.

MyCommandNumber156 = AsyncCommandCreate<MyClass>(async arg => await SomeMethodAsync(arg, param1));

...

public AsyncRelayCommand<T> AsyncCommandCreate<T>(Func<T?, Task> execute, Predicate<T?> canExecute)
{
    var command = new AsyncRelayCommand<T>(execute, canExecute);
    command.PropertyChanged += (s, e) =>
    {
        if (s is null)
            return;

        if (e.PropertyName == nameof(AsyncRelayCommand.ExecutionTask) &&
            ((IAsyncRelayCommand)s).ExecutionTask is Task task &&
            task.Exception is AggregateException exception)
        {
            logger.LogCritical(exception, "Exception");
            var errorVm = new ErrorDialogViewModel(exception, additionalArgs);
            DialogService.ShowDialog(this, errorVm);
        }
    };
    return command;
}

@Sergio0694

[virzak]

I ended up creating a Factory interface in the Mvvm Layer

/// <summary>
/// Factory for creating <see cref="AsyncRelayCommand"/>s with additional processing.
/// </summary>
public interface IAsyncRelayCommandFactory
    {
        /// <summary>
        /// Create a reference to AsyncRelayCommand.
        /// </summary>
        /// <param name="cancelableExecute">The cancelable execution logic.</param>
        /// <returns>AsyncRelayCommand.</returns>
        AsyncRelayCommand Create(Func<CancellationToken, Task> cancelableExecute);

        /// <summary>
        /// Create a reference to AsyncRelayCommand.
        /// </summary>
        /// <param name="cancelableExecute">The cancelable execution logic.</param>
        /// <param name="canExecute">The execution status logic.</param>
        /// <returns>AsyncRelayCommand.</returns>
        AsyncRelayCommand Create(Func<CancellationToken, Task> cancelableExecute, Func<bool> canExecute);

        /// <summary>
        /// Create a reference to AsyncRelayCommand.
        /// </summary>
        /// <param name="execute">The execution logic.</param>
        /// <returns>AsyncRelayCommand.</returns>
        AsyncRelayCommand Create(Func<Task> execute);

        /// <summary>
        /// Create a reference to AsyncRelayCommand.
        /// </summary>
        /// <param name="execute">The execution logic.</param>
        /// <param name="canExecute">The execution status logic.</param>
        /// <returns>AsyncRelayCommand.</returns>
        AsyncRelayCommand Create(Func<Task> execute, Func<bool> canExecute);

        /// <summary>
        /// Create a reference to AsyncRelayCommand.
        /// </summary>
        /// <typeparam name="T">The type of the command parameter.</typeparam>
        /// <param name="cancelableExecute">The cancelable execution logic.</param>
        /// <returns>AsyncRelayCommand.</returns>
        AsyncRelayCommand<T> Create<T>(Func<T?, CancellationToken, Task> cancelableExecute);

        /// <summary>
        /// Create a reference to AsyncRelayCommand.
        /// </summary>
        /// <typeparam name="T">The type of the command parameter.</typeparam>
        /// <param name="cancelableExecute">The cancelable execution logic.</param>
        /// <param name="canExecute">The execution status logic.</param>
        /// <returns>AsyncRelayCommand.</returns>
        AsyncRelayCommand<T> Create<T>(Func<T?, CancellationToken, Task> cancelableExecute, Predicate<T?> canExecute);

        /// <summary>
        /// Create a reference to AsyncRelayCommand.
        /// </summary>
        /// <typeparam name="T">The type of the command parameter.</typeparam>
        /// <param name="execute">The execution logic.</param>
        /// <returns>AsyncRelayCommand.</returns>
        AsyncRelayCommand<T> Create<T>(Func<T?, Task> execute);

        /// <summary>
        /// Create a reference to AsyncRelayCommand.
        /// </summary>
        /// <typeparam name="T">The type of the command parameter.</typeparam>
        /// <param name="execute">The execution logic.</param>
        /// <param name="canExecute">The execution status logic.</param>
        /// <returns>AsyncRelayCommand.</returns>
        AsyncRelayCommand<T> Create<T>(Func<T?, Task> execute, Predicate<T?> canExecute);
    }

and its platform specific implementation in the application layer. This one is for a WPF app, but some parts could be different for Blazor:

public class AsyncRelayCommandFactory : IAsyncRelayCommandFactory
{
    private readonly ILogger<AsyncRelayCommandFactory> logger;
    private readonly IDialogService dialogService;

    public AsyncRelayCommandFactory(ILogger<AsyncRelayCommandFactory> logger, IDialogService dialogService)
    {
        this.logger = logger;
        this.dialogService = dialogService;
    }

    public AsyncRelayCommand<T> Create<T>(Func<T?, Task> execute)
        => Register(new AsyncRelayCommand<T>(execute));

    public AsyncRelayCommand<T> Create<T>(Func<T?, CancellationToken, Task> cancelableExecute)
        => Register(new AsyncRelayCommand<T>(cancelableExecute));

    public AsyncRelayCommand<T> Create<T>(Func<T?, Task> execute, Predicate<T?> canExecute)
        => Register(new AsyncRelayCommand<T>(execute, canExecute));

    public AsyncRelayCommand<T> Create<T>(Func<T?, CancellationToken, Task> cancelableExecute, Predicate<T?> canExecute)
        => Register(new AsyncRelayCommand<T>(cancelableExecute, canExecute));

    public AsyncRelayCommand Create(Func<Task> execute)
        => Register(new AsyncRelayCommand(execute));

    public AsyncRelayCommand Create(Func<CancellationToken, Task> cancelableExecute)
        => Register(new AsyncRelayCommand(cancelableExecute));

    public AsyncRelayCommand Create(Func<Task> execute, Func<bool> canExecute)
        => Register(new AsyncRelayCommand(execute, canExecute));

    public AsyncRelayCommand Create(Func<CancellationToken, Task> cancelableExecute, Func<bool> canExecute)
        => Register(new AsyncRelayCommand(cancelableExecute, canExecute));

    private AsyncRelayCommand Register(AsyncRelayCommand command)
    {
        RegisterError(command);
        return command;
    }

    private AsyncRelayCommand<T> Register<T>(AsyncRelayCommand<T> command)
    {
        RegisterError(command);
        return command;
    }

    private void RegisterError(IAsyncRelayCommand command) => command.PropertyChanged += (s, e) =>
        {
            if (s is null)
            {
                return;
            }

            if (e.PropertyName == nameof(AsyncRelayCommand.ExecutionTask) &&
                ((IAsyncRelayCommand)s).ExecutionTask is Task task &&
                task.Exception is AggregateException exception)
            {
                logger.LogCritical(exception, "Exception");
                var errorVm = new ErrorDialogViewModel(exception, additionalArgs);
                dialogService.ShowDialog(this, errorVm);
            }
        };
}

The interface would be injected into the caller class and used like this:

private readonly IAsyncRelayCommandFactory asyncRelayCommandFactory;
public MyViewModel(IAsyncRelayCommandFactory asyncRelayCommandFactory)
{
    this.asyncRelayCommandFactory = asyncRelayCommandFactory;
}
...
MyCommandNumber156 = asyncRelayCommandFactory.Create<MyClass>(async arg => await SomeMethodAsync(arg, param1));

[ShahinDohan]

As @virzak pointed out, this can really be a problem if a developer forgets to catch the exception, because then it just disappears and the user is left wondering what happened, and the developer has no logs to look at.

I really like the way Caliburn.Micro handles this with a custom global event handler:

Coroutine.Completed += (s, a) =>
{
    if (a.Error != null)
    {
        // Handle exception here
    }
};

As I mentioned before I had a similar issue like this in Caliburn.Micro, but they provided a way to catch the exceptions globally using the Coroutine.Completed event handler.

Could we have something similar in MVVM Toolkit? I think this really needs to be addressed as it's quite a big "gotcha" with potentially huge consequences. Preferably the exception would be thrown, because developers can forget to subscribe to the custom event handler as well, but if there are technical limitations then the custom event handler solution seems to be the next best thing.

@Sergio0694 Thoughts?

Thanks!

[jhm-ciberman]

Hello. I just started using Windows Comunity Toolkit MVVM and I don't really know what would be the best approach to handle exceptions inside async commands. I also expect the exception to just be thrown. Any idea?

I tried to extend to AsyncRelayCommand class to automatically add the event handler to every command I create, but the class is sealed, so I can't extend it 😕

[ShahinDohan]

The easiest solution would be to simply use the normal RelayCommand and mark the handler method as async void then any exceptions would be caught by the dispatcher. This is how event handlers work anyway.

Now that I think about it, I actually don't even know why I would want to use AsyncRelayCommand unless I needed to monitor the commands IsRunning or to cancel the command, but even then that can all be done manually with boolean flags and CancellationTokenSource.

It's possible to also throw the exception on the Dispatcher manually, and it would eventually be caught by TaskScheduler.UnobservedTaskException, but it's a very ugly hack and should never ever be done:
_ = Dispatcher.CurrentDispatcher.InvokeAsync(() => throw exception);

I prefer the async void method, similar to other events in WPF. Just set a flag to disable the originating control in the UI while the task is running, then clear the flag when it's done.

I'm actually curious as to what is the use case for AsyncRelayCommand? Is there some UI platform that is able to await tasks? Forgive my ignorance I only know WPF 🙂

[jhm-ciberman]

Yes... I think I will end up doing that. I feel very insecure when exceptions are being swallowed. It's a pity because sometimes I use the ´IsRunning´ property. But as you said, I can set a flag manually. Thank you!

[virzak]

@Sergio0694, looks like there is no way to use [ICommand] source generator without swallowing the exception at this point. Are you considering a global error handling mechanism?

[Sergio0694]

Hi @virzak, yeah investigating support for exception handling in commands (both on their own and through source generators) is definitely on my ToDo list. I have lots of things planned for the MVVM Toolkit and for the .NET Community Toolkit as a whole, it's just been somewhat of a slower couple of months on this front due to some vacation time and also me being about to get started at Microsoft. I hope to be able to ramp things up with work on the Toolkit again in the next couple of months, and this is definitely one of the things I will be looking into 😄

[virzak]

@Sergio0694 thanks for the efforts. Loving your various libraries and good luck at Microsoft, you'll kick some serious ass there!!!

[andreinitescu]

Implementing an error handling mechanism is ideal but I think more problematic is the way the Task version is being called, which has different implications (exception handling, unobserved exceptions, etc.).

I noticed that in the implementation of the AsyncRelayCommand, the synchronous void version of ICommand.Execute calls the asynchronous Task version:

public sealed class AsyncRelayCommand : IAsyncRelayCommand
{
  
    public void Execute(object? parameter)
    {
        _ = ExecuteAsync(parameter);
    }

    public Task ExecuteAsync(object? parameter)
    {
          ...
    }
    
    ...
}

https://github.com/CommunityToolkit/dotnet/blob/main/CommunityToolkit.Mvvm/Input/AsyncRelayCommand.cs#L265

Isn't this (very) problematic?

[Sergio0694]

Why do you think it is (very) problematic?

[SmRiley]

I would like to know if there is any progress so far or a more appropriate way to deal with it?

[Sergio0694]

Given the first post mentions TaskScheduler.UnobservedException, I'll mention that as of #113, this is now resolved, and all exceptions being thrown from async commands will correctly bubble up to that global event if not handled before.