Putting HttpClientFactory back on the radar

[abelbraaksma]

Is your feature request related to a problem? Please describe.

As described before in #1918 (specifically, this comment: #1918 (comment)), HttpClientFactory should be supported after dropping backwards compat support for .NET Framework 4.5, as described in that article.

Describe the solution you'd like

Refactor the code such that now we'd use an HttpClientFactory under-the-hood. This also adds the ability to provide (in the near future) DI middleware points and perhaps other extension-abilities.

Anyway, the main goal should first be to use an http client factory.

Describe alternatives you've considered

There are no alternatives, other than forking this codebase.

Additional context

Original issue: #1918
Dependency injection request: #1882

PS: I am willing to put in the work for this, however, I'd first like to have consensus on getting this in ;).

[pakrym-stripe]

I agree that integrating with HttpClientFactory is a great idea. I don't think we should do it in the main Stripe.net package. Rather we should have a separate package that integrates Stripe with Microsoft.Extensions and ASP.NET core.

We have a prototype of one at https://github.com/cecilphillip-stripe/stripe-dotnet-extensions/tree/main/src but never got to productizing it.

It is possible to have StripeClient use the HttpClientFactory without having client directly taking dependency on HttpClientFactory.

Example:
https://github.com/cecilphillip-stripe/stripe-dotnet-extensions/blob/main/src/Stripe.Extensions.DependencyInjection/ServiceCollectionExtensions.cs#L50

I don't think we'll take a dependency on Microsoft.Extensions.Http package as that force a lot of Microsoft.Extensions.* abstractions (DI, Logging, etc) on our users for this.

[abelbraaksma]

@pakrym-stripe, thanks for this. I didn't consider this possibility, tbh, and yes, that will allow us to use the HttpClientFactory with the current Stripe.NET implementation.

From your referenced code:

var clientFactory = s.GetRequiredService<IHttpClientFactory>();
var systemHttpClient = new SystemNetHttpClient(
    httpClient: clientFactory.CreateClient(HttpClientName),
    maxNetworkRetries: stripeOptions.MaxNetworkRetries,
    appInfo: stripeOptions.AppInfo,
    enableTelemetry: stripeOptions.EnableTelemetry);

Also, I wasn't aware of the telemetry options. That'll be very useful!!!

[AraHaan]

Also there are problems with that as well.

For starters what can be problematic:

• projects that use the main Stripe.net package in .NET Framework 4.7.2 with ASP.NET MVC which does not support DI, or any of the Microsoft.Extensions packages unlike the ASP.NET Core counterpart.
• Would force projects to migrate to Core which simply can't because they cannot install the IIS extensions to allow running MVC code that is compiled targeting Core / .NET 6+ (because they lack administrator permissions on the webserver).

[RichardD2]

There's no need to drop .NET Framework support to add this; HttpClientFactory is supported in .NET Standard 2.0, and hence .NET Framework 4.6.2+. You just need to reference the Microsoft.Extensions.Http NuGet package.

[AraHaan]

There's no need to drop .NET Framework support to add this; HttpClientFactory is supported in .NET Standard 2.0, and hence .NET Framework 4.6.2+. You just need to reference the Microsoft.Extensions.Http NuGet package.

None of the Microsoft.Extensions packages including the Microsoft.Extensions.Http NuGet package does not integrate well with ASP.NET Framework MVC applications that many colleges in the United States uses for students due to the limitations they put in place where they do not allow Core to be used on their servers for student developer web applications.

As much as I hate the fact that Microsoft does not shove the ASP.NET Core insertion package for IIS for servers even for them as well that would bypass their group policy on it 😂.

[RichardD2]

None of the Microsoft.Extensions packages including the Microsoft.Extensions.Http NuGet package does not integrate well with ASP.NET Framework MVC applications

Not sure where you've got that idea from. I'm using that package in multiple .NET Framework MVC 5 projects without any issues.

[AraHaan]

None of the Microsoft.Extensions packages including the Microsoft.Extensions.Http NuGet package does not integrate well with ASP.NET Framework MVC applications

Not sure where you've got that idea from. I'm using that package in multiple .NET Framework MVC 5 projects without any issues.

In the nuget package link it mentions to use DI's IServiceCollection which for ASP.NET Framework is not really an option that I know of.

[RichardD2]

In the nuget package link it mentions to use DI's IServiceCollection which for ASP.NET Framework is not really an option that I know of.

It's not built-in, but it's not too difficult to implement.

For example, here's the code I'm using: NBS.Core.Web.Mvc.DependencyInjection

[IanKemp]

This really needs to be implemented. For example, because the SDK is directly creating its own HttpClient instances, there is none of the per-request logging that HttpClientFactory brings.

I don't think we'll take a dependency on Microsoft.Extensions.Http package as that force a lot of Microsoft.Extensions.* abstractions (DI, Logging, etc) on our users for this.

Anyone not already using these abstractions is doing so because their codebase is ancient. This isn't Stripe's problem, and nor should it be a problem for the rest of us who are working on codebases from this century. If you aren't willing to keep your codebase somewhat up to date with industry standards, you shouldn't be working in this industry.

None of the Microsoft.Extensions packages including the Microsoft.Extensions.Http NuGet package does not integrate well with ASP.NET Framework MVC applications that many colleges in the United States uses for students due to the limitations they put in place where they do not allow Core to be used on their servers for student developer web applications.

Again, not Stripe's problem that American colleges are over a decade behind the curve.

[AraHaan]

This rally needs to be implemented. For example, because the SDK is directly creating its own HttpClient instances, there is none of the per-request logging that HttpClientFactory brings.

I don't think we'll take a dependency on Microsoft.Extensions.Http package as that force a lot of Microsoft.Extensions.* abstractions (DI, Logging, etc) on our users for this.

Anyone not already using these abstractions is doing so because their codebase is ancient. This isn't Stripe's problem, and nor should it be a problem for the rest of us who are working on codebases from this century. If you aren't willing to keep your codebase somewhat up to date with industry standards, you shouldn't be working in this industry.

None of the Microsoft.Extensions packages including the Microsoft.Extensions.Http NuGet package does not integrate well with ASP.NET Framework MVC applications that many colleges in the United States uses for students due to the limitations they put in place where they do not allow Core to be used on their servers for student developer web applications.

Again, not Stripe's problem that American colleges are over a decade behind the curve.

It is not just that, people tend to have requirements where they must only use code that comes from companies like Stripe and Microsoft as prime examples due to an enforced and strict security requirement.

[IanKemp]

It is not just that, people tend to have requirements where they must only use code that comes from companies like Stripe and Microsoft as prime examples due to an enforced and strict security requirement.

So how does Stripe taking a dependency on Microsoft's libraries cause a problem?