HttpClientFactory Keyed DI docs #44533
Conversation
dotnet-policy-service
bot
added
dotnet-fundamentals/svc
dotnet-networking/subsvc
labels
Acrolinx score (91, 94)
|
Co-authored-by: Genevieve Warren <[email protected]>
Co-authored-by: Anton Firszov <[email protected]>
|
@antonfirsov thanks for the review! I will address the remaining suggestions in a follow-up PR if you don't mind 🙏 we need the doc to be live to link from the blogpost |
|
|
||
| ## Background | ||
|
|
||
| `IHttpClientFactory` and Named `HttpClient` instances, unsurprisingly, align well with the Keyed Services idea. Historically, among other things, `IHttpClientFactory` was a way to overcome this long-missing DI feature. But plain Named clients require you to obtain, store, and query the `IHttpClientFactory` instance—instead of injecting a configured `HttpClient`—which might be inconvenient. While Typed clients attempt to simplify that part, it comes with a catch: Typed clients are easy to [misconfigure](httpclient-factory-troubleshooting.md#typed-client-has-the-wrong-httpclient-injected) and [misuse](httpclient-factory.md#avoid-typed-clients-in-singleton-services), and the supporting infrastructure can also be a tangible overhead in certain scenarios (for example, on mobile platforms). |
There was a problem hiding this comment.
-
I'd avoid feeling words like "unsurprisingly" here, and I saw "nasty" somewhere later in the doc.
-
What does this has to do with Keyed DI:
and the supporting infrastructure can also be a tangible overhead in certain scenarios (for example, on mobile platforms).
???
| {"name":"runtime","url":"https://api.github.com/repos/dotnet/runtime"} | ||
| ``` | ||
|
|
||
| In the example, the configured `HttpClient` is injected into the request handler through the standard Keyed DI infrastructure, which is integrated into ASP.NET Core parameter binding. For more information on Keyed Services in ASP.NET Core, see [Dependency injection in ASP.NET Core](/aspnet/core/fundamentals/dependency-injection#keyed-services). |
There was a problem hiding this comment.
What does "request handler" mean here? It's confusing because usually the handlers are put inside the client, not the other way around. So either I'm missing something or it's used here to mean something else, which I'm also missing.
| Additionally, the Scoped lifetime of the clients can help catch cases of captive dependencies: | ||
|
|
||
| ```csharp | ||
| services.AddHttpClient("scoped").AddAsKeyed(); |
There was a problem hiding this comment.
I know scoped is default, but I'd use it here explicitly or used a different name. Some people might get confused and think that the string "scoped" has any meaning appart from just being the name.
| .AddAsKeyed(ServiceLifetime.Singleton); | ||
| ``` | ||
|
|
||
| If you call `AddAsKeyed()` within a Typed client registration, only the underlying Named client is registered as Keyed. The Typed client itself continues to be registered as a plain Transient service. |
There was a problem hiding this comment.
This is not related to the previous example, is it? It took me a while to understand what it's saying, so maybe another small example wouldn't hurt.
| > | ||
| > —the `HttpClient` instance becomes _captive_, and will likely outlive its expected `HandlerLifetime`. `IHttpClientFactory` has no control over captive clients, they're NOT able to participate in the handler rotation, and it can result in [the loss of DNS changes](httpclient-factory-troubleshooting.md#httpclient-doesnt-respect-dns-changes). A similar issue [already exists](httpclient-factory.md#avoid-typed-clients-in-singleton-services) for Typed clients, which are registered as Transient services. | ||
|
|
||
| In cases when client's longevity can't be avoided—or if it's consciously desired, for example, for a Keyed Singleton—it's advised to [leverage `SocketsHttpHandler`](httpclient-factory.md#using-ihttpclientfactory-together-with-socketshttphandler) by setting `PooledConnectionLifetime` to a reasonable value. |
There was a problem hiding this comment.
What does "reasonable" mean? "Small enough value to observe and react to DNS changes regularly"...
Also, I'd add a link to PooledConnectionLifetime API ref docs.
| > `KeyedService.AnyKey` registrations define a mapping from _any_ key value to some service instance. However, as a result, the Container validation doesn't apply, and an _erroneous_ key value _silently_ leads to a _wrong instance_ being injected. | ||
|
|
||
| > [!IMPORTANT] | ||
| > For Keyed `HttpClient`s, a mistake in the client name can result in erroneously injecting an "unknown" client—meaning, a client whose name was never registered. |
There was a problem hiding this comment.
I feel like you started overusing — instead of splitting into 2 sentences, or just using comma.
| services.AddHttpClient("known", /* ... */); | ||
|
|
||
| provider.GetRequiredKeyedService<HttpClient>("known"); // OK | ||
| provider.GetRequiredKeyedService<HttpClient>("unknown"); // OK (unconfigured instance) |
There was a problem hiding this comment.
This is very similar to the example from the beginning of the document and I'd explicitly call out the difference and why it doesn't throw here.
|
|
||
| ### "Opt-in" strategy considerations | ||
|
|
||
| Even though the "global" opt-in is a one-liner, it's unfortunate that the feature still requires it, instead of just working "out of the box." For full context and reasoning on that decision, see [dotnet/runtime#89755](https://github.com/dotnet/runtime/issues/89755) and [dotnet/runtime#104943](https://github.com/dotnet/runtime/pull/104943). In short, the main blocker for "on by default" is the `ServiceLifetime` "controversy": for the current (`9.0.0`) state of the DI and `IHttpClientFactory` implementations, there's no single `ServiceLifetime` that would be reasonably safe for all `HttpClient`s in all possible situations. There's an intention, however, to address the caveats in the upcoming releases, and switch the strategy from "opt-in" to "opt-out". |
There was a problem hiding this comment.
- Quotes for global are unnecessary. It's not so-called global, it is global.
- Remove unfortunate.
|
|
||
| Even though the "global" opt-in is a one-liner, it's unfortunate that the feature still requires it, instead of just working "out of the box." For full context and reasoning on that decision, see [dotnet/runtime#89755](https://github.com/dotnet/runtime/issues/89755) and [dotnet/runtime#104943](https://github.com/dotnet/runtime/pull/104943). In short, the main blocker for "on by default" is the `ServiceLifetime` "controversy": for the current (`9.0.0`) state of the DI and `IHttpClientFactory` implementations, there's no single `ServiceLifetime` that would be reasonably safe for all `HttpClient`s in all possible situations. There's an intention, however, to address the caveats in the upcoming releases, and switch the strategy from "opt-in" to "opt-out". | ||
|
|
||
| ## How to: Opt out from keyed registration |
There was a problem hiding this comment.
You just said that it's opt-in, so opting out might not make sense here.
I'd describe scenarios when one would want to use that.
|
|
||
| 1. If called for the same name, the last setting wins: the lifetime from the last `AddAsKeyed()` is used to create the Keyed registration (unless `RemoveAsKeyed()` was called last, in which case the name is excluded). | ||
| 2. If used only within `ConfigureHttpClientDefaults`, the last setting wins. | ||
| 3. If both `ConfigureHttpClientDefaults` and specific client name were used, all defaults are considered to "happen" before all per-name settings. Thus, defaults can be disregarded, and the last of the per-name settings wins. |
Moving all the details from the blogpost to the dedicated conceptual doc.
cc @dotnet/ncl @ManickaP
Internal previews
IHttpClientFactory