v12.3.0 Breaking Change #1038
Comments
|
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 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. |
|
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?
Thanks. |
|
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
}
}
} |
|
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? |
|
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. |
|
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 Maybe it would help if I had some of these examples too. haha. |
|
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): |
|
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. |
|
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? |
|
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 |
|
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. |
|
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. |
|
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]; |
|
This fix is on the MyGet feed: https://myget.org/gallery/mediatr-ci Can you verify that this fixes the issue? |
|
It was working, but this actually broke it again for me. |
|
It was working with which version? |
|
@jbogard It's working as intended.. when trying to register generic requests with two or more type parameters each parameter needs to have a constraint so that mediatr isn't trying to close with every single type in the assemblies passed to the service registrar. |
If you read the message it is telling you what the issue is.. You're going to have to put some constraints on your generic type parameters or turn off the feature via configuration. services.AddMediatR(cfg =>
{
//opt out flag set
cfg.RegisterGenericHandlers = false;
cfg.RegisterServicesFromAssembly(assembly);
}); |
This sounds to me like you were under the impression of being on 12.3.0 but you actually downgraded to the previous version. Because the bug in 12.3.0 could not be avoided unless you created a Since you would have to actively code the type evaluator I'm just assuming you were on a previous version. Your issue wouldn't just resolve itself.. It'd be nice, but that would be magic. haha. you can look at previous comments to get some context. |
|
This seems to be the issue: //all three type parameters will need constraints
public class Create<TEntity, TDtoIn, TDtoOut>
where TEntity : Common
where TDtoIn : IEntityDto //needs to be some sort of constraint on types that can close this generic argument
where TDtoOut : IEntityDto //needs to be some sort of constraint on types that can close this generic argument
{
//the rest goes here
}The reason for this is because when mediatR registers the handlers that can close this generic type, it has to scan the assembly and then find each and every combination of concrete But in any case this is why the error is being thrown in your application. @jbogard if this is a bad design then I can update it.. |
|
We started to get the same error message (when registering generic requests with more than two type parameters, each type parameter must have at least one constraint of type interface or class) after upgrading from 12.2.0 to 12.4.0 on the following code: public class GenericSelectListMessageHandler<TRequest, TEntity, TModel> : IRequestHandler<TRequest, IEnumerable<SelectListItemModel>>
where TRequest : SelectListRequest<TEntity>
where TEntity : class
where TModel : SelectListItemModelApparently |
This is by design. Class is a valid constraint but it doesn't help in narrowing down the types that can close the generic handler. Your best bet will be to add a constraint of a type IEntity or something or opt out of the auto registration functionality. services.AddMediatR(cfg =>
{
//opt out flag set
cfg.RegisterGenericHandlers = false;
cfg.RegisterServicesFromAssembly(assembly);
}); |
|
Could that |
It's not the I.E. an Edit: when trying to register handlers with three or more generic arguments, the generic registration is enforcing that there needs to be a specific type constraint on all three type arguments. |
I guess another option is to just remove the rules around type constraints and just let the user register every single possible concrete handler that can satisfy the generic handler created by the user. But I fear there will be many deadlock at startup issues that would arise from that, since mediatr will attempt to create registrations for each of those possible concrete combinations. For example, if your assemblies contained 100 classes and you have a generic that has three type parameters ( number of types that close T1 = 100 Total combinations would be 100^3 = 1,000,000 separate concrete service registrations. This seems kind of ridiculous to me since you only need maybe 20 of those tops for some specific implementation. The only way to cut this down is to use Type Constraints. Also, coding to the abstraction is just in general a good thing to do anyways. |
|
Ah I missed exactly how these PRs worked. So they're attempting to close ALL possible combinations of types instead of just leaving them open? I don't know how I missed that, I shouldn't review PRs on international flights... Honestly I'd rather remove this feature entirely in that case. I don't think it's a great approach for solving this problem. What I'd rather see happen is a more intelligent resolution algorithm. For example, the scanning keeps track of all the open generic handlers and THEN when you try to resolve them, don't rely completely on the container for resolution. If MediatR knew all the open handlers registered, it could fall back to attempting to close open generic handlers only when a concrete one is not registered. |
Correct, I'm scanning the assembly and finding ALL possible combinations that close and then creating the types via I don't know how to implement what you are suggesting but by all means maybe the feature is more trouble than it's worth. I can maintain this in an extension method in my own repository for those who would like to use it. Edit: wait.. are you suggesting that instead of registering all combinations you could somehow intercept the service provider and then register a service right before it's being used? in a kind of Lazy-Load sort of fashion? Edit 2: it sounds like you want to do what lamar is doing.. haha. good luck with that I guess. cheers Edit 3: Oh after looking at lamar's source I understand much more haha.. |
|
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. |
|
This issue was closed because it has been stalled for 14 days with no activity. |
After updating from 12.2.0 to 12.3.0, I'm getting a runtime exception during app instantiation.
The code:
The exception:
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.
The text was updated successfully, but these errors were encountered: