Generic NotificationHandler not called

[fretje]

Not sure if this is a bug, or something I'm doing wrong...

Also not sure how to explain it properly... I think the easiest will be to just show the code.

This is a basic full web project (yay for minimal apis) with everything functioning as intended:

using System.Diagnostics;
using MediatR;

WebApplicationBuilder? builder = WebApplication.CreateBuilder(args);

builder.Services.AddMediatR(typeof(Program));

WebApplication? app = builder.Build();

app.MapGet("/", (IPublisher publisher) =>
{
    publisher.Publish(new EventNotification<SomethingHappenedEvent>(new SomethingHappenedEvent()));
    return "Event published";
});

app.Run();

public abstract class DomainEvent
{
}

public class SomethingHappenedEvent : DomainEvent
{
}

public class EventNotification<TDomainEvent> : INotification
    where TDomainEvent : DomainEvent
{
    public EventNotification(TDomainEvent domainEvent) => DomainEvent = domainEvent;

    public TDomainEvent DomainEvent { get; }
}

public class SomethingHappenedEventHandler : INotificationHandler<EventNotification<SomethingHappenedEvent>>
{
    public Task Handle(EventNotification<SomethingHappenedEvent> notification, CancellationToken cancellationToken)
    {
        Debug.WriteLine("something happened handled");
        return Task.CompletedTask;
    }
}

When you go to the homepage, an event is published and handled by the SomethingHappendEventHandler. (something happened handled is printed in the debug output).

Now I want to add another notificationhandler that handles all notifications:

public class GenericEventHandler<TNotification> : INotificationHandler<TNotification>
    where TNotification : INotification
{
    public Task Handle(TNotification notification, CancellationToken cancellationToken)
    {
        Debug.WriteLine("generic handled");
        return Task.CompletedTask;
    }
}

Now when I go to the homepage, the SomethingHappenedEventHandler is called twice (!) and the GenericEventHandler isn't called at all. (something happened handled is printed twice in the debug output, no generic handled is printed)

I'm really confused as to how this happens or where to go from here...

The problem might be with how the DomainEvents are designed... but this is what I have to work with for now...

When I create a simple notification (not derived from EventNotification):

public class SimpleNotification : INotification
{
}

And publish that, the generic handler does get called.

Really strange...

[jbogard]

You might need to register the open generic explicitly.

[fretje]

Hmm, I thought open generics were handled nowadays.

Still doesn't explain why the other handler is called twice though.

When I add this before builder.Build:

builder.Services.AddTransient<INotificationHandler<EventNotification<SomethingHappenedEvent>>, GenericEventHandler<EventNotification<SomethingHappenedEvent>>>();

The generic handler does indeed get called... but the other one is still called twice!

[jbogard]

This will come down to the features of the container I think. Check to see what's registered in the container. You might have to switch containers, the default one isn't too great with the more complex generic scenarios.

[fretje]

These are all the services related to MediatR that are registered:

Service: MediatR.ServiceFactory
Lifetime: Transient
Instance: 

Service: MediatR.IMediator
Lifetime: Transient
Instance: MediatR.Mediator

Service: MediatR.ISender
Lifetime: Transient
Instance: 

Service: MediatR.IPublisher
Lifetime: Transient
Instance: 

Service: MediatR.IPipelineBehavior`2
Lifetime: Transient
Instance: MediatR.Pipeline.RequestPreProcessorBehavior`2

Service: MediatR.IPipelineBehavior`2
Lifetime: Transient
Instance: MediatR.Pipeline.RequestPostProcessorBehavior`2

Service: MediatR.IPipelineBehavior`2
Lifetime: Transient
Instance: MediatR.Pipeline.RequestExceptionActionProcessorBehavior`2

Service: MediatR.IPipelineBehavior`2
Lifetime: Transient
Instance: MediatR.Pipeline.RequestExceptionProcessorBehavior`2

Service: MediatR.INotificationHandler`1[[EventNotification`1[[SomethingHappenedEvent, MediatRWebApplication, Version=1.0.0.0, Culture=neutral, PublicKeyToken=null]], MediatRWebApplication, Version=1.0.0.0, Culture=neutral, PublicKeyToken=null]]
Lifetime: Transient
Instance: SomethingHappenedEventHandler

Service: MediatR.INotificationHandler`1
Lifetime: Transient
Instance: GenericEventHandler`1

