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

v12.3.0 Breaking Change #1038

Open
bsal649 opened this issue Jun 7, 2024 · 13 comments
Open

v12.3.0 Breaking Change #1038

bsal649 opened this issue Jun 7, 2024 · 13 comments

Comments

@bsal649
Copy link

bsal649 commented Jun 7, 2024

After updating from 12.2.0 to 12.3.0, I'm getting a runtime exception during app instantiation.

The code:

// Register MediatR
Assembly[] assemblies = Assembly.GetEntryAssembly()!.GetReferencedAssemblies()
                        .Select(Assembly.Load).Append(Assembly.GetCallingAssembly()).ToArray();

_ = services.AddMediatR(config => config.RegisterServicesFromAssemblies(assemblies)); // exception happens here

The exception:

Exception has occurred: CLR/System.ArgumentException
An exception of type 'System.ArgumentException' occurred in System.Private.CoreLib.dll but was not handled in user code: 'The number of generic arguments provided doesn't equal the arity of the generic type definition.'
   at System.RuntimeType.MakeGenericType(Type[] instantiation)
   at MediatR.Registration.ServiceRegistrar.<>c__DisplayClass5_0.<GetConcreteRequestTypes>b__2(Type type)
   at System.Linq.Enumerable.SelectListIterator`2.Fill(ReadOnlySpan`1 source, Span`1 destination, Func`2 func)
   at System.Linq.Enumerable.SelectListIterator`2.ToList()
   at MediatR.Registration.ServiceRegistrar.GetConcreteRequestTypes(Type openRequestHandlerInterface, Type openRequestHandlerImplementation, IEnumerable`1 assembliesToScan)
   at MediatR.Registration.ServiceRegistrar.AddAllConcretionsThatClose(Type openRequestInterface, List`1 concretions, IServiceCollection services, IEnumerable`1 assembliesToScan)
   at MediatR.Registration.ServiceRegistrar.ConnectImplementationsToTypesClosing(Type openRequestInterface, IServiceCollection services, IEnumerable`1 assembliesToScan, Boolean addIfAlreadyExists, MediatRServiceConfiguration configuration)
   at MediatR.Registration.ServiceRegistrar.AddMediatRClasses(IServiceCollection services, MediatRServiceConfiguration configuration)
   at Microsoft.Extensions.DependencyInjection.ServiceCollectionExtensions.AddMediatR(IServiceCollection services, MediatRServiceConfiguration configuration)
   at Microsoft.Extensions.DependencyInjection.ServiceCollectionExtensions.AddMediatR(IServiceCollection services, Action`1 configuration)
   at API.Extensions.ApplicationServiceExtensions.AddApplicationServices(IServiceCollection services, IConfiguration config) in C:\app\API\Extensions\ApplicationServiceExtensions.cs:line 75
   at Program.<<Main>$>d__0.MoveNext() in C:\app\API\ProgramException has occurred: CLR/System.ArgumentException
An exception of type 'System.ArgumentException' occurred in System.Private.CoreLib.dll but was not handled in user code: 'The number of generic arguments provided doesn't equal the arity of the generic type definition.'
   at System.RuntimeType.MakeGenericType(Type[] instantiation)
   at MediatR.Registration.ServiceRegistrar.<>c__DisplayClass5_0.<GetConcreteRequestTypes>b__2(Type type)
   at System.Linq.Enumerable.SelectListIterator`2.Fill(ReadOnlySpan`1 source, Span`1 destination, Func`2 func)
   at System.Linq.Enumerable.SelectListIterator`2.ToList()
   at MediatR.Registration.ServiceRegistrar.GetConcreteRequestTypes(Type openRequestHandlerInterface, Type openRequestHandlerImplementation, IEnumerable`1 assembliesToScan)
   at MediatR.Registration.ServiceRegistrar.AddAllConcretionsThatClose(Type openRequestInterface, List`1 concretions, IServiceCollection services, IEnumerable`1 assembliesToScan)
   at MediatR.Registration.ServiceRegistrar.ConnectImplementationsToTypesClosing(Type openRequestInterface, IServiceCollection services, IEnumerable`1 assembliesToScan, Boolean addIfAlreadyExists, MediatRServiceConfiguration configuration)
   at MediatR.Registration.ServiceRegistrar.AddMediatRClasses(IServiceCollection services, MediatRServiceConfiguration configuration)
   at Microsoft.Extensions.DependencyInjection.ServiceCollectionExtensions.AddMediatR(IServiceCollection services, MediatRServiceConfiguration configuration)
   at Microsoft.Extensions.DependencyInjection.ServiceCollectionExtensions.AddMediatR(IServiceCollection services, Action`1 configuration)
   at API.Extensions.ApplicationServiceExtensions.AddApplicationServices(IServiceCollection services, IConfiguration config) in C:\app\API\Extensions\ApplicationServiceExtensions.cs:line 75
   at Program.<<Main>$>d__0.MoveNext() in C:\app\API\Program.cs:line 46.cs:line 46

