jbogard / MediatR.Extensions.Microsoft.DependencyInjection

MediatR extensions for Microsoft.Extensions.DependencyInjection
MIT License
327 stars 89 forks source link

Question on ServiceFactory #48

Closed t11omas closed 5 years ago

t11omas commented 5 years ago

Hi,

Question on the behaviour of the ServiceFactory, If resolving the service throws an ArgumentException it tries to build the class via reflection.

I have a situation where I have a pipeline and one of the steps failed to get resolved because of a dependency issue. What happens is, MediaR returns the pipeline but all the values where null because it couldn't build it via reflection.

The DI issue was swallowed and all I got from MediaR was a null reference exception

at MediatR.Internal.RequestHandlerWrapperImpl2.<>c__DisplayClass0_1.b__2()\r\n at MediatR.Internal.RequestHandlerWrapperImpl2.Handle(IRequest1 request, CancellationToken cancellationToken, ServiceFactory serviceFactory)\r\n at MediatR.Mediator.Send[TResponse](IRequest1 request, CancellationToken cancellationToken)\r\n

This was a pain because the issue wasn't obvious. What I have done as an interim solution is setup my own ServiceFactory impl, which is just a copy of yours without the catch.

Do you think that is ok, or is there a reason which I am unaware of what the catch block is suppose to handle.

services.AddScoped<ServiceFactory>(p => (type =>
            {
                try
                {
                    return p.GetService(type);
                }
                catch (ArgumentException)
                {
                    // Let's assume it's a constrained generic type
                    if (type.IsConstructedGenericType &&
                        type.GetGenericTypeDefinition() == typeof(IEnumerable<>))
                    {
                        var serviceType = type.GenericTypeArguments.Single();
                        var serviceTypes = new List<Type>();
                        foreach (var service in services)
                        {
                            if (serviceType.IsConstructedGenericType &&
                                serviceType.GetGenericTypeDefinition() == service.ServiceType)
                            {
                                try
                                {
                                    var closedImplType = service.ImplementationType.MakeGenericType(serviceType.GenericTypeArguments);
                                    serviceTypes.Add(closedImplType);
                                } catch { }
                            }
                        }

                        services.Replace(new ServiceDescriptor(type, sp =>
                        {
                            return serviceTypes.Select(sp.GetService).ToArray();
                        }, ServiceLifetime.Transient));

                        var resolved = Array.CreateInstance(serviceType, serviceTypes.Count);

                        Array.Copy(serviceTypes.Select(p.GetService).ToArray(), resolved, serviceTypes.Count);

                        return resolved;
                    }

                    throw;
                }
            }));
jbogard commented 5 years ago

I think that catch block can be improved - it's trying to catch generic constraints not matching and it's really the only decent way of doing this.