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

Autofac behaves differently than the Microsoft ServiceProvider with lambda registrations that return null #60

Closed
scottt732 opened this issue Oct 17, 2019 · 4 comments
Assignees
Labels

Comments

@scottt732
Copy link

scottt732 commented Oct 17, 2019

Describe the bug
I'm not sure this is a bug or maybe something worth calling out in the documentation. I just wanted to bring it to your attention in case you're trying to be a 100% drop-in replacement for Microsoft's IServiceProvider implementation.

I ran into a runtime exception resolving a service graph after introducing Autofac to an existing .NET Core app. It was a straightforward fix & a code smell in our codebase, but it was surprising nonetheless.

To reproduce
Reproduction of the issue, ideally in a unit test format.

[Fact]
public void BuildAutofacServiceProvider_WhenLambdaServiceRegistrationViaMs_ReturnsNull()
{
    var serviceCollection = new ServiceCollection();

    serviceCollection.AddTransient<ConcreteSampleService1>(sp => null);

    var serviceProvider = serviceCollection.BuildServiceProvider();
    var service = serviceProvider.GetService<ConcreteSampleService1>();

    Assert.Null(service); 
    // Pass
}

[Fact]
public void BuildAutofacServiceProvider_WhenLambdaServiceRegistrationViaAutofac_ReturnsNull()
{
    var serviceCollection = new ServiceCollection();
    var containerBuilder = new ContainerBuilder();

    serviceCollection.AddTransient<ConcreteSampleService1>(sp => null);
    containerBuilder.Populate(serviceCollection);

    var container = containerBuilder.Build();
    var serviceProvider = new AutofacServiceProvider(container);
    var service = serviceProvider.GetService<ConcreteSampleService1>();

    // Fail --> throws DependencyResolutionException (see below)

    Assert.Null(service); 
}

