Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Handlers for interfaced notifications are not working as expected #1050

Open
Kjelli opened this issue Jul 17, 2024 · 7 comments
Open

Handlers for interfaced notifications are not working as expected #1050

Kjelli opened this issue Jul 17, 2024 · 7 comments
Labels

Comments

@Kjelli
Copy link

Kjelli commented Jul 17, 2024

The issue at hand

When using interfaced notifications, and handlers for the interfaces - the handler does not run unless there exists a handler targetting a concrete implementation of the respective interface.

Steps to reproduce:

Bootstrap

HostApplicationBuilder builder = Host.CreateApplicationBuilder(args);
builder.Services.AddMediatR(c => c.RegisterServicesFromAssembly(typeof(Program).Assembly));
using IHost host = builder.Build();

var mediator = host.Services.GetRequiredService<IMediator>();

A) A simple notification handler that runs

// Simple case, nothing new

record SimpleGreeterNotification(string Message) : INotification;
class SimpleHandler : INotificationHandler<SimpleGreeterNotification>
{
    public Task Handle(SimpleGreeterNotification notification, CancellationToken cancellationToken)
    {
        Console.WriteLine(notification.Message);
        return Task.CompletedTask;
    }
}

// ...

await mediator.Publish(new SimpleGreeterNotification("Hello")); 
// Outputs "Hello", no surprises here

B) An interfaced notification handler that runs

// Interfaced case

public interface IGreeterInterface : INotification;
record ImplementingGreeterNotification(string Message) : IGreeterInterface;

class InterfacedGreeterHandler : INotificationHandler<IGreeterInterface>
{
    public Task Handle(IGreeterInterface notification, CancellationToken cancellationToken)
    {
        Console.WriteLine("I only run when ImplementingGreeterHandler exists");
        return Task.CompletedTask;
    }
}

class ImplementingGreeterHandler : INotificationHandler<ImplementingGreeterNotification>
{
    public Task Handle(ImplementingGreeterNotification notification, CancellationToken cancellationToken)
    {
        Console.WriteLine(notification.Message);
        return Task.CompletedTask;
    }
}

// ...

await mediator.Publish(new ImplementingGreeterNotification("Hello interface"));
// Outputs "I only run when ImplementingGreeterHandler exists"
// Outputs "Hello interface

C) Now for the interfaced notification handler that does not run

// Interfaced case

public interface IGreeterInterface : INotification;
record ImplementingGreeterNotification(string Message) : IGreeterInterface;

class InterfacedGreeterHandler : INotificationHandler<IGreeterInterface>
{
    public Task Handle(IGreeterInterface notification, CancellationToken cancellationToken)
    {
        Console.WriteLine("I only run when ImplementingGreeterHandler exists");
        return Task.CompletedTask;
    }
}

// ...

await mediator.Publish(new ImplementingGreeterNotification("Hello interface"));
// No output

Cases A and B are as expected, whereas case C is strange - I would expect InterfacedGreeterHandler to be run.

Further notes

I am not sure any of this is relevant, but I did some investigating.

I have a theory that this issue is caused near NotificationHandlerWrapper. Inspecting the service collection, I verify that it correctly contains INotificationHandler<IGreeterInterface>, but it does not contain INotificationHandler<ImplementingGreeterNotification>.

From these lines I see that IServiceProvider.GetServices<INotificationHandler<ImplementingGreeterNotification>> does not return anything, which I suspect subsequently fails to resolve a respective NotificationHandlerExecutor.

var handlers = serviceFactory
.GetServices<INotificationHandler<TNotification>>()
.Select(static x => new NotificationHandlerExecutor(x, (theNotification, theToken) => x.Handle((TNotification)theNotification, theToken)));

It does make sense I suppose, but at face value this looks like odd behavior either way.

I tested a more basic, yet potentially related case; implementing a catch-all INotificationHandler<INotification> never runs if there are no other INotificationHandler-types registered.

@Kjelli Kjelli changed the title Handlers for interfaced notifications are not registered Handlers for interfaced notifications are not working as expected Jul 17, 2024
Copy link

This issue is stale because it has been open 60 days with no activity. Remove stale label or comment or this will be closed in 14 days.

@github-actions github-actions bot added the Stale label Sep 15, 2024
@Kjelli
Copy link
Author

Kjelli commented Sep 23, 2024

bump

@github-actions github-actions bot removed the Stale label Sep 23, 2024
@richardpauly
Copy link

I seem to have the same problem in my project

@Robert-LTH
Copy link

I stumpled upon this issue as well but solved it by copying the definition of the handler from https://github.com/jbogard/MediatR.Extensions.Microsoft.DependencyInjection/blob/020d8817ae84e18d61bbf343b00558249c37f899/src/TestApp/ConstrainedPingedHandler.cs#L8

@inkvizytor
Copy link

I stumpled upon this issue as well but solved it by copying the definition of the handler from https://github.com/jbogard/MediatR.Extensions.Microsoft.DependencyInjection/blob/020d8817ae84e18d61bbf343b00558249c37f899/src/TestApp/ConstrainedPingedHandler.cs#L8

It does not solve case C.

@adam-dot-cohen
Copy link

It's been over a year since I last wrestled with notifications. I threw in the towel and pursued an alternative approach described with pseudo code in this thread last year. Drastically simplified notifications and cloud events and made execution order and debugging possible. Unfortunately, it seems the saga continues and I no longer have access to the code base. In case it helps anyone, the suggested alternative at the bottom of this thread worked like a charm...

How to change MediatR publish strategy

Copy link

This issue is stale because it has been open 60 days with no activity. Remove stale label or comment or this will be closed in 14 days.

@github-actions github-actions bot added the Stale label Dec 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

5 participants