jbogard / MediatR.Extensions.Microsoft.DependencyInjection

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

Migration from DependencyInjection 7.0 to 9.0 - Error constructing handlers #90

Open BH4NG opened 4 years ago

BH4NG commented 4 years ago

Hi All,

Been using MediatR since version 7, yesterday I migrated from 8.1 to 9.0.

Basically I had to inject IPublisher in DbContext, EventDispatcher and CommandHandlers that trigger Events and inject ISender in EventHandlers & Controllers?

I get following error: System.InvalidOperationException: Error constructing handler for request of type MediatR.IRequestHandler2, Register your handlers with the container. Cannot resolve 'MediatR.IRequestHandler2[]' from root provider because it requires scoped service 'xxxRepo'.

the operation continues, the request gets handled, the correct data is found, but my API-call returns 500StatusCode and subsequent calls to the same endpoint are not executed.

Had no issues with unregistered handlers of any kind prior to migrating to 9.0 In my startup I register Handlers first, then the repos, services and finally validators. Do I need to register IPublisher & ISender as well?

Thanks in advance

lilasquared commented 4 years ago

IPublisher and ISender will need to be registered if you want to use those interfaces. The DI extension project for dotnet core has not had this updated yet if you are using that as far as I know.

BH4NG commented 4 years ago

I am indeed working in dotnet core. What would be my best move? rollback to 8.0 for now, register the new interfaces or migrate to dotnet 5?

If I were to register IPublisher & ISender, would I need an implementation of them?

lilasquared commented 4 years ago

Actually it looks like the dependency injection project was updated to include the new interfaces, so you should just have to update the DI package to 9.0

BH4NG commented 4 years ago

With my DI package at 9.0 I still get the same error though

Output gives me: Exception thrown: 'System.InvalidOperationException' in MediatR.dll An exception of type 'System.InvalidOperationException' occurred in MediatR.dll but was not handled in user code

Error is thrown in the Handle() of ValidationBehaviour

jbogard commented 4 years ago

Do you have a minimal repro?

On Oct 15, 2020, at 3:59 PM, BH4NG notifications@github.com wrote:

 With my DI package at 9.0 I still get the same error though

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub, or unsubscribe.

BH4NG commented 4 years ago

I don't have a minimal repro atm, it occurs in a rather large project of mine, so I'm still investigating to narrow down atm.

ValidationBehaviour.cs public Task Handle(TRequest request, CancellationToken cancellationToken, RequestHandlerDelegate next) { var context = new ValidationContext(request); var failures = _validators .Select(v => v.Validate(context)) .SelectMany(result => result.Errors) .Where(f => f != null) .ToList();

  if (failures.Count != 0)
  {
    throw new ValidationException(failures);
  }

  return next();
}

There are 0 failures, but next() has a couple of errors, most of them related to reflection (see attachment) mediatr

This comes out of output: Exception thrown: 'System.InvalidOperationException' in MediatR.dll An exception of type 'System.InvalidOperationException' occurred in MediatR.dll but was not handled in user code Error constructing handler for request of type MediatR.IRequestHandler2[..System.Collections.Generic.IEnumerable1[Item]]. Register your handlers with the container. See the samples in GitHub for examples.

mediatr2

i'll try and set up a min repro, thanks for looking into to this!

lilasquared commented 4 years ago

@jbogard could it be because of these lines

services.TryAdd(new ServiceDescriptor(typeof(ISender), sp => sp.GetService<IMediator>(), serviceConfiguration.Lifetime));
services.TryAdd(new ServiceDescriptor(typeof(IPublisher), sp => sp.GetService<IMediator>(), serviceConfiguration.Lifetime));

are the IPublisher and ISender using the root container rather than scoped due to this?

BH4NG commented 4 years ago

I checked my registered services, to see what's what... seems like ISender & IPublisher are missing ImplementationType

mediatr3

jbogard commented 4 years ago

https://github.com/jbogard/MediatR.Extensions.Microsoft.DependencyInjection/blob/master/src/MediatR.Extensions.Microsoft.DependencyInjection/Registration/ServiceRegistrar.cs#L221

I've got it registered there.

BH4NG commented 4 years ago

Couldn't make a min repro, but did some more investigating: rolled-back MediatR from 9.0 to 8.1, problem still occured rolled back MediatR DependencyInjection from 9.0 to 7.0, no problem occured updated MediatR to 9.0 injecting IMediator instead of IPublisher and ISender, no problem occured MediatR 9.0 with injected ISender & IPublisher:

Unable to resolve service for type 'MediatR.ISender' while attempting to activate 'Controller'. at 
Microsoft.Extensions.DependencyInjection.ActivatorUtilities.GetService(IServiceProvider sp, Type type, Type requiredBy, 
Boolean isDefaultParameterRequired)

