StrongGrid icon indicating copy to clipboard operation
StrongGrid copied to clipboard

Refactor to use HttpClientFactory for .NET 8+ targets

Open jmbryan4 opened this issue 4 months ago • 5 comments

Migrate from Pathoschild.Http.Client.FluentClient to the native .NET HttpClientFactory pattern with resilience policies for .NET 8+ target frameworks

Reasons:

  • HttpClientFactory provides better connection pooling and resource management
  • Native integration with Microsoft.Extensions.Http.Resilience for retry policies and circuit breakers
  • Reduce external dependencies and align with modern .NET patterns

jmbryan4 avatar Aug 13 '25 17:08 jmbryan4

There are two concerns in this issue you are raising. The first concern is: you think we should stop using Pathoschild.Http.Client.FluentClient and the second concern is: you think we should use.NET HttpClientFactory. These are two distinct issues that need to be addressed separately. We could decide to move forward with one or the other, or both (or neither!). My point is: they are independent from each other.

Let me address the first one: discontinue using Pathoschild.Http.Client.FluentClient. Hard no. There are so many nice features provided by this library. I have zero intention to discontinue using this library. I just want to be honest with you that this library is deeply entrenched into StrongGrid and the cost of switching to plain HttpClient would be extremely high, I have no desire to take on this monumental task. And for what benefit? Removing one NuGet reference? That does not justify the effort in my eyes. Additionally, I have contributed to the FluentClient project, several features were added for our benefit. All this to say that I have zero intention to switch away.

Regarding using HttpClientFactory: you make a good point that developers should be mindful to avoid socket exhaustion. They should not instantiate a new instance of the StrongGrid client with every request; they should instead reuse the same instance for the duration of their application. This is, incidentally, the same recommendation for Microsoft's HttpClient. All this to say that developers should avoid constantly instantiating StrongGrid clients. Also, I want to point out that StrongGrid client has a constructor that allows you to provide your own HttpClient. This means that you can manage the lifetime of the http client by yourself. You can use the HttpClientFactory to get a client from the pool and pass this instance to the StrongGrid client. In this scenario, StrongGrid does not instantiate a new Http client which puts you in control of managing the pool of http clients as you see fit.

Having said all this, if we decide to move forward with using httpfactory, how do you envision this integration? What do you think an overload of the client's constructor like this:

public Client(string apiKey, IHttpClientFactory httpClientFactory, StrongGridClientOptions options = null, ILogger logger = null)

Jericho avatar Aug 14 '25 00:08 Jericho

@Jericho First, thank you for your time and for maintaining this library!

I agree that I raised 2 points in my issue:

  1. remove Pathoschild.Http.Client.FluentClient for net8.0 target
  2. enable easy use of HttpClientFactory for StrongGrid client

1 will be more difficult to accomplish. For 2, I don't think many changes are needed (if any). An update to the ReadMe could be done to show a recommended way to register the client for DI and/or some helper methods to make it easier for consumers of this library.

var builder = WebApplication.CreateSlimBuilder(args);
builder.Services.AddHttpClient("StrongGrid").AddStandardResilienceHandler();
builder.Services.AddScoped<StrongGrid.Client>(sp =>
{
    // linked by the "StrongGrid" name from the AddHttpClient above
    var httpClient = sp.GetRequiredService<IHttpClientFactory>().CreateClient("StrongGrid");
    var apiKey = builder.Configuration["SendGrid:ApiKey"];
    var options = new StrongGridClientOptions() { LogLevelFailedCalls = LogLevel.Error, LogLevelSuccessfulCalls = LogLevel.Debug };
    var logger = sp.GetRequiredService<ILogger<StrongGrid.Client>>();
    return new StrongGrid.Client(apiKey, httpClient, options, logger);
});
var app = builder.Build();

example extension that could be added:

public static class StrongGridServiceCollectionExtensions
{
  // with just API key
    public static IHttpClientBuilder AddStrongGrid(this IServiceCollection services,
        string apiKey, string httpClientName = "StrongGrid")
    {
        var httpClientBuilder = services.AddHttpClient(httpClientName);
        services.AddScoped<StrongGrid.Client>(sp =>
        {
            var httpClient = sp.GetRequiredService<IHttpClientFactory>().CreateClient(httpClientName);
            return new StrongGrid.Client(apiKey, httpClient);
        });
        return httpClientBuilder;
    }

