[API Proposal]: Make AddHttpClient() inject an ITypedHttpClientFactory into instantiated class

[Eli-Black-Work]

Background and motivation

We currently have some code like this:

UserService.cs

public class UserService : IUserService
{
    private readonly HttpClient _httpClient;
    
    public UserService(HttpClient httpClient)
    {
        _httpClient = httpClient;
    }
    
    public async Task Example()
    {
        await _httpClient.GetAsync("...");
    }
}

UserRepo.cs

public class UserRepo : IUserRepo
{
    private readonly IUserService _userService;

    /// <summary>
    /// Class that allows for managing target lists including CRUD and commands
    /// </summary>
    public Operator(IUserService userService)
    {
        _userService = userService;
    }
}

Startup.cs

services
    .AddHttpClient<IUserService, UserService>(httpClient =>
    {
        httpClient.BaseAddress = new Uri(Configuration["...."], UriKind.Absolute);
    })
    .ConfigurePrimaryHttpMessageHandler(sp =>
    {
        return new HttpClientHandler() {
            UseProxy = true,
            DefaultProxyCredentials = CredentialCache.DefaultCredentials
        };
    });

services.AddSingleton<IUserRepo, UserRepo>();

This is a problem, because while IUserService is transient, IUserRepo is a singleton, so the HttpClient that's passed to IUserService will never be refreshed.

One fix is to make IUserRepo also be transient, but this isn't always ideal.

API Proposal

I propose that calling AddHttpClient() should no only inject HttpClient but should also inject a new interface, ITypedHttpClientFactory. The user could then use ITypedHttpClientFactory to create new instances of HttpClient that have the settings that were configured via AddHttpClient().

The advantage is that service singleton services that wrap typed clients could remain singletons instead of needing to be reconfigured to be transient.

API Usage

UserService.cs

public class UserService : IUserService
{
    private readonly ITypedHttpClientFactory _httpClientFactory;
    
    public UserService(ITypedHttpClientFactory httpClientFactory)
    {
        _httpClientFactory = httpClientFactory;
    }
    
    public async Task Example()
    {
        // This HttpClient will have the BaseUrl, Proxy, etc. that was configured via the AddHttpService() call.
        using var httpClient = _httpClientFactory.CreateClient();
        
        await httpClient.GetAsync("...");
    }
}

UserRepo.cs (Same as above – no changes)

public class UserRepo : IUserRepo
{
    private readonly IUserService _userService;

    /// <summary>
    /// Class that allows for managing target lists including CRUD and commands
    /// </summary>
    public Operator(IUserService userService)
    {
        _userService = userService;
    }
}

Startup.cs (Same as above – no changes)

services
    .AddHttpClient<IUserService, UserService>(httpClient =>
    {
        httpClient.BaseAddress = new Uri(Configuration["...."], UriKind.Absolute);
    })
    .ConfigurePrimaryHttpMessageHandler(sp =>
    {
        return new HttpClientHandler() {
            UseProxy = true,
            DefaultProxyCredentials = CredentialCache.DefaultCredentials
        };
    });

services.AddSingleton<IUserRepo, UserRepo>();

Alternative Designs

No response

Risks

No response

[ghost]

Tagging subscribers to this area: @dotnet/ncl
See info in area-owners.md if you want to be subscribed.

Issue Details

---

Background and motivation

We currently have some code like this:

UserService.cs

public class UserService : IUserService
{
    private readonly HttpClient _httpClient;
    
    public UserService(HttpClient httpClient)
    {
        _httpClient = httpClient;
    }
    
    public async Task Example()
    {
        await _httpClient.GetAsync("...");
    }
}

UserRepo.cs

public class UserRepo : IUserRepo
{
    private readonly IUserService _userService;

    /// <summary>
    /// Class that allows for managing target lists including CRUD and commands
    /// </summary>
    public Operator(IUserService userService)
    {
        _userService = userService;
    }
}

Startup.cs

services
    .AddHttpClient<IUserService, UserService>(httpClient =>
    {
        httpClient.BaseAddress = new Uri(Configuration["...."], UriKind.Absolute);
    })
    .ConfigurePrimaryHttpMessageHandler(sp =>
    {
        return new HttpClientHandler() {
            UseProxy = true,
            DefaultProxyCredentials = CredentialCache.DefaultCredentials
        };
    });

services.AddSingleton<IUserRepo, UserRepo>();

This is a problem, because while IUserService is transient, IUserRepo is a singleton, so the HttpClient that's passed to IUserService will never be refreshed.

One fix is to make IUserRepo also be transient, but this isn't always ideal.

API Proposal

I propose that calling AddHttpClient() should no only inject HttpClient but should also inject a new interface, ITypedHttpClientFactory. The user could then use ITypedHttpClientFactory to create new instances of HttpClient that have the settings that were configured via AddHttpClient().

The advantage is that service singleton services that wrap typed clients could remain singletons instead of needing to be reconfigured to be transient.