so I Add

  services.TryAdd(new ServiceDescriptor(typeof(IMediator), new MediatRServiceConfiguration().MediatorImplementationType, 
  ServiceLifetime.Singleton));
  services.TryAdd(new ServiceDescriptor(typeof(ISender), new MediatRServiceConfiguration().MediatorImplementationType, 
  ServiceLifetime.Singleton));
  services.TryAdd(new ServiceDescriptor(typeof(IPublisher), new MediatRServiceConfiguration().MediatorImplementationType, 
  ServiceLifetime.Singleton));

and again I get

'Error constructing handler for request of type MediatR.IRequestHandler`2[Query,Item]. Register your handlers with the container. See the samples in GitHub for examples.'

lilasquared commented 4 years ago

Did you try registering them as transient? I don't think the mediator(s) should be singletons.

BH4NG commented 4 years ago

I kept typeof(IMediator) singleton and changed ISender & IPublisher to transient and indeed things work again. Big thanks for the help gentlemen.

I do need to add, if I update the DependencyInjection package from 7.0 to 9.0, I again get error constructing handler for request of type MediatR.IRequestHandler

Not a big issue as I can keep using 7.0, but seems like something is not quite right with the registering of handlers moving from 7.0 to 9.0

lilasquared commented 4 years ago

@BH4NG the only time I am able to get an error similar to your original one was when the IMediator or ISender are registered as Singleton. Can you show the code you are using to register the IMediator when you upgrade to 9.0?

BH4NG commented 4 years ago

Sure, before and after upgrade I used:

services.AddMediatR(typeof(TStartup).GetTypeInfo().Assembly);
services.AddMediatR(cfg => cfg.AsScoped(), typeof(QueryHandler).GetTypeInfo().Assembly);
services.AddScoped<IDbContext, DbContext>(); 
services.AddTransient(typeof(IPipelineBehavior<,>), typeof(ValidationBehavior<,>));
services.AddScoped<IRepo<Item>, Repo<Item>>();

I also added the following, but I don't think it's necessary

      services.TryAdd(new ServiceDescriptor(typeof(IMediator), new 
MediatRServiceConfiguration().MediatorImplementationType, ServiceLifetime.Singleton));
  services.TryAdd(new ServiceDescriptor(typeof(ISender), new MediatRServiceConfiguration().MediatorImplementationType, 
ServiceLifetime.Transient));
  services.TryAdd(new ServiceDescriptor(typeof(IPublisher), new MediatRServiceConfiguration().MediatorImplementationType, 
ServiceLifetime.Transient));

On Upgrading I just changed the injected interface from IMediator to ISender & IPublisher respectively Im at .Net core 3.1 but for EFcore I'm on a preview for .NET

lilasquared commented 4 years ago

you shouldn't be making two calls to .AddMediatR(), that could possibly be messing it up. You can pass multiple assemblies to the call. Can you try reducing that to just one call?

BH4NG commented 4 years ago
services.AddMediatR(cfg => cfg.AsSingleton(), typeof(ILogger<>).GetTypeInfo().Assembly, typeof(ItemQueryHandler).GetTypeInfo().Assembly);

works with DependencyInjection at 7.0 but not with 9.0

lilasquared commented 4 years ago

I wouldn't expect AsSingleton to work at all so that is surprising. I wouldn't recommend it being singleton anyway so if registering that as scoped or transient fixes it for you that is probably best. I'm not able to get a repro of the issue between 7 and 9, both have the same behavior for me (throwing exceptions as expected when singleton, and not when transient)

BH4NG commented 4 years ago

Can I register the handlers asScoped() along as registering something asSingleton()? I cannot used two lambdas, no?

lilasquared commented 4 years ago

that configuration only dictates how the mediator implementation itself is registered, all pipeline behaviors and handlers are registered as transient.

BH4NG commented 4 years ago

The Handlers are not registered by default are they? because when I don't register them myself I get Handler was not found for request of type MediatR.IRequestHandler`2

I read somewhere I just needed to register one handler to add them all to the services, but because you can't add one specific implementation of 'the handlers' I did it like services.AddMediatR(cfg => cfg.AsScoped(), typeof(QueryHandler).GetTypeInfo().Assembly);