    public static IHttpClientBuilder AddStrongGrid(this IServiceCollection services,
        string apiKey, StrongGridClientOptions? options = null, string httpClientName = "StrongGrid")
    {
        var httpClientBuilder = services.AddHttpClient(httpClientName);
        services.AddScoped<StrongGrid.Client>(sp =>
        {
            var httpClient = sp.GetRequiredService<IHttpClientFactory>().CreateClient(httpClientName);
            var logger = sp.GetRequiredService<ILogger<StrongGrid.Client>>();
            var clientOptions = options ?? new StrongGridClientOptions();
            return new StrongGrid.Client(apiKey, httpClient, clientOptions, logger);
        });
        return httpClientBuilder;
    }
}

// usage:
var apiKey = builder.Configuration["SendGrid:ApiKey"];
var options = new StrongGridClientOptions()
{
    LogLevelFailedCalls = LogLevel.Error,
    LogLevelSuccessfulCalls = LogLevel.Debug
};
builder.Services.AddStrongGrid(apiKey);
// or:
builder.Services.AddStrongGrid(apiKey, options).AddStandardResilienceHandler();

For 1. Would you be open to exploring a multi-targeting approach that drops Pathoschild.Http.Client.FluentClient for net8.0 target, but keeps it for the existing?

Existing targets (net48, netstandard2.1) would continue using FluentClient exactly as today with no changes.

Even though you provide the constructor that accepts an HttpClient, there's still a concern with transitive dependencies from Pathoschild.Http.Client.FluentClient. It depends on the legacy Microsoft.AspNet.WebApi.Client → Newtonsoft.Json, Newtonsoft.Json.Bson. These assemblies get included in the deployment even if unused and add some package bloat and potential version conflicts.

approach could be something like this in csproj

<TargetFrameworks>net48;netstandard2.1;net8.0</TargetFrameworks>

<!-- move Pathoschild.Http.FluentClient from shared to here  -->
<ItemGroup Condition="'$(TargetFramework)' != 'net8.0'">
  <PackageReference Include="Pathoschild.Http.FluentClient" Version="4.4.1" />
</ItemGroup>

<ItemGroup Condition="'$(TargetFramework)' == 'net8.0'">
  <PackageReference Include="Microsoft.Extensions.Http" Version="8.0.0" />
  <PackageReference Include="Microsoft.Extensions.Http.Resilience" Version="8.0.0" />
  <PackageReference Include="Microsoft.Extensions.DependencyInjection.Abstractions" Version="8.0.0" />
</ItemGroup>

Unfortunately, I think NET8_0_OR_GREATER preprocessor directives would need to be added throughout the library. It could be done per file.

// StrongGrid/
// ├── BaseClient.cs                    (net48, netstandard2.1)
// ├── BaseClient.Net8.cs               (net8.0+ only)
// ├── Client.cs                        (net48, netstandard2.1) 
// ├── Client.Net8.cs                   (net8.0+ only)
// ├── Resources/
// │   ├── Mail.cs                      (net48, netstandard2.1)
// │   ├── Mail.Net8.cs                 (net8.0+ only)
// │   └── ... (other resources)
// └── Extensions/
//     └── extension methods for easy DI registration of the client (net8.0+ only)

// ========================================
// BaseClient.cs (unchanged from original)
// ========================================
#if !NET8_0_OR_GREATER

using Pathoschild.Http.Client;
using Pathoschild.Http.Client.Extensibility;
using StrongGrid.Json;
// .. etc.

namespace StrongGrid
{
    /// <summary>
    /// Base class for StrongGrid's REST clients.
    /// </summary>
    public abstract class BaseClient : IBaseClient, IDisposable
    {
        // ... exact same implementation as the original file
    }
}

#endif

// ========================================
// BaseClient.Net8.cs (New - .NET 8+ only)
// ========================================
#if NET8_0_OR_GREATER

// usings

