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

The IEnumerable<Object> DependencyInjectionMvcDependencyResolver.GetServices(Type serviceType) causes an NRE #19

Open
djseng opened this issue Sep 4, 2020 · 6 comments
Labels
bug Something isn't working fix-verification-needed Issue is now fixed in latest release, needs OP to confirm it's fixed for them at their end.

Comments

@djseng
Copy link
Contributor

djseng commented Sep 4, 2020

The GetServices call causes an NRE to happen when nothing can be resolved from the container.

This issue is similar; coalescing an empty enumerable appears to resolve the issue.

@daiplusplus
Copy link
Owner

daiplusplus commented Sep 18, 2020

Thank you for the contribution!

Looking at my original code I see that closedGenericType is unused - so I wonder if I should have passed that as the parameter argument to GetService instead... or if I left that-in as a reference without commenting it out by mistake.

I do remember (during my development of support for ASP.NET MVC) that I broke DI by returning a non-null, but empty, IEnumerable<T> instead of null.

I need to take a quick look at how ASP.NET MVC consumes the result of IEnumerable<Object> GetServices(Type serviceType) to verify your fix.

I see in the linked SimpleInjector thread that @dotnetjunkie writes (emphasis mine):

Although Simple Injector 3 contains both methods, GetAllInstances does not match the expectations of MVC, because MVC expects that method to return an empty collection in case no collection is registered.

I assume he's correct on this though.

Do you have a bug repro scenario for this issue? And do you have a unit-test or integration-test that proves this fixes it?

Thanks again!

@djseng
Copy link
Contributor Author

djseng commented Sep 21, 2020

To reproduce the issue, #17 would need to be resolved, and then visiting one of the basic routes in the SampleMvcWebApplication will demonstrate this issue. It is happening, if I recall, in the bowels of the Mvc pipeline. I will explore creating an integration test soon.

@TheJayMann
Copy link
Collaborator

I can confirm that the closedGenericType parameter is necessary and is what needs to be passed to the GetService call, assuming you actually want to take advantage of dependency injection for supplying the providers. Technically passing in the raw type itself works so long as you don't ever register one of the providers via dependency injection, but immediately causes a cast error if you do.

@rli-dseng
Copy link

fix in fa60c05

@daiplusplus
Copy link
Owner

I've just published version 5.0.0-beta01 which solves this issue. Can you give it a try to confirm it fixed it for you?

https://www.nuget.org/packages/Jehoel.AspNetDependencyInjection/5.0.0-beta01

https://www.nuget.org/packages/Jehoel.AspNetDependencyInjection.Mvc/5.0.0-beta01

@rli-dseng
Copy link

I would, but I am not on that project anymore. I'm sorry.

@daiplusplus daiplusplus added fix-verification-needed Issue is now fixed in latest release, needs OP to confirm it's fixed for them at their end. bug Something isn't working labels Aug 7, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working fix-verification-needed Issue is now fixed in latest release, needs OP to confirm it's fixed for them at their end.
Projects
None yet
Development

No branches or pull requests

4 participants