Is it OK to have one handler call another when using MediatR? [closed]
Asked Answered
T

2

19

Or is that considered bad practice or something?

I have one notification triggers 4-5 handlers, which in turn call database to retrieve data. Each those calls can also be called separately, so they are request/handler themselves.

Thanks.

Twitter answered 1/3, 2018 at 3:23 Comment(7)
Are you talking about this library?Adjunct
I have done this. It's ok, as long as you manage it. It's a quick way of getting in to a stack overflow exception if you're not careful, however. Keep the handlers small, and you'll be fineBiracial
@DannyChen Yes.Twitter
@Biracial Thanks. Overflow, you mean getting into circular calling? Now that I looked at it again, some of those sub calls are mostly internal and do not have a direct "request" from end user. Maybe writing them as "service" instead?Twitter
yeah, exactly that. AbcHandler calls XyzHandler which in turn calls AbcHandlerBiracial
it's hard to say without seeing your code, but I've found there's very little that can't be architected into MediatR commands / handlers - especially with notifications tooBiracial
Thanks. I'll just make a mental note that "service" should not call handler.Twitter
C
10

Jimmy Bogard (the author of mediatr) is saying you should avoid this.

See his blog post

https://lostechies.com/jimmybogard/2016/12/12/dealing-with-duplication-in-mediatr-handlers/

or this stack overflow question and answer https://github.com/jbogard/MediatR/issues/400

Also note that if you are using behaviors they will run multiple times. This could lead to inefficient code but of course it could also be what you want to happen :-)

Costplus answered 9/12, 2019 at 7:33 Comment(1)
While this may answer the question, consider the fact that links rot away. Therefore please consider summarising the content you link to.Stclair
C
5

I run into this answer while I was investigating a strange issue in a project that I was asked to work on.

The stated answer is correct, but the provided argumentation is rather philosophical (and long to read).

For future visitors, I present a less philosophical and more technical explanation as to why you should never ever call another handler within a handler (within Handle(...) method).

Summary

  • Yes, you can call other handlers from a handler.
  • You have to resolve a new dependency injection scope for such a call.
  • You must never pass any entity to a request.

TL;DR


You can call a MediatR handler in two ways:

  1. Directly var result = new SomeHandler(Dependency1, ...).Handle(new Request(...), ...);
  2. Via IMediator interface and the Send(...) method.

The 1st way is just tedious, fragile, and sensitive to refactoring, but still doable.

The 2nd will burn you immediately once you start with EF Core and navigation properties, or just once you start work with entities generally. Why?


IMediator is a service registered in DI as scoped. If you inject IMediator as a dependency into your handler, you will share a state with all other handlers invoked by this IMediator dependency instance, including the caller handler.

So... If the caller handler pulls out an entity from a repository, you call another handler within the Handle(...) that pulls out the same entity from the repository, you end up with two instances of the same entity in memory that are tracked by the EF Core.

An exception thrown immediately once you try to alter one of the instance and save the updated state to the repository.


Of course, you can bypass the ⬆️⬆️⬆️ problem by injecting IServiceScopeFactory into the caller handler, and do this

using var scope = _serviceScopeFactory.CreateScope();
var mediator = scope.ServiceProvider.GetRequiredService<IMediator>();
await mediator.Send(new SomeRequest { SomeId = Id }, cancellationToken);

thanks to which you effectively avoid the scope sharing problem, but you have strictly hand over only entity Ids, not the entities themselves.


record SomeRequest(Foo Foo, Bar Bar) : IRequest<SomeResult>; // ❌

record SomeRequest(Guid FooId, int BarId) : IRequest<SomeResult>; // ✅

and pull every single entity out from the repository again in each scope that you have created for your IMediator call. Why?


Once again, IMediator is a scoped service, same like DbContext (at least usually we register it as scoped).

Once you create a new scope for your IMediator call, a new DbContext is injected to all handlers in that scope. This also means that a new change tracker is created that is not aware of any created/pulled/altered/deleted entities in the caller handler scope (or vice versa).

For example, if you create an entity in the caller scope and propagate it as a property in some IRequest<T> implementation for another DB operation (such as a navigation property for another entity that needs to be created in the repository), you will (most likely) encounter a low-level DB error due to a primary key violation. This happens because the new change tracker will try to store the same entity with the same primary key that already exists in the repository (and it has been created in the caller scope where it still exists as an already created entity in the caller change tracker).

public class SomeHandler : IRequestHandler<SomeRequest, SomeEntity>
{
    private readonly IMediator _mediator;
    private readonly IRepository _repository;
    private readonly IServiceScopeFactory _serviceScopeFactory;

    public SomeHandler(IMediator mediator, IRepository repository, IServiceScopeFactory serviceScopeFactory)
    {
        _mediator = mediator;
        _repository = repository;
        _serviceScopeFactory = serviceScopeFactory;
    }

    public async Task<SomeEntity> Handle(SomeRequest request, CancellationToken cancellationToken)
    {
        // Direct call - not recommended
        // var result = new SomeOtherHandler(Dependency1, ...).Handle(new OtherRequest(...), ...);

        // Call via IMediator - problematic with shared state
        // var result = await _mediator.Send(new OtherRequest(...), cancellationToken);

        // Recommended way: create a new scope
        using var scope = _serviceScopeFactory.CreateScope();
        var mediator = scope.ServiceProvider.GetRequiredService<IMediator>();
        
        // pass only Ids to the handler
        var result = await mediator.Send(new OtherRequest(request.FooId, request.BarId), cancellationToken);

        // Pull out the entity again from the repository to avoid change tracker collisions
        var resultEntity = _repository.GetById(result.ResultId, cancellationToken);
        return resultEntity;
    }
}

public record SomeRequest(Guid FooId, int BarId) : IRequest<SomeResult>;
public record SomeResult(Guid ResultId);
public record SomeEntity(Guid Id);


If you think that this might solve your problems, be aware of potential deadlocks in your DB. Resolving multiple nested scopes could and probably will cause a performance issue for you DB, so eventually...

You will realize that you have to flatten your IMediator calls and you should avoid calling handlers within another handlers on the first place. 😉

Choanocyte answered 8/2 at 13:55 Comment(1)
Thank you for this amazing answer! Especially the "You must never pass any entity to a request." is not talked about enough and this explains some issues I have been coming acrossFriar

© 2022 - 2024 — McMap. All rights reserved.