namespace StrongGrid
{
    /// <summary>
    /// Base class for StrongGrid's REST clients (.NET 8+ implementation).
    /// </summary>
    public abstract class BaseClient : IBaseClient, IDisposable
// net8 specific implementation

jmbryan4 avatar Aug 14 '25 05:08 jmbryan4

Regarding issue number 2: I really like the extension methods you wrote. Do you mind if I copy/paste them into the StrongGrid library and make them available to other developers? I will also write a short paragraph in the readme to explain how to use them and ask you to review it.

Regarding issue number 1: I really, really, really have no appetite for removing FluentHttpClient. I still don't see the benefit and/or the problem it would solve and I really, really, really don't want to sign up for refactoring our library; the amount of work is just too great. Your suggestion to add #if ... #else ... #end in the "resource" classes essentially means that the amount of code would nearly double in those files which honestly sounds like a maintenance nightmare. I appreciate you bringing this up and I am enjoying the discussion but the amount of work is simply too overwhelming for too little benefit. Like I said before: hard no.

Besides, the FluentHttpClient library has been great for us: it has simplified how we dispatch requests, it provides automatic retries when we trigger "TOO MANY REQUESTS", simplifies error detection, automatic serialization of JSON in requests, automatic parsing of JSON in response payload, easy logging of diagnostic info about each request/response, etc. I can't remember when exactly I started using FluentHttpClient but it's probably been close to a decade at this point. I have collaborated with its author (@pathoschild) on numerous occasions over the years to suggest improvements, fix issues, etc and he's been great to work with.

Having said all this:

  • I wish @pathoschild would modernize the list of frameworks that his library targets. This would potentially get rid of some of the legacy packages such as WinInsider.System.Net.Http.Formatting for example. This is something I brought up to him back in 2020 and again in the fall 2024 but so far he hasn't been open to my suggestion. For instance, his library still targets net452 (among others). This particular framework has been out of support since 2016!
  • I also wish he would decouple JSON serialization from his main library because we don't use his built-in Newtonsoft-based serialization. I switched to System.Text.Json serialization/deserialization in 2021.

If these two improvements were implemented in FluentHttpClient, I think it would take care of your concerns regarding bloat and potential version conflicts.

Jericho avatar Aug 15 '25 00:08 Jericho

Yes, please do add the extension methods!

I started an example of how someone might use them here: https://github.com/jmbryan4/StrongGridExampleApi/blob/main/src/StrongGridMinimalApi/Extensions/ServiceCollectionExtensions.cs

I would be happy to help review any code changes or readme changes.

I understand your point on FluentHttpClient. It's probably a change that the FluentHttpClient maintainer should consider. I think at this point in 2025 that library can probably just target netstandard2.0 and maybe latest .NET LTS target to drop anything legacy.

jmbryan4 avatar Aug 15 '25 05:08 jmbryan4

Hi! I'm always open to suggestions for FluentHttpClient. That said, the design philosophy for the package is:

  1. It just wraps the official packages with a fluent interface, so it's always up-to-date with the latest improvements and patterns.
  2. It just works regardless of your .NET version or platform (e.g. you're not stuck using manual WebClient in legacy code).

So for the topics in this thread:

  • Since it's just a fluent wrapper around HttpClient, all the official patterns work with it (including HttpClientFactory and resilience policies). We could certainly improve the README or add extensions to make that clearer if needed.

  • The dependencies for old .NET versions shouldn't matter, since they don't apply to newer projects. For example, note that this .NET 8 project doesn't reference any of the legacy packages:

    Can you clarify why the dependencies for old .NET versions are an issue here? (Here's our last discussion about it for reference.)

  • The Microsoft.AspNet.WebApi.Client package isn't legacy so far as I know. It's the latest official package which extends HttpClient with features like content negotiation. I guess we could reimplement those features without using the official package, but that'd go against the design philosophy I mentioned above.

  • That previous point makes removing Json.NET from FluentHttpClient tricky though, since the dependency is coming from the official package. If they dropped it from the official package, I'd have no objections to updating the fluent client accordingly.

Pathoschild avatar Aug 18 '25 16:08 Pathoschild