Service: MediatR.INotificationHandler`1[[EventNotification`1[[SomethingHappenedEvent, MediatRWebApplication, Version=1.0.0.0, Culture=neutral, PublicKeyToken=null]], MediatRWebApplication, Version=1.0.0.0, Culture=neutral, PublicKeyToken=null]]
Lifetime: Transient
Instance: GenericEventHandler`1[[EventNotification`1[[SomethingHappenedEvent, MediatRWebApplication, Version=1.0.0.0, Culture=neutral, PublicKeyToken=null]], MediatRWebApplication, Version=1.0.0.0, Culture=neutral, PublicKeyToken=null]]

The last one is added by the manual registration (from my previous comment), but I think the culprit is

Service: MediatR.INotificationHandler`1
Lifetime: Transient
Instance: GenericEventHandler`1

Which somehow manages to call the one above it instead?

[dandohotaru]

you might want to have the open generics defined explicitly.
had the very same behavior the other day and fixed it using this approach
jbogard/MediatR#702 (comment)

[fretje]

This is exactly the same issue I'm experiencing here like you are... but I'm developing on windows, and I have it on my machine... so I don't think the os has anything to do with it (at least in my case). It has indeed something to do with how the handlers are resolved from DI...

I'm not sure MediatR can do anything about it, but it's very confusing when you run into it to say the least...

I don't think my case is very special, as it can be minimized to the code you see above. I would think that would have to work out of the box, or otherwise throw some meaningful exception that there's something wrong with the registration?

Just thinking out loud here... not sure if it's possible or feasible... but when you're resolving the handlers to be executed couldn't you compare them against each other first to see if they're not the same and throw an exception if they are?

[jbogard]

This test passes for me:

[Fact]
public void Should_resolve_handlers()
{
    var services = new ServiceCollection();
    services.AddMediatR(typeof(HandlerResolutionTests));

    var serviceProvider = services.BuildServiceProvider();

    var handlers = serviceProvider
        .GetServices<INotificationHandler<EventNotification<SomethingHappenedEvent>>>()
        .ToList();

    handlers.Count.ShouldBe(1);
    handlers.Select(handler => handler.GetType())
        .ShouldContain(typeof(SomethingHappenedEventHandler));
}

In .NET 6 on Windows.

[fretje]

It doesn't if you define a GenericEventHandler like this in the same assembly:

public class GenericEventHandler<TNotification> : INotificationHandler<TNotification>
    where TNotification : INotification
{
    public Task Handle(TNotification notification, CancellationToken cancellationToken)
    {
        Debug.WriteLine("generic handled");
        return Task.CompletedTask;
    }
}

I mean that's probably normal in how the test is set up... but my point is that I don't have a problem as long as I don't define that Generic handler...

And apparently it's only in an aspnetcore context? I can't seem to repeat the problem in a test context in the way you set up your test above (when I do a publish there, both handlers are called correctly like they should).

[fretje]

This is the best I could do to set up tests that fail in this situation:

https://github.com/fretje/MediatRHandlerCountTest/blob/master/TestProject1/UnitTest1.cs

[jbogard]