API Usage

UserService.cs

public class UserService : IUserService
{
    private readonly ITypedHttpClientFactory _httpClientFactory;
    
    public UserService(ITypedHttpClientFactory httpClientFactory)
    {
        _httpClientFactory = httpClientFactory;
    }
    
    public async Task Example()
    {
        // This HttpClient will have the BaseUrl, Proxy, etc. that was configured via the AddHttpService() call.
        using var httpClient = _httpClientFactory.CreateClient();
        
        await httpClient.GetAsync("...");
    }
}

UserRepo.cs (Same as above – no changes)

public class UserRepo : IUserRepo
{
    private readonly IUserService _userService;

    /// <summary>
    /// Class that allows for managing target lists including CRUD and commands
    /// </summary>
    public Operator(IUserService userService)
    {
        _userService = userService;
    }
}

Startup.cs (Same as above – no changes)

services
    .AddHttpClient<IUserService, UserService>(httpClient =>
    {
        httpClient.BaseAddress = new Uri(Configuration["...."], UriKind.Absolute);
    })
    .ConfigurePrimaryHttpMessageHandler(sp =>
    {
        return new HttpClientHandler() {
            UseProxy = true,
            DefaultProxyCredentials = CredentialCache.DefaultCredentials
        };
    });

services.AddSingleton<IUserRepo, UserRepo>();

Alternative Designs

No response

Risks

No response

Author:	Bosch-Eli-Black
Assignees:	-
Labels:	api-suggestion, area-System.Net.Http, untriaged
Milestone:	-

[ghost]

Tagging subscribers to this area: @dotnet/ncl
See info in area-owners.md if you want to be subscribed.

Issue Details

---

Background and motivation

We currently have some code like this:

UserService.cs

public class UserService : IUserService
{
    private readonly HttpClient _httpClient;
    
    public UserService(HttpClient httpClient)
    {
        _httpClient = httpClient;
    }
    
    public async Task Example()
    {
        await _httpClient.GetAsync("...");
    }
}

UserRepo.cs

public class UserRepo : IUserRepo
{
    private readonly IUserService _userService;

    /// <summary>
    /// Class that allows for managing target lists including CRUD and commands
    /// </summary>
    public Operator(IUserService userService)
    {
        _userService = userService;
    }
}

Startup.cs

services
    .AddHttpClient<IUserService, UserService>(httpClient =>
    {
        httpClient.BaseAddress = new Uri(Configuration["...."], UriKind.Absolute);
    })
    .ConfigurePrimaryHttpMessageHandler(sp =>
    {
        return new HttpClientHandler() {
            UseProxy = true,
            DefaultProxyCredentials = CredentialCache.DefaultCredentials
        };
    });

services.AddSingleton<IUserRepo, UserRepo>();

This is a problem, because while IUserService is transient, IUserRepo is a singleton, so the HttpClient that's passed to IUserService will never be refreshed.

One fix is to make IUserRepo also be transient, but this isn't always ideal.

API Proposal

I propose that calling AddHttpClient() should no only inject HttpClient but should also inject a new interface, ITypedHttpClientFactory. The user could then use ITypedHttpClientFactory to create new instances of HttpClient that have the settings that were configured via AddHttpClient().

The advantage is that service singleton services that wrap typed clients could remain singletons instead of needing to be reconfigured to be transient.

API Usage

UserService.cs

public class UserService : IUserService
{
    private readonly ITypedHttpClientFactory _httpClientFactory;
    
    public UserService(ITypedHttpClientFactory httpClientFactory)
    {
        _httpClientFactory = httpClientFactory;
    }
    
    public async Task Example()
    {
        // This HttpClient will have the BaseUrl, Proxy, etc. that was configured via the AddHttpService() call.
        using var httpClient = _httpClientFactory.CreateClient();
        
        await httpClient.GetAsync("...");
    }
}

UserRepo.cs (Same as above – no changes)

public class UserRepo : IUserRepo
{
    private readonly IUserService _userService;

    /// <summary>
    /// Class that allows for managing target lists including CRUD and commands
    /// </summary>
    public Operator(IUserService userService)
    {
        _userService = userService;
    }
}

Startup.cs (Same as above – no changes)

services
    .AddHttpClient<IUserService, UserService>(httpClient =>
    {
        httpClient.BaseAddress = new Uri(Configuration["...."], UriKind.Absolute);
    })
    .ConfigurePrimaryHttpMessageHandler(sp =>
    {
        return new HttpClientHandler() {
            UseProxy = true,
            DefaultProxyCredentials = CredentialCache.DefaultCredentials
        };
    });

services.AddSingleton<IUserRepo, UserRepo>();

Alternative Designs

No response

Risks

No response

Author:	Bosch-Eli-Black
Assignees:	-
Labels:	api-suggestion, untriaged, area-Extensions-HttpClientFactory
Milestone:	-

[CarnaViire]

Thanks for the suggestion @Bosch-Eli-Black!
Typed Client not being refreshed in a singleton is a valid concern which pains me too.

ITypedHttpClientFactory<T> is actually already being registered, however, it is not doing what you'd like it to do -- it will require a pre-resolved HttpClient from HttpClientFactory to be passed inside, see ITypedHttpClientFactory.CreateClient(HttpClient).
One of the reasons for that is the variety of ways you can register your Typed Client, including binding it to an already registered Named Client by passing the name, see e.g. AddHttpClient<TClient>(IServiceCollection, String).

While it should be possible to extend ITypedHttpClientFactory<TClient> to have a method CreateClient without parameter, there is a downside -- It is a breaking change. If anyone used their own implementation of that interface, it will stop compiling.

P.S.: It would be even better to have IHttpClientFactory.CreateClient<TClient>(), but that's impossible while HttpClientFactory is a singleton. However, that may change in #47091.

[stijnherreman]

It would be even better to have IHttpClientFactory.CreateClient<TClient>(), but that's impossible while HttpClientFactory is a singleton.

@CarnaViire why's that? HttpClientFactory can resolve named clients, and aren't typed clients just named clients under the hood?

[CarnaViire]

HttpClientFactory can resolve named clients, and aren't typed clients just named clients under the hood?

It depends on what you call a Typed client. I would say Typed client is a user-defined transient service which would have an HttpClient automatically injected in constructor. That injected HttpClient - yes, under the hood it is a named client (while that's an implementation detail). But the Typed client can have and usually has additional dependencies beside HttpClient. And in case a Typed client is resolved within a scope, all it's dependencies should be resolved within that scope too. But you cannon capture a scope from within a singleton service. That's why singleton HttpClientFactory cannot create Typed clients as-is.

@stijnherreman does it make sense? Should I elaborate further?

[stijnherreman]

That makes sense yes. I assumed the hypothetical IHttpClientFactory.CreateClient<TClient>() would return a HttpClient, and I was only considering the case in the OP here (which matches my case).
Indeed when considering scoped dependencies, a possible solution will require more work.

[adcorduneanu]

What's the actual status of this one, as I'm facing the same performance issue?

As in the initial request, I need it to happen only once, and not every time, as at this point I'm paying a huge price when comes to initializing a new instance of the service.

[CarnaViire]

@adcorduneanu if the problem you are facing is the one where you need to inject Typed clients into a singleton, you can consider a workaround with SocketsHttpHandler and PooledConnectionLifetime. Basically, if you replace the factory's handler management by SocketsHttpHandler's connection pool management, that will make Typed clients safe to be injected into singletones:

services.AddHttpClient<MyTypedClient>()
    .ConfigurePrimaryHttpMessageHandler(() =>
    {
        return new SocketsHttpHandler()
        {
            PooledConnectionLifetime = TimeSpan.FromMinutes(2)
        };
    })
    .SetHandlerLifetime(Timeout.InfiniteTimeSpan); // Disable rotation, as it is handled by PooledConnectionLifetime

You can read a bit more in the docs and you can also check out this gist.

[gjermystreeva]

An alternative 'workaround' suggestion would be to register a Func<IUserService> and take a dependency on that in UserRepo, storing the Func<IUserService> rather than storing a (transient) IUserService.

I've put a gist up based on @CarnaViire gist above.

For me it seems (maybe?) cleaner to keep the registration of the typed client cleaner, and leaves it down to the user of the typed client whether they are also transient and take a dependency on the typed client directly, or need to be singleton and use as example below.

Its similar in concept to approach here, but without needing extra factory classes.

UserRepo.cs

public class UserRepo : IUserRepo
{
    private readonly Func<IUserService> _userServiceFunc;

    /// <summary>
    /// Class that allows for managing target lists including CRUD and commands
    /// </summary>
    public UserRepo(Func<IUserService> userServiceFunc)
    {
        _userServiceFunc = userServiceFunc;
    }
    
    public DoSomething()
    {
        # Create a IUserService typed http client for each request
        _ = await this._userServiceFunc().Example();
    }
}

Startup.cs

    # all the code in original example plus...

    services.AddSingleton<Func<IUserService>>(sp => () => sp.GetRequiredService<IUserService>());

It could be all that is needed to be considered a 'fix' is AddHttpClient to register the 'factory' function also (plus docs). In fact I'm pretty sure running in Azure Functions the DI already can resolve a Func<TClient>, but would need to double check.

Any thoughts or feedback?

[chunick]

@CarnaViire Would the workaround using SocketsHttpHandler and PooledConnectionLifetime cause any issues if the typed client was injected into a short-lived service as opposed to a singleton? I would like to create an extension method to use in place of all calls to AddHttpClient that will be safe for both singletons and short-lived services.

[CarnaViire]

@chunick the workaround would work for scoped and transient services as well.

The change essentially makes the handler to have a "singleton" DI lifetime. Meaning that the same handler instance will be reused in all instances of typed clients of a specific type.