CarterCommunity / Carter

Carter is framework that is a thin layer of extension methods and functionality over ASP.NET Core allowing code to be more explicit and most importantly more enjoyable.
MIT License
2.05k stars 172 forks source link

Registering modules as Singleton is causing problems #279

Closed enriquein closed 2 years ago

enriquein commented 2 years ago

I'm currently trying to upgrade a web app to use .Net 6 and the pre-release version of Carter. Upgrading was pretty much painless, and the new API is pretty cool, however the changes to module DI registration introduced in 632c7c71cb3125fc6b5fed32e3204bb96a88dc21 are causing me startup problems when a ICarterModule instance needs to request a scoped dependency.

The error I get is:

Unhandled exception. System.AggregateException: Some services are not able to be constructed (Error while validating the service descriptor 'ServiceType: Carter.ICarterModule Lifetime: Singleton ImplementationType: MyServer.Modules.MyModule': Cannot consume scoped service 'MyServer.SomeScopedService' from singleton 'Carter.ICarterModule'.)

Is the idea now that all dependencies will need to be specified as part of the function that handles the request (ie: the RequestDelegate) instead of the module constructor? Or did I just miss an alternate way to register my modules for my use-case?

I guess as as workaround I can just skip calling AddCarter and register my modules, validators, and response negotiators by hand. But would be very helpful to know the official stance on this.

Thanks!

jchannon commented 2 years ago

For .NET 6 dependencies should be in the RequestDelegate function and no dependencies in the Carter module constructor. There was no api I could make to force this for users unfortunately. I mean you could use singleton dependencies injected into the constructor but my best practice recommendation would be to always pass in dependencies into your route functions.

Personally whilst this may result in more classes and interfaces which I don’t see as a problem you can get rid of repositories and have query interfaces/classes that might only be used for one route. It has a clear separation of concerns.

On Thu, 30 Sep 2021 at 22:35, Enrique Ramirez @.***> wrote:

I'm currently trying to upgrade a web app to use .Net 6 and the pre-release version of Carter. Upgrading was pretty much painless, and the new API is pretty cool, however the changes to module DI registration introduced in 632c7c7 https://github.com/CarterCommunity/Carter/commit/632c7c71cb3125fc6b5fed32e3204bb96a88dc21 are causing me startup problems when a ICarterModule instance needs to request a scoped dependency.

The error I get is:

Unhandled exception. System.AggregateException: Some services are not able to be constructed (Error while validating the service descriptor 'ServiceType: Carter.ICarterModule Lifetime: Singleton ImplementationType: MyServer.Modules.MyModule': Cannot consume scoped service 'MyServer.SomeScopedService' from singleton 'Carter.ICarterModule'.)

Is the idea now that all dependencies will need to be specified as part of the function that handles the request (ie: the RequestDelegate) instead of the module constructor? Or did I just miss an alternate way to register my modules for my use-case?

I guess as as workaround I can just skip calling AddCarter and register my modules, validators, and response negotiators by hand. But would be very helpful to know the official stance on this.

Thanks!

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/CarterCommunity/Carter/issues/279, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAAZVJT6DFU2DID5M3QFG43UETJ2PANCNFSM5FDIV3NA .

enriquein commented 2 years ago

Looking at the new APIs I was afraid this was going to be the answer. Thanks for clarifying!

jchannon commented 2 years ago

No problem but note that although the modules are registered as singleton the request delegates are not so your scoped dependencies should work as expected

On Thu, 30 Sep 2021 at 22:52, Enrique Ramirez @.***> wrote:

Looking at the new APIs I was afraid this was going to be the answer. Thanks for clarifying!

— You are receiving this because you commented.

Reply to this email directly, view it on GitHub https://github.com/CarterCommunity/Carter/issues/279#issuecomment-931731082, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAAZVJVWKNV5THBWKQILKX3UETL2TANCNFSM5FDIV3NA .