So I converted these to unit tests just around the container (to remove the test host and actions from the equation, and the tests all pass:

[Fact]
public async Task TestSomethingHappened()
{
    var services = new ServiceCollection();
    services.AddMediatR(typeof(HandlerResolutionTests));
    var counts = new HandlerCounter();
    services.AddSingleton(counts);

    var serviceProvider = services.BuildServiceProvider();

    var mediator = serviceProvider.GetRequiredService<IMediator>();

    await mediator.Publish(new EventNotification<SomethingHappenedEvent>(new SomethingHappenedEvent()));

    counts.ShouldNotBeNull();
    counts.SomethingHappenedCount.ShouldBe(1);
    counts.SimpleCount.ShouldBe(0);
    counts.GenericCount.ShouldBe(1);
}

[Fact]
public async Task TestSimple()
{
    var services = new ServiceCollection();
    services.AddMediatR(typeof(HandlerResolutionTests));
    var counts = new HandlerCounter();
    services.AddSingleton(counts);

    var serviceProvider = services.BuildServiceProvider();

    var mediator = serviceProvider.GetRequiredService<IMediator>();

    await mediator.Publish(new SimpleNotification());

    counts.ShouldNotBeNull();
    counts.SomethingHappenedCount.ShouldBe(0);
    counts.SimpleCount.ShouldBe(1);
    counts.GenericCount.ShouldBe(1);
}

[Fact]
public async Task TestBoth()
{
    var services = new ServiceCollection();
    services.AddMediatR(typeof(HandlerResolutionTests));
    var counts = new HandlerCounter();
    services.AddSingleton(counts);

    var serviceProvider = services.BuildServiceProvider();

    var mediator = serviceProvider.GetRequiredService<IMediator>();

    await mediator.Publish(new EventNotification<SomethingHappenedEvent>(new SomethingHappenedEvent()));
    await mediator.Publish(new SimpleNotification());

    counts.ShouldNotBeNull();
    counts.SomethingHappenedCount.ShouldBe(1);
    counts.SimpleCount.ShouldBe(1);
    counts.GenericCount.ShouldBe(2);
}

It looks like your code doesn't await the Publish calls. That will cause all sorts of weirdness. Try adding the async/await on those actions.

[fretje]

Async await is not the problem. I was really hoping to, but I just forgot it in the example code (my original code has the await).

I fixed that in the example code... same problem still exists.

The problem is only when run from an action method. Can't repeat it by only using the serviceProvider.

Could it be a bug in aspnetcore maybe?

[jbogard]

Well nuts. When I debug and examine the builder.Services right after registration, I only see the registrations once:

Looking at the return values from the container in the test:

var services = application.Services
    .GetServices<INotificationHandler<EventNotification<SomethingHappenedEvent>>>()
    .ToList();

[fretje]

Yep, strange indeed... it should actually give you 2 handlers, but one of them should be that GenericEventHandler instead of 2 SomethingHappenedEventHandlers...

[jbogard]

This is even odder, building the service provider right after registration and resolving there:

WebApplicationBuilder? builder = WebApplication.CreateBuilder(args);

var singleton = new HandlerCounter();

builder.Services.AddSingleton(singleton);

builder.Services.AddMediatR(typeof(Program));

var serviceProvider = builder.Services.BuildServiceProvider();
var handlers = serviceProvider.GetServices<INotificationHandler<EventNotification<SomethingHappenedEvent>>>();

Gives me:

[fretje]

Yep, what I was trying to say the whole time :-) only in the context of a request...

[jbogard]

Then after calling Build:

WebApplication? app = builder.Build();

var appHandlers = app.Services.GetServices<INotificationHandler<EventNotification<SomethingHappenedEvent>>>();

Gives:

It's not even the request context, the Build() method is messing with something here.

[fretje]

Oh yeah, was just trying to find that out as wel... guess it is a bug in aspnetcore then?

[jbogard]

Yeah it's bizarre, I can't repro that in a unit test. This passes:

[Fact]
public void Should_register_correctly()
{
    var builder = WebApplication.CreateBuilder();

    var singleton = new HandlerCounter();

    builder.Services.AddSingleton(singleton);

    builder.Services.AddMediatR(typeof(Program));

    var serviceProvider = builder.Services.BuildServiceProvider();
    var handlers = serviceProvider
        .GetServices<INotificationHandler<EventNotification<SomethingHappenedEvent>>>()
        .ToList();

    handlers.Count.ShouldBe(2);
    var handlersTypes = handlers
        .Select(h => h.GetType())
        .ToList();
    handlersTypes.ShouldContain(typeof(SomethingHappenedEventHandler));
    handlersTypes.ShouldContain(typeof(GenericEventHandler<EventNotification<SomethingHappenedEvent>>));
    
    var app = builder.Build();

    var appHandlers = app.Services
        .GetServices<INotificationHandler<EventNotification<SomethingHappenedEvent>>>()
        .ToList();

    appHandlers.Count.ShouldBe(2);
    var appHandlersTypes = appHandlers
        .Select(h => h.GetType())
        .ToList();
    appHandlersTypes.ShouldContain(typeof(SomethingHappenedEventHandler));
    appHandlersTypes.ShouldContain(typeof(GenericEventHandler<EventNotification<SomethingHappenedEvent>>));
}

[fretje]

So maybe kestrel messes it up somehow?

[jbogard]

No, Kestrel isn't run from the unit tests. I got it to fail though, it's args that are different when you run from VS. The Development environment makes the test fail:

[Fact]
public void Should_register_correctly()
{
    var args = new[]
    {
        "--environment=Development"
    };
    var builder = WebApplication.CreateBuilder(args);

    var singleton = new HandlerCounter();

    builder.Services.AddSingleton(singleton);

    builder.Services.AddMediatR(typeof(Program));

    var serviceProvider = builder.Services.BuildServiceProvider();
    var handlers = serviceProvider
        .GetServices<INotificationHandler<EventNotification<SomethingHappenedEvent>>>()
        .ToList();

    handlers.Count.ShouldBe(2);
    var handlersTypes = handlers
        .Select(h => h.GetType())
        .ToList();
    handlersTypes.ShouldContain(typeof(SomethingHappenedEventHandler));
    handlersTypes.ShouldContain(typeof(GenericEventHandler<EventNotification<SomethingHappenedEvent>>));
    
    var app = builder.Build();

    var appHandlers = app.Services
        .GetServices<INotificationHandler<EventNotification<SomethingHappenedEvent>>>()
        .ToList();

    appHandlers.Count.ShouldBe(2);
    var appHandlersTypes = appHandlers
        .Select(h => h.GetType())
        .ToList();
    appHandlersTypes.ShouldContain(typeof(SomethingHappenedEventHandler));
    appHandlersTypes.ShouldContain(typeof(GenericEventHandler<EventNotification<SomethingHappenedEvent>>));
}

This looks like a bug.

[fretje]

Ahaaa, nice catch!

Now how to report this... or will you do the honours? ;-)

[jbogard]

I've got a repro that completely removes MediatR from the equation, I'll open an issue with those tests.

[fretje]

Nice! Thanks a lot! Please post a link... I'd like to follow ;-)

[jbogard]

Sure thing! dotnet/runtime#65145

[fretje]

Nice, thanks again.

Spotted a mistake I think in your last part... It says

The first set of assertions fails when I build...

Should probably be

The first set of assertions succeeds when I build...

[jbogard]

Lol good catch!

[Isitar]

Is there any known workaround?
I really would like to add a NotificationHandler for all update events and the project should go live by the end of the month...

[Isitar]

I think I found a Solution that works for me and I'll leave it here in case someone else is in a similar situation

You can extend this solution by iterating over all INotificationHandler in your assemblies, but in my project this is enough

Events:

public record BlamableCreatedEvent(BlamableEntity Blamable) : INotification;
public record UserCreatedEvent(User User) : BlamableCreatedEvent(User);

NotificationHandler:

public class UpdateBlamableFieldsWhenBlamableCreated : INotificationHandler<BlamableCreatedEvent>
{
  // Code to update blamable fields
}

Configure Services:

