jbogard / MediatR.Extensions.Microsoft.DependencyInjection

MediatR extensions for Microsoft.Extensions.DependencyInjection
MIT License
326 stars 90 forks source link

Order of registration regarding RequestExceptionActionProcessorBehavior and RequestExceptionProcessorBehavior #80

Closed Expecho closed 2 years ago

Expecho commented 4 years ago

Issue According to the documentation there is a difference in behavior regarding the handling of exceptions using a combination of RequestExceptionActionProcessorBehavior and RequestExceptionProcessorBehavior:

Exception action implemented by using IPipelineBehavior concept. It requires to add the RequestExceptionActionProcessorBehavior to the request execution Pipeline. If place RequestExceptionActionProcessorBehavior before RequestExceptionProcessorBehavior, actions will be called only for unhandled exceptions.

The code in this repo currently registers the RequestExceptionActionProcessorBehavior before the RequestExceptionProcessorBehavior (source)

I would like to be able to influence this order.

Proposal Extend the MediatRServiceConfiguration class with an option to influence the order. Since the configuration is already passed to the method registering the behaviors I think we can easily facilitate this proposal without introducing breaking changes and preserving the current order as the default, unless overriden by using an overload of the AddMediatR extension method that accepts an Action<MediatRServiceConfiguration> configuration. See the source.

jbogard commented 4 years ago

Is the order incorrect for these behaviors? Should they be reversed?

Expecho commented 4 years ago

Is the order incorrect for these behaviors? Should they be reversed?

There is no such thing as a(n) (in)correct order in this case. My issue is about the ordering being fixed. Given the current (fixed) ordering I can only create an Exception Action that deals with unhandled exceptions. I want to process all exceptions using an Action but that can only happen if the order is reversed.

The correct ordering therefore depends on the use case. My PR lets a developer change the order and therefore allows to handle all exceptions by setting RequestExceptionActionProcessorStrategy to ApplyForAllExceptions. By letting the RequestExceptionActionProcessorStrategy have the default set to ApplyForUnhandledExceptions the current default order is preserved therefore avoiding different out-of-the-box behavior and so there won't be any breaking changes