PrismLibrary / Prism

Prism is a framework for building loosely coupled, maintainable, and testable XAML applications in WPF, Xamarin Forms, and Uno / Win UI Applications..
Other
6.35k stars 1.64k forks source link

[Enhancement] Global Command Exception Handler #1902

Closed aritchie closed 5 years ago

aritchie commented 5 years ago

Summary

Provide a centralized global command exception handler that sits over top of all commands. If the command has an exception for any reason that isn't trapped, it will fall back to the global handler. This removes the usually boilerplate.

NOTE: this is employed by RXUI. You can see a sample of how I use it here: https://github.com/shinyorg/shinysamples/blob/master/Samples/Infrastructure/GlobalExceptionHandler.cs

API Changes

Additions only

containerRegistry.AddGlobalCommandExceptionHandler<TYourType>();

public interface IPrismCommandExceptionHandler {
    Task OnError(Exception exception);  // async because isn't everything :P
}

This would allow for DI'ing logging, maybe a dialog service, etc to manage the standard errors. We could even return false to say (error wasn't managed - implode?).

Intended Use Case

Whenever an error occurs in a command, you generally end up doing the boilerplate try/catch that logs the error and probably displays an alert saying "ya dude - sorry we suck". Now you can centralize the boilerplate. Note that the global command exception handler is a FALLBACK. You can still do the boilerplate where applicable.

Also, if you accept this - I'll do the work involved. DAN - THIS MEANS YOU!!

Brian - I already convinced Dan. We need you to bless this 💯

brianlagunas commented 5 years ago

Nope, not gonna happen. The DelegateCommand is not going to take an DI dependency and this is not something that Prism should be responsible for . If you need this in your app, just derive from DelegateCommand and implement it there.

aritchie commented 5 years ago

Nope - not something I really need. Was just offering to add it to Prism if you wanted it. Thanks for the quick response.

brianlagunas commented 5 years ago

Keep in mind that DelegateCommand is used in more than just an XF app. It's used in non-DI apps on multiple platforms. Also, taking the hit of using DI in a command can get very perf expensive on plaforms like WPF where there can be MANY commands defined. Not to mention from an architecture perspective, that's giving the DelegateCommand class too many responsibilities. These types of things really should be handled by the developers app-level code.

aritchie commented 5 years ago

I somewhat agree with you that these should be the responsibility of the developer. My approach here is it is like an exception handler in asp.net controller. It is a top level. It removes the general try/catch/logging/alerts cruft of things.

You also don't have to use DI. In fact, RXUI uses a static singleton (RxApp.DefaultExceptionHandler) which I basically resolve 1 time at a startup to inject things like logging. From that point on, you don't take the hit.

brianlagunas commented 5 years ago

Beside the fact that I try to avoid singletons like the plague, even the singleton approach makes assumptions about the environment, structure, and architecture the command is running on. Prism doesn't try to solve all the domain problems an app has, but remain extensible to allow the dev to hook into it where necessary.

lock[bot] commented 4 years ago

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.