simpleinjector / SimpleInjector

An easy, flexible, and fast Dependency Injection library that promotes best practice to steer developers towards the pit of success.
https://simpleinjector.org
MIT License
1.21k stars 154 forks source link

RE: DelegatingHandler disposable transient error #967

Open legion-gaming opened 1 year ago

legion-gaming commented 1 year ago

This is in reference to: https://github.com/simpleinjector/SimpleInjector/issues/183 posted back in 2016 that was closed by saying the solution was to register a handler as a singleton.

There are multiple reasons why you should not do this, but one of them being that handlers can be configurable so that they can be re-used with multiple http clients and configured on a per-client basis. Consider a handler called OAuthHandler where the constructor takes in IOptionsSnapshot<OAuthHandlerOptions>. This allows me to have two different HttpClient instances that can both use the same handler type to authenticate with two different OAuth endpoints by configuring each handler per the client's needs.

Another reason has to do with DNS changes which was one of the driving forces that IHttpClientFactory was intended to solve.

I don't believe, however, that making an alteration to ignore the DelegatingHandler type from the AddTransient call is the answer, because there could also be other exceptions to this rule for other things within the framework. Maybe this is a configuration that SimpleInjector can read in to know that "Here's a list of exceptions to the rule for disposing transients." or something to that effect.

crate-scarter commented 1 year ago

I agree with this as well. We are running into a use case where we would like to provide a cloud provider handler to get information back for that specific cloud provider and being able to pass in the option on which cloud provider is being used for this HttpClient usage. Otherwise you would need a separate handler for each cloud provider to do the exact same function, which seems like a violation of DRY.