Not too sure how to fix this unfortunately. I don't have any generic handlers (that I know of at least) and I didn't change anything else.

@zachpainter77
Copy link
Contributor

I think the problem might be with the assemblies you are passing to mediatR for registration. MediatR uses those assemblies to scan for types. I think when you use GetReferencedAssemblies you might be returning too many. I usually explicitly build my assmeblies list like so:

var assemblies = new Assembly[] 
{
    typeof(Request1).Assembly,
    typeof(Request2).Assembly
};

That's just my initial thought... I will mess around with this some more to see if I can reproduce this issue on my end.

@zachpainter77
Copy link
Contributor

I am creating a test project to test out your code to see if I can reproduce the error. So far I have not been successful. The application seems to run fine with the way you are retrieving assemblies to pass to MediatR and the way that I am explicitly setting those assemblies. Can you provide some more detail perhaps?

  • What version of .net core is your app using?
  • What are some examples of your request and handler definitions?

Thanks.

@bsal649
Copy link
Author

bsal649 commented Jun 12, 2024

SDK version: 8.0.302

Definitions example:

public static class Create
{
    public class Command : IRequest<Result<Unit>>
    {
        public required int foo { get; set; }
    }

    public class Handler : IRequestHandler<Command, Result<Unit>>
    {
        private readonly IConfiguration _config;
        private readonly DataContext _context;
        private readonly ILogger<Handler> _logger;
        private readonly IMediator _mediator;

        public Handler(
            IConfiguration config,
            DataContext context,
            ILogger<Handler> logger,
            IMediator mediator
        )
        {
            _config = config;
            _context = context;
            _logger = logger;
            _mediator = mediator;
        }

        public async Task<Result<Unit>> Handle(Command request, CancellationToken cancellationToken)
        {
            // endpoint logic
        }
    }
}

@zachpainter77
Copy link
Contributor

hmm.. well.. I tested your exact code and I can't seem to reproduce the issue on my end. Are there any other details that could be helpful? Anything that you have in your code that I could be missing or important to the issue?

@bsal649
Copy link
Author

bsal649 commented Jun 12, 2024

I'm looking into the code, it's a large codebase that I recently inherited so it's possible. While googling I found this old MediatR issue that seems to be the same problem, and it's had some activity in the last few weeks: #674.

It could be the Handler registrations, which occur directly after the AddMediatR call. My predecessor stopped adding these at some point, for whatever reason, so I didn't add them either and it worked fine so far with only about 25% of the Handlers being registered like this.

Handler registrations:

_ = services.AddTransient<IRequestHandler<Create<Foo, Bar, FooBar>.Command, Result<Unit>>, Create<Foo, Bar, FooBar>.Handler>();
_ = services.AddTransient<IRequestHandler<Read<Foo, Bar, FooBar>.Query, Result<List<FooBar>>>, Read<Foo, Bar, FooBar>.Handler>();
_ = services.AddTransient<IRequestHandler<Update<Foo, Bar, FooBar, FooFooBarBar>.Command, Result<Unit>>, Update<Foo, Bar, FooBar, FooFooBarBar>.Handler>();
_ = services.AddTransient<IRequestHandler<Delete<Foo, Bar>.Command, Result<Unit>>, Delete<Foo, Bar>.Handler>();

Edit: Pretty sure it's not that because these come after anyway.

@zachpainter77
Copy link
Contributor

yeah those are the same error but probably not the same issue entirely.. The error of arity is suggesting that Mediator is attempting to register a dependency like so:

services.AddTransient(typeof(IRequestHandler<,>), typeof(ConcreteRequestHandler<,,>));

this will not work since the arity (number of generic type arguments) do not match.

I must admit.. I've never seen MediatR used like this before.. But the above code does seem to be trying to register generic commands and handlers. I think those service registrations is what the last update was intended to alleviate. It appears that the registrations are concrete registrations for generic request handlers, correct?

Are the Command and Handler static properties?

Maybe it would help if I had some of these examples too. haha.

@bsal649
Copy link
Author

bsal649 commented Jun 12, 2024

Ah yeah you're right, they are concrete registrations for generic handlers. It seems to be an attempt to implement generic CRUD operations but it was abandoned at some point so is only used in old code.

Here's 'Create' as an example, which I believe is an open generic? I'm new to C# so not too sure.

