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.

Sign up for GitHub

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

Jump to bottom

Unity DI fails to instantiate EndpointMetadataApiDescriptionProvider in .NET 8 #57694

Closed
1 task done
naamunds opened this issue Sep 4, 2024 · 8 comments · Fixed by #57797
Closed
1 task done

Unity DI fails to instantiate EndpointMetadataApiDescriptionProvider in .NET 8 #57694

naamunds opened this issue Sep 4, 2024 · 8 comments · Fixed by #57797
Labels
area-minimal Includes minimal APIs, endpoint filters, parameter binding, request delegate generator etc feature-openapi
Milestone
Backlog

Comments

@naamunds
Copy link
Member

naamunds commented Sep 4, 2024

Is there an existing issue for this?

  • I have searched the existing issues

Describe the bug

When upgrading an ASP.NET MVC app which uses Unity DI and Swagger from .NET 6 to .NET 8, the Swagger endpoint fails with an InvalidOperationException.

This happens because the AddEndpointsApiExplorer method which is called by the MVC bootstrapping (see Get started with Swashbuckle and ASP.NET Core) registers a Microsoft.AspNetCore.Mvc.ApiExplorer.EndpointMetadataApiDescriptionProvider object with the DI framework, and EndpointMetadataApiDescriptionProvider was changed in Support setting ApiParameterRouetInfo for endpoints to remove the constructor without an IServiceProviderIsService parameter. When Unity tries to instantiate the EndpointMetadataApiDescriptionProvider, it throws an InvalidOperationException because it doesn't have an IServiceProviderIsService instance to provide. Unity was last updated in 2019, and IServiceProviderIsService was introduced in 2021, so it makes sense that Unity wouldn't have one whereas the default DI framework would. However, in .NET 6 this resulted in the EndpointMetadataApiDescriptionProvider being created with null for the IServiceProviderIsService, but in .NET 8 (and I think 7) it fails, making the app's Swagger endpoint unusable.

Expected Behavior

I believe this could be fixed by keeping a constructor on EndpointMetadataApiDescriptionProvider which doesn't have an IServiceProviderIsService parameter, or possibly just a constructor where the IServiceProviderIsService parameter is optional (defaulting to null).

Steps To Reproduce

I've created a sample app to demonstrate this issue at https://github.com/naamunds/samples/tree/main/UnityMvcApp.

When you run it, you'll see that the Swagger page fails to load. You can see the exception (also pasted in the section below) in the Debug output in Visual Studio.

If you change the TargetFramework in UnityMvcApp.csproj from net8.0 to net6.0, the issue doesn't occur. Also, if you remove the .UseUnityServiceProvider() line in Program.cs, the issue doesn't occur.

You can add the following line to the beginning of Startup.Configure and put a breakpoint to see whether (and how) the EndpointMetadataApiDescriptionProvider is created. (In the Locals window, expand providers and then expand Results View.)

IEnumerable<object?> providers = app.ApplicationServices.GetServices(typeof(Microsoft.AspNetCore.Mvc.ApiExplorer.IApiDescriptionProvider));

Exceptions (if any)

System.InvalidOperationException: Unable to resolve service for type 'Swashbuckle.AspNetCore.Swagger.ISwaggerProvider' while attempting to Invoke middleware 'Swashbuckle.AspNetCore.Swagger.SwaggerMiddleware'.
   at lambda_method1(Closure, Object, HttpContext, IServiceProvider)
   at Microsoft.WebTools.BrowserLink.Net.BrowserLinkMiddleware.InvokeAsync(HttpContext context)
   at Microsoft.AspNetCore.Watch.BrowserRefresh.BrowserRefreshMiddleware.InvokeAsync(HttpContext context)
   at Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http.HttpProtocol.ProcessRequests[TContext](IHttpApplication`1 application)

.NET Version

8.0.400

Anything else?

Visual Studio 17.11.1
@dotnet-issue-labeler dotnet-issue-labeler bot added the area-web-frameworks *DEPRECATED* This label is deprecated in favor of the area-mvc and area-minimal labels label Sep 4, 2024
@naamunds
Copy link
Member Author

naamunds commented Sep 4, 2024

Tagging @captainsafia to look at this issue.

Here is the workaround that my team used to allow us to upgrade to .NET 8:

            if (false == unityContainer.IsRegistered<IServiceProviderIsService>())
            {
                unityContainer.RegisterFactory<IServiceProviderIsService>(
                    (IUnityContainer c) => null,
                    new ContainerControlledLifetimeManager());
            }

@captainsafia
Copy link
Member

captainsafia commented Sep 5, 2024 •
edited
Loading

@naamunds Thanks for filing this issue!

I think the diff to fix this should be fairly straightforward:

-IServiceProviderIsService? serviceProviderIsService)
+IServiceProviderIsService? serviceProviderIsService = null)

Would you be interested in submitting a PR with the fix to main?

As for back porting, we'll have to evaluate the relative cost/severity of the issue -- especially because you've been successful with a workaround for now.

P.S. Sticking this in the backlog based on severity but still open to a bug fix/backport.

@captainsafia captainsafia added feature-openapi area-minimal Includes minimal APIs, endpoint filters, parameter binding, request delegate generator etc and removed area-web-frameworks *DEPRECATED* This label is deprecated in favor of the area-mvc and area-minimal labels labels Sep 5, 2024
@captainsafia captainsafia added this to the Backlog milestone Sep 5, 2024
@marcominerva
Copy link
Contributor

Hi @captainsafia!

I think the diff to fix this should be fairly straightforward:

-IServiceProviderIsService? serviceProviderIsService)
+IServiceProviderIsService? serviceProviderIsService = null)

Do you refer to this line?

aspnetcore/src/Mvc/Mvc.ApiExplorer/src/EndpointMetadataApiDescriptionProvider.cs

Line 36 in f2ea31d

IServiceProviderIsService? serviceProviderIsService)

@captainsafia
Copy link
Member

@marcominerva Yep -- this is the piece of code I was referring to.

@marcominerva
Copy link
Contributor

marcominerva commented Sep 10, 2024 •
edited
Loading

@captainsafia So, if no one else asks for it, I would be happy to take care of the PR 😀

@captainsafia
Copy link
Member

@marcominerva No one has jumped on opening a PR yet so I'm happy to review one from you!

@naamunds
Copy link
Member Author

@marcominerva that would be great if you could create the PR for this. I verified that adding the null default for the serviceProviderIsService parameter on the constructor should fix this.

@naamunds
Copy link
Member Author

@captainsafia although there's a simple workaround for this issue, people who hit it may not easily figure out what the issue is. It wasn't clear from the exception where exactly the issue was coming from, and it took my colleague and me a few hours to figure it out even though we're experienced with ASP.NET and Unity. This could potentially block ASP.NET developers from upgrading their app's .NET version, so I'd recommend backporting it to .NET 8 if it's not too much trouble.

@marcominerva marcominerva mentioned this issue Sep 11, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
area-minimal Includes minimal APIs, endpoint filters, parameter binding, request delegate generator etc feature-openapi
Projects
None yet
Milestone
Backlog
3 participants
@captainsafia @marcominerva @naamunds