Fixes for some servicing-proposed HttpClient Factory issues #2710
Conversation
|
Looks like this didn't make it into 3.1, is it coming soon? |
jamshedd
added
servicing-approved
and removed
servicing-consider
|
Any news when 3.1.1 might be released? |
|
These were approved for 3.1.2, so updating the milestone accordingly /cc @jamshedd |
|
@mkArtakMSFT, Will the fix in 3.1.2 also work for the combo of typed+named clients? services.AddHttpClient<MyHttpClient>("client1", config.GetSection("client1config"));
services.AddHttpClient<MyHttpClient>("client2", config.GetSection("client2config"));
services.AddHttpClient<MyHttpClient>("client3", config.GetSection("client3config"));
var httpClient = _clientFactory.CreateClient("client1"); // MyHttpClient using client1config |
|
Nowpe... I already tried this out myself. Seems like .net core 3 only
relies on the type that is provided.
…On Thu, Dec 19, 2019 at 1:14 PM John Korsnes ***@***.***> wrote:
Will the fix also work for the combo of typed+named clients?
services.AddHttpClient<MyHttpClient>("client1", config.GetSection("client1config"));services.AddHttpClient<MyHttpClient>("client2", config.GetSection("client2config"));services.AddHttpClient<MyHttpClient>("client3", config.GetSection("client3config"));
var httpClient = _clientFactory.CreateClient("client1"); // MyHttpClient using client1config
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
<#2710>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/ABNK6BATSTLVCJ6C7GQQYGDQZNQQZANCNFSM4JRBQBHA>
.
|
|
Proposal: Add validation to the validation to prevent mistakes. |
|
Moved to Mar as per Tactics. Will need to be explicitly approved for Feb, if required. |
|
Pulling back into 3.1.2. |
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
@mkArtakMSFT @rynowak - can we get someone to review this and get it merged? |
|
Has this been reviewed offline? If it's ready, please someone to sign off then get it in before 4PM today 😃 |
|
Is there a preview-nuget we can install to test this? I did not get a response on this comment, which works in 2.1, but is a regression in 3.0/3.1. Reading the commits, it still looks like there is validation around |
Fixes: #2077 In 3.0 we introduced validation to try and prevent some cases of invalid usage on the factory that had lead to user bug reports. Unfortunately we blocked a few legitimate usage scenarios behind expections. In this case the common usage is for a library to register a typed client with `AddHttpClient<MyClient>(...)`. User code can then collaborate by calling the same thing, and interacting with the builder that's returned. This change explicitly allows this pattern by fine-tuning the validation.
Fixes: dotnet/aspnetcore#13346 In 3.0 we added validation to try and report exceptions for some common HttClient factory usage mistakes in the registration code. Unfortunately we blocked from legitimate usage cases. In this case, users are blocked from associating multiple types with the same logical 'name'. Ex: ```C# services.AddHttpClient("Foo").AddTypedClient<A>().AddTypedClient<B>(); ``` This is useful and should be allowed because it's a good way to DRY up configuration code. ---- This change relaxes the validation when `AddTypedClient` is called, but not when `AddHttpClient` is called without supplying a name. We still want to block cases like the following: ```C# services.AddHttpClient<A.SomeClient>(); services.AddHttpClient<B.SomeClient>(); ``` The type short name is used as the logical name for the client (named options) so usage like this is always a bug.
Also reported as part of #2077 In this pattern users register many instances of the same client with different configurations, and multiplex between them in a round-robin fashion.
rynowak
force-pushed
the
rynowak/http-client-factory-fixes
branch
from
6b412ee
to
edcf5f2
Compare
|
Has this change been released? Calling I'm using Microsoft.Extensions.Hosting 3.1.7, if that's the relevant package. |
|
Hi, it looks like you are posting on a closed issue/PR/commit! We're very likely to lose track of your bug/feedback/question unless you:
Thanks! |
Description
In 3.0 we added validation to HttpClient Factory's registration code path to prevent some common mistakes. This new validation code blocks some valid use cases that we didn't know about.
Customer Impact
dotnet/aspnetcore#13346 Attempting to call
AddTypedClient<T>twice on the same builder with different types throws an exception. This is a valuable shorthand way of expressing a configuration once and then binding it to multiple types.#2077 Attempting to call
AddHttpClient<T>twice with the sameTthrows an exception. This pattern is used when both library code and user code want to collaborate on configuration of a client type.Regression?
Yes, this is a regression from 2.2
Risk
Low. This add special casing to our validation code path to allow more patterns. The impact of this is that cases that are explicitly blocked by an exception in 3.0 (but were allowed in 2.2) will be allowed again.