services.AddMediatR(applicationAssembly);
foreach (var assembly in new[] { applicationAssembly, domainAssembly }) // add all your assemblies here
{
    foreach (var blamableCreatedEvent in assembly
        .DefinedTypes
        .Where(dt => !dt.IsAbstract && dt.IsSubclassOf(typeof(BlamableCreatedEvent)))
    )
    {
        services.AddTransient(typeof(INotificationHandler<>).MakeGenericType(blamableCreatedEvent), typeof(UpdateBlamableFieldsWhenBlamableCreated));
    }
}
            ```

[KuraiAndras]

@jbogard I found some problems in MediatR.Courier KuraiAndras/MediatR.Courier#10, and I am unsure whether I am affected by this bug?
(No other types in the project)

using Microsoft.Extensions.DependencyInjection;

namespace MediatR.Courier.GenericTests;

public class GenericTest
{
    public record GenericMessage<T>(T Data) : INotification;


    public class GenericHandler<T> : INotificationHandler<GenericMessage<T>>
    {
        public Task Handle(GenericMessage<T> notification, CancellationToken cancellationToken) => Task.CompletedTask;
    }

    [Fact]
    public void This_Should_Not_Throw()
    {
        var services = new ServiceCollection()
            .AddMediatR(typeof(GenericTest).Assembly);

        var serviceProvider = services.BuildServiceProvider();

        var mediator = serviceProvider.GetRequiredService<IMediator>();

        mediator.Publish(new GenericMessage<string>("Hello"));
    }

    [Fact]
    public void This_Should_Not_Throw_Also()
    {
        var services = new ServiceCollection()
            .AddMediatR(typeof(GenericTest).Assembly)
            .AddTransient(typeof(INotificationHandler<>), typeof(GenericHandler<>));

        var serviceProvider = services.BuildServiceProvider();

        var mediator = serviceProvider.GetRequiredService<IMediator>();

        mediator.Publish(new GenericMessage<string>("Hello"));
    }
}

Both of these tests throw the following:

  Message: 
System.ArgumentException : Implementation type 'MediatR.Courier.GenericTests.GenericTest+GenericHandler`1[MediatR.Courier.GenericTests.GenericTest+GenericMessage`1[System.String]]' can't be converted to service type 'MediatR.INotificationHandler`1[MediatR.Courier.GenericTests.GenericTest+GenericMessage`1[System.String]]'

  Stack Trace: 
ConstructorCallSite.ctor(ResultCache cache, Type serviceType, ConstructorInfo constructorInfo, ServiceCallSite[] parameterCallSites)
CallSiteFactory.CreateConstructorCallSite(ResultCache lifetime, Type serviceType, Type implementationType, CallSiteChain callSiteChain)
CallSiteFactory.TryCreateOpenGeneric(ServiceDescriptor descriptor, Type serviceType, CallSiteChain callSiteChain, Int32 slot, Boolean throwOnConstraintViolation)
CallSiteFactory.TryCreateEnumerable(Type serviceType, CallSiteChain callSiteChain)
CallSiteFactory.CreateCallSite(Type serviceType, CallSiteChain callSiteChain)
CallSiteFactory.GetCallSite(Type serviceType, CallSiteChain callSiteChain)
ServiceProvider.CreateServiceAccessor(Type serviceType)
ConcurrentDictionary`2.GetOrAdd(TKey key, Func`2 valueFactory)
ServiceProvider.GetService(Type serviceType, ServiceProviderEngineScope serviceProviderEngineScope)
ServiceProviderEngineScope.GetService(Type serviceType)
ServiceProviderServiceExtensions.GetRequiredService(IServiceProvider provider, Type serviceType)
ServiceFactoryExtensions.GetInstances[T](ServiceFactory factory)
NotificationHandlerWrapperImpl`1.Handle(INotification notification, CancellationToken cancellationToken, ServiceFactory serviceFactory, Func`4 publish)
Mediator.PublishNotification(INotification notification, CancellationToken cancellationToken)
Mediator.Publish[TNotification](TNotification notification, CancellationToken cancellationToken)
GenericTest.This_Should_Not_Throw_Also() line 39

Is this: the same bug/I am doing something wrong/unsupported behavior?

[Soundman32]

I think I found a Solution that works for me and I'll leave it here in case someone else is in a similar situation

You can extend this solution by iterating over all INotificationHandler in your assemblies, but in my project this is enough

Events:

public record BlamableCreatedEvent(BlamableEntity Blamable) : INotification;
public record UserCreatedEvent(User User) : BlamableCreatedEvent(User);

NotificationHandler:

public class UpdateBlamableFieldsWhenBlamableCreated : INotificationHandler<BlamableCreatedEvent>
{
  // Code to update blamable fields
}

Configure Services:

services.AddMediatR(applicationAssembly);
foreach (var assembly in new[] { applicationAssembly, domainAssembly }) // add all your assemblies here
{
    foreach (var blamableCreatedEvent in assembly
        .DefinedTypes
        .Where(dt => !dt.IsAbstract && dt.IsSubclassOf(typeof(BlamableCreatedEvent)))
    )
    {
        services.AddTransient(typeof(INotificationHandler<>).MakeGenericType(blamableCreatedEvent), typeof(UpdateBlamableFieldsWhenBlamableCreated));
    }
}
            ```

This doesn't work for me, as it's identical to the way I already register the generic handlers.

[afederici75]

The last solution didn't work for me either: tests are fine with a brand new ServiceCollection, but at runtime it gets confused. The only only suspicious difference between runtime and test is an ObjectPool being transient in one and singleton in the other but it shouldn't affect any of this stuff.

In the end I replaced the mediator implementation with a custom one (ReMediator :) ) where I overrode PublishCore and made sure each MethodInfo is only called once. Is this an ok solution for now?

public class ReMediator : Mediator
{
    public ReMediator(ServiceFactory serviceFactory)
        : base(serviceFactory)
    { }

    protected override async Task PublishCore(IEnumerable<Func<INotification, CancellationToken, Task>> allHandlers, INotification notification, CancellationToken cancellationToken)
    {
        HashSet<Type> doneMethods = new ();
        foreach (var handler in allHandlers)
        {
            var handlerType = handler.GetMethodInfo().DeclaringType;
            if (!doneMethods.Add(handlerType))
                continue;

            await handler(notification, cancellationToken).ConfigureAwait(false);
        }
    }
}

I then registered the new Mediator this way:

 services.AddMediatR(asmList, options => 
        {
            // Unfortunately Publish() seems to hit the handlers twice!!!
            // https://github.com/jbogard/MediatR/issues/702
            // https://github.com/jbogard/MediatR/issues/718

            options.Using<ReMediator>();
        });