Full exception with stack trace:

    Autofac.Core.DependencyResolutionException : An exception was thrown while activating λ:Sample.Test.DependencyInjection.ConcreteSampleService1.
    ---- Autofac.Core.DependencyResolutionException : A delegate registered to create instances of 'Sample.Test.DependencyInjection.ConcreteSampleService1' returned null.
       at Autofac.Core.Resolving.InstanceLookup.Activate(IEnumerable`1 parameters, Object& decoratorTarget) in C:\projects\autofac\src\Autofac\Core\Resolving\InstanceLookup.cs:line 135
       at Autofac.Core.Resolving.InstanceLookup.Execute() in C:\projects\autofac\src\Autofac\Core\Resolving\InstanceLookup.cs:line 83
       at Autofac.Core.Resolving.ResolveOperation.GetOrCreateInstance(ISharingLifetimeScope currentOperationScope, IComponentRegistration registration, IEnumerable`1 parameters) in C:\projects\autofac\src\Autofac\Core\Resolving\ResolveOperation.cs:line 131
       at Autofac.Core.Resolving.ResolveOperation.Execute(IComponentRegistration registration, IEnumerable`1 parameters) in C:\projects\autofac\src\Autofac\Core\Resolving\ResolveOperation.cs:line 84
       at Autofac.ResolutionExtensions.TryResolveService(IComponentContext context, Service service, IEnumerable`1 parameters, Object& instance) in C:\projects\autofac\src\Autofac\ResolutionExtensions.cs:line 1041
       at Autofac.ResolutionExtensions.ResolveOptionalService(IComponentContext context, Service service, IEnumerable`1 parameters) in C:\projects\autofac\src\Autofac\ResolutionExtensions.cs:line 814
       at Microsoft.Extensions.DependencyInjection.ServiceProviderServiceExtensions.GetService[T](IServiceProvider provider)
       at Sample.Test.DependencyInjection.ServiceCollectionExtensionsTests.BuildAutofacServiceProvider_WhenLambdaServiceRegistrationViaAutofacReturnsNull_ReturnsNull() in /Users/scottholodak/Code/peakpass/Sample.Test/DependencyInjection/ServiceCollectionExtensionsTests.cs:line 25
    ----- Inner Stack Trace -----
       at Autofac.Core.Activators.Delegate.DelegateActivator.ActivateInstance(IComponentContext context, IEnumerable`1 parameters) in C:\projects\autofac\src\Autofac\Core\Activators\Delegate\DelegateActivator.cs:line 74
       at Autofac.Core.Resolving.InstanceLookup.Activate(IEnumerable`1 parameters, Object& decoratorTarget) in C:\projects\autofac\src\Autofac\Core\Resolving\InstanceLookup.cs:line 117

Assembly/dependency versions:

<PackageReference Include="Autofac" Version="4.9.4" />
<PackageReference Include="Autofac.Extensions.DependencyInjection" Version="5.0.1" />

I was also able to reproduce this in Autofac 4.9.4 + Autofac.Extensions.DependencyInjection 4.4.0

Additional context
Add any other context about the problem here.

@alsami alsami self-assigned this Oct 18, 2019
@alsami
Copy link
Member

alsami commented Oct 18, 2019

Hey @scottt732,

thanks for reporting this. I am not sure which constellation led to your error but what you are trying to do actually works the same as the service-provider created by Microsoft and is therefor indeed a full-replacement for it.

Here the code I've used

    public class DepTests
    {
        private class SomeDependency {}
        
        [Fact]
        public void DependencyRegisteredResolved()
        {
            var serviceCollection = new ServiceCollection();
            serviceCollection.AddTransient<SomeDependency>();
            var containerBuilder = new ContainerBuilder();
            containerBuilder.Populate(serviceCollection);
            var container = containerBuilder.Build();
            var autofacServiceProvider = new AutofacServiceProvider(container);
            var service = autofacServiceProvider.GetService<SomeDependency>();
            
            Assert.NotNull(service);
        }
        
        [Fact]
        public void DependencyOptionalResolved()
        {
            var serviceCollection = new ServiceCollection();
            var containerBuilder = new ContainerBuilder();
            containerBuilder.Populate(serviceCollection);
            var container = containerBuilder.Build();
            var autofacServiceProvider = new AutofacServiceProvider(container);
            var service = autofacServiceProvider.GetService<SomeDependency>();
            
            Assert.Null(service);
        }
    }

I've created a repository that you can clone and try to find out what went wrong for you.

@alsami alsami closed this as completed Oct 18, 2019
@tillig
Copy link
Member

tillig commented Oct 18, 2019

@alsami Note the first test in the @scottt732 example doesn't use Autofac:

    serviceCollection.AddTransient<ConcreteSampleService1>(sp => null);
    var serviceProvider = serviceCollection.BuildServiceProvider();
    var service = serviceProvider.GetService<ConcreteSampleService1>();

The issue here is that the Microsoft service provider apparently allows you to register a dependency that returns null while Autofac does not.

I'm not sure this is something we actually want to fix, though, so I think there's still no action required.

  • People choose Autofac because they want the Autofac behavior.
  • Autofac doesn't let you inject a null value. You shouldn't ever register a delegate that might return null. Constructor parameters, for example, should all be required. Optional parameters should likely be properties, not constructor parameters.
  • The difference between IServiceProvider.GetService and IServiceProvider.GetRequiredService isn't "is the value null or not?" - it's "is the service registered or not?"
  • There are spec tests that Microsoft provides that illustrate what non-interface-based behaviors are required. Autofac fulfills all of those tests at the time of this writing.

Long story already too long, I think this is a "won't fix." If Microsoft adds this to their tests and requires all of the conforming container implementations to support it, we can revisit it at that time.

@tillig tillig added the wontfix label Oct 18, 2019
@alsami
Copy link
Member

alsami commented Oct 18, 2019

@alsami Note the first test in the @scottt732 example doesn't use Autofac:

Yeah missed that one thanks for the hint :)

@scottt732
Copy link
Author

scottt732 commented Oct 18, 2019

Thanks for the quick responses.

  • I'm totally fine with the won't fix until/unless the MS library has a test in place.
  • Also fine with "People choose Autofac because they want the Autofac behavior".
  • Totally agree with "You shouldn't ever register a delegate that might return null" (the trivial fix in our project was to address this)

I just wanted to clarify a few points a bit above for future Googlers' sake.

"the Microsoft service provider apparently allows you to register a dependency that returns null while Autofac does not."
Microsoft's IServiceCollection isn't required. This test is also inconsistent w/MS' implementation. Both Microsoft.Extensions.DependencyInjection and Autofac allow you to register lambdas/anonymous factories that can return null (see below). Only Microsoft's allows you to resolve it. Neither can catch this at registration time in part because they depend on an IServiceProvider/IComponentContext (available post-build) and in part because they'd have no straightforward way to eagerly execute the lambda and exercise all possible code paths, predict all possible return values, and predict what might be pulled out of the IServiceProvider / ComponentContext.

[Fact]
public void BuildAutofacServiceProvider_WhenLambdaServiceRegistrationViaAutofac1_UnfortunatelyThrowsException()
{
    var containerBuilder = new ContainerBuilder();

    containerBuilder.Register<ConcreteSampleService1>(cc => null);

    var container = containerBuilder.Build();
    var serviceProvider = new AutofacServiceProvider(container);
    var service = serviceProvider.GetService<ConcreteSampleService1>();

    // Fail --> throws DependencyResolutionException (see below)
    
    Assert.Null(service);
}

"The difference between IServiceProvider.GetService and IServiceProvider.GetRequiredService isn't "is the value null or not?" - it's "is the service registered or not?"
I updated my tests to reflect this but it looks like it also depends on "is the value null or not" (or at least that's how I'm interpreting the passing test below--correct me if I'm wrong on that). Also worth calling out this project is using .NET Core 2.1 so possibly the behavior has changed.

[Fact]
public void BuildAutofacServiceProvider_WhenLambdaServiceRegistrationViaMs_ReturnsNullServiceReference()
{
    var serviceCollection = new ServiceCollection();

    serviceCollection.AddTransient<ConcreteSampleService1>(sp => null);

    var serviceProvider = serviceCollection.BuildServiceProvider();
    var service = serviceProvider.GetService<ConcreteSampleService1>();
    
    Assert.Null(service);
    Assert.Throws<InvalidOperationException>(() => serviceProvider.GetRequiredService<ConcreteSampleService1>());
}
sc.AddTransient<Widget>(sp => null); cb.Register<Widget>(cc => null);
msServiceProvider.GetService<Widget>() null N/A
autofacServiceProvider.GetService<Widget>() Autofac.DependencyResolutionException Autofac.DependencyResolutionException
msServiceProvider.GetRequiredService<Widget>() InvalidOperationException N/A
autofacServiceProvider.GetRequiredService<Widget>() Autofac.DependencyResolutionException Autofac.DependencyResolutionException

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

3 participants