And that worked with DependencyInjection at 7.0. (Before that, I registered each handler seperatly and got the 'bug' my events where handled as many times as the number of registered handlers, so to me it seemed okay, even though I used .AddMediatR() twice..

Seeing as I can't make a min rep, I could invite you to my devops to have a look if you're interested.

lilasquared commented 4 years ago

Using that extension method for Microsoft DI searches the given assemblies and registers all the ones it finds as transient. You aren't registering one handler when you are using the typeof(QueryHandler) but you are telling that extension which assemblies to look in and it registers them. I wouldn't mind taking a look but I have a feeling the problem will be with registration somehow and not with MediatR

BH4NG commented 4 years ago

Yeah somehow the extension method alone does not find the handlers it would seem to me.

paul07481 commented 4 years ago

I had this same problem. Do you have Microsoft.Azure.AppConfiguration.AspNetCore installed and did you also recently upgrade that from 3.02 to 4.0.0? If so that will cause this problem. Downgrading or uninstalling that package solves the issue.

BH4NG commented 4 years ago

Hi Paul,

Glad I'm not the only one haha ;-) Unfortunately I don't Microsoft.Azure.AppConfiguration.AspNetCore installed. Do you have more insight as to why exaclty that package causes trouble? The only Azure package I have is Microsoft.Azure.ServiceBus. Before I start fiddling with it, I'm curious about your findings!

Kind regards

paul07481 commented 4 years ago

It took a while to figure out the culprit, but there is no doubt that is what caused my problem. As to why - that I don't know! My first thought is that the DI extension method used in the MediatR DI package is conflicting with a newly defined DI extension method elsewhere. In your case, if you also recently upgraded Microsoft.Azure.ServiceBus you could try rolling that back to the previous version you had and see if that solves the problem. Good luck.

BH4NG commented 4 years ago

Hi Paul, I thought as well that the problem is with the DI. seems like it gets reset or overwritten. I did not upgrade my Azure package, but had recently upgraded to EF Core 5.0 preview, maybe inner workings there differ much from 3.1 (and 2.2 for sure, the version my project was started in before upping to 3.1, which also had some breaking changes around AspNetCore package if I recall correctly). ATM I'm abit on a deadline, so can't look deeper into it right now, but for sure I will end up here on migrating to .NET 5 is my guess. I'll update any new findings. Thanks for your input either way!)

rickyzu commented 4 years ago

We're having similar issues. Using the below as part of .Net Core 3.1 causes a DI error.

services.AddMediatR(typeof(StartupExtensions).Assembly);

The above causes the below error: InvalidOperationException: Unable to resolve service for type 'xxxxx.Model.Repositories.IEntityRepository`1[xxxxx.EntityClasses.NamedEntity]' while attempting to activate 'xxxxx.CommandQuery.Command.SaveDocument.SaveDocumentCmdHandler'.

This exception was originally thrown at this call stack: Microsoft.Extensions.DependencyInjection.ServiceLookup.CallSiteFactory.CreateArgumentCallSites(System.Type, System.Type, Microsoft.Extensions.DependencyInjection.ServiceLookup.CallSiteChain, System.Reflection.ParameterInfo[], bool) Microsoft.Extensions.DependencyInjection.ServiceLookup.CallSiteFactory.CreateConstructorCallSite(Microsoft.Extensions.DependencyInjection.ServiceLookup.ResultCache, System.Type, System.Type, Microsoft.Extensions.DependencyInjection.ServiceLookup.CallSiteChain) Microsoft.Extensions.DependencyInjection.ServiceLookup.CallSiteFactory.TryCreateExact(Microsoft.Extensions.DependencyInjection.ServiceDescriptor, System.Type, Microsoft.Extensions.DependencyInjection.ServiceLookup.CallSiteChain, int) Microsoft.Extensions.DependencyInjection.ServiceLookup.CallSiteFactory.GetCallSite(Microsoft.Extensions.DependencyInjection.ServiceDescriptor, Microsoft.Extensions.DependencyInjection.ServiceLookup.CallSiteChain) Microsoft.Extensions.DependencyInjection.ServiceLookup.ServiceProviderEngine.ValidateService(Microsoft.Extensions.DependencyInjection.ServiceDescriptor)

Replacing with the below. services.AddMediatR(Assembly.GetExecutingAssembly());

resolves the DI issue but then on executing a command we get the below error:

'Handler was not found for request of type MediatR.IRequestHandler`2'

The Handlers sit in another project and it worked fine when the application was a .Net Core 2.2 application.

I can fix the problem by manually adding each Handler as a services.AddTransient(typeof(IRequestHandler<GetSomeQuery, SomeDto>), typeof(GetSomeQueryHandler)) but that's a work around.

As the same code worked fine in .Net Core 2.2 is there some migration steps I missed? It did not work with Mediatr 7.0 nor Mediatr 9.0

jbogard commented 4 years ago

Is this using the MediatR DI package?

rickyzu commented 4 years ago

image

@jbogard Yes, as far as I can tell it is using the installed MediatR Di package.

jbogard commented 4 years ago

Yeah I'm not sure, I've upgraded a few applications without any issues. I would look at the service collection before/after each call in Startup to make sure it has everything and what is overwriting it.

rickyzu commented 3 years ago

@jbogard - found our issue and all is working as intended now. The previous developer of the source had not registered one of the IEntityRepository in the DI so the handler that was dependent on it was correctly failing.