dotnet icon indicating copy to clipboard operation
dotnet copied to clipboard

[Feature] AsyncRelayCommand.Execute should catch and raise an Exception if possible

Open juwens opened this issue 3 years ago • 7 comments

It easily happens that you use the AsyncRelayCommand somewhere as ICommand, if i call the ICommand.Execute i get no Exception, even if the async Method throws an Exception.

I know it's nearly impossible (only via AsyncEx.Run), to synchronous call an async method wait for it and raise the Exception in sync context.

Alternative:

AsyncRelayCommand should have a public static Event where i can register an EventHandler for these kind of Exceptions.

So Execute would look like this:

public async void Execute()
{
    try
    {
        await ExecuteAsync()
    }
    catch(Exception e)
    {
        ExceptionInSyncExecute?.Invoke(this, new UnhandledExceptionEventArgs(e));
    }
}

juwens avatar May 31 '21 21:05 juwens

Hello, 'juwens! Thanks for submitting a new feature request. I've automatically added a vote 👍 reaction to help get things started. Other community members can vote to help us prioritize this feature in the future!

ghost avatar May 31 '21 21:05 ghost

@juwens this seems similar to the discussion happening here maybe?

FYI @Sergio0694

michael-hawker avatar Jun 01 '21 16:06 michael-hawker

This seems to keep being raised in multiple places (including the conversation at https://github.com/windows-toolkit/WindowsCommunityToolkit/discussions/3961), so I think it can be worth looking into at least to get a feel for what it would be required to add something like a static event for command exceptions. I'm not a fan of the ugly workarounds users are doing right now to detect crashes from property changes, not to mention the fact that the way exceptions are retrieved that way is rather clunky. @michael-hawker I can see if I can put together a prototype if you agree 😄

Sergio0694 avatar Jun 01 '21 21:06 Sergio0694

@Sergio0694 yeah, I mean there can be general problems with tracking async exceptions in general too (even outside of commands).

I know there was some helper library you could tack on calls to async calls to help better log stacktraces across source and execution, but I don't remember what it's called.

Is this something that a proposal in .NET or WinUI itself could help us solve too? Like just want to make sure we're addressing the root of the issue vs. just being another library having a solution to work around some fundamental problem.

Thoughts?

michael-hawker avatar Jun 01 '21 22:06 michael-hawker

Thanks for pointing out the discussion #3961, i didn't notice it in advance. You are right, my feature-request seems to be covered by the discussion.

juwens avatar Jun 02 '21 13:06 juwens

We are using IAsyncCommand from Xamarin Community Toolkit, which has a parameter for onException action. It basically call the action whenever an exception occurred, which is very handy.

We are hoping to use CommunityToolkit.Mvvm.Input.ICommandAttribute instead to reduce our coding effort. Would be nice if it also support onException action too.

Suggest usage would be

[ICommand(onException = SomeAction)]
private async Task MyCommand() {
    // code
}

timdinhdotcom avatar Mar 11 '22 02:03 timdinhdotcom

🦙 Related to discussion #175 as well (brought up again from recent PR #251)

michael-hawker avatar May 12 '22 20:05 michael-hawker

Closing this, as exceptions can now be configured to be rethrown, via the new option added in the PR linked above.

Sergio0694 avatar Dec 18 '22 02:12 Sergio0694