public class Create<TEntity, TDtoIn, TDtoOut> where TEntity : Common
{
    public class Command : IRequest<Result<Unit>>
    {
        public required List<TDtoIn> DtoInList { get; set; }
        public Func<TEntity, string>? TagStrategy { get; set; }
    }

    public class Handler : IRequestHandler<Command, Result<Unit>>
    {
        private readonly DataContext _context;
        private readonly IMapper _mapper;
        private readonly IUserAccessor _userAccessor;

        public Handler(DataContext context, IMapper mapper, IUserAccessor userAccessor)
        {
            _userAccessor = userAccessor;
            _mapper = mapper;
            _context = context;
        }

        public async Task<Result<Unit>> Handle(Command request, CancellationToken cancellationToken)
        {
            // endpoint logic
        }
    }
}

Also, I'm not sure if assemblies is supposed to contain all these dependencies and standard libs (from debug window):
image

@zachpainter77
Copy link
Contributor

Ok I was able to reproduce the issue.. I added some generic type parameters to the static class.

 public static class Create<T, T1, T2>
 {

     public class Command : IRequest<Result<Unit>>
     {
         public required int Foo { get; set; }
     }

     public class Handler : IRequestHandler<Command, Result<Unit>>
     {
         private readonly IConfiguration _config;
         private readonly DataContext _context;
         private readonly ILogger<Handler> _logger;
         private readonly IMediator _mediator;

         public Handler(
             IConfiguration config,
             DataContext context,
             ILogger<Handler> logger,
             IMediator mediator
         )
         {
             _config = config;
             _context = context;
             _logger = logger;
             _mediator = mediator;
         }

         public Task<Result<Unit>> Handle(Command request, CancellationToken cancellationToken)
         {
             return Task.FromResult(new Result<Unit>());
         }
     }
 }

I then ran the code as is, and I was able to get the arity error like above. I then downgraded to the previous version. The code ran but also did not register the handler.

I then registered the handler concretely like so:

builder.Services.AddTransient<IRequestHandler<Create<Foo, Bar, FooBar>.Command, Result<Unit>>, Create<Foo, Bar, FooBar>.Handler>();

Then the code worked again and the handler was indeed found.

Also, yes there doesn't need to be that many assemblies for MediatR to scan through. though it didn't seem to matter either way when testing this specific issue but it could speed up initialization for your app. Mediator scans the assemblies passed to it for mediator registrations. So by providing more assemblies than needed you are adding extra work.

So in summary the issue is that in the previous version MediatR, it would not even attempt to register generic commands and queries so you had to explicitly register them like your example above.

With that said, I think there should be a check to make sure that MediatR isn't trying to register generic abstractions to concrete implementations when the arity does not match. So that is a bug that will need to be fixed.

A workaround for you might be to essentially turn off that feature by providing a type evaluator like so:

builder.Services.AddMediatR(config => {
    config.TypeEvaluator = x => !x.ContainsGenericParameters;
    config.RegisterServicesFromAssemblies(assemblies);
});

This will tell MediatR to not attempt to register types that have generic parameters. Or you can simply roll back to the previous version. In the meantime I will be working on a fix for this bug.

Thanks.

@bsal649
Copy link
Author

bsal649 commented Jun 12, 2024

Really appreciate your help. That probably explains why the app is so deathly slow to build as well. I might suggest adding a warning if that check is triggered, because it could be an indicator of a bad configuration like mine.

If the issue is a mismatch between arity of the concrete handler and the IRequestHandler, could I maybe be more specific about the type parameters of IRequestHandler?

@zachpainter77
Copy link
Contributor

Well.. The real issue in why your code is not working has to deal more with the new code that I added. The code attempts to find all types to close your generic request, for example Create<TEntity> however the code didn't have any support for more than one type parameter. I am working on a fix that will close this issue and allow for "N" type parameters.

@bsal649
Copy link
Author

bsal649 commented Jun 13, 2024

For reasons beyond my understanding, I'm no longer getting the exception now that I'm specifying the assembly. But it doesn't register the generic handlers, they have to be done separately as before.

@zachpainter77
Copy link
Contributor

I would think you must have rolled back the MediatR version or you must have added the type evaluator lambda so that MedaitR does not try and register the generic handlers.

Because as of right now, trying to register generic commands and handlers with more than one type parameter via AddMediatR assembly scanning results in the exception.

@bsal649
Copy link
Author

bsal649 commented Jun 13, 2024

Still on 12.3.0, only thing I changed was went from

Assembly[] assemblies = Assembly.GetEntryAssembly()!.GetReferencedAssemblies().Select(Assembly.Load).Append(Assembly.GetCallingAssembly()).ToArray();

to

Assembly[] assemblies = [typeof(Application.Consent.Read).Assembly];

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants