Allow HttpClient to be injected

[eikster-dk]

Hello :-)

I was wondering if it was possible to make the HttpClient Injectable or at least make it possible so an IHttpClientFactory can be in control of the creation of the HttpClient.

Reasoning for this is due to the connection limits within an azure functions project. Here we need to control how the HttpClients are instantiated to avoid hitting the limit. Since resources are shared among the functions within a function app, we get closer to the limit each time a service is created (since it creates it's own httpClient).

So when you use this awesome library to make multiple calls to multiple shopify shops, we get closer and closer to hitting that connection limit that is set by Azure..

I hope that the problem makes sense, otherwise I can try my best to answer any questions that may arise :-)

Further reading:
https://docs.microsoft.com/en-us/azure/azure-functions/manage-connections
https://docs.microsoft.com/en-us/azure/azure-functions/functions-dotnet-dependency-injection

[clement911]

I'm not familiar with Azure functions, but we discussed a related matter here: #360
For sure, I think it would be useful to be able to customize http client

[dkershner6]

I have had no trouble with ShopifySharp's HttpClient in azure functions, it only creates one per instance for me (same as your injection would).

This is likely still a good idea, but I don't think it will solve your connection limit issues.

I would add that I make very heavy use of functions and have had all sorts of connection related issues, but never ShopifySharp related.

[nozzlegear]

Yes, I 100% want to add support for the HttpClientFactory! I don't personally need it myself but we've had a lot of requests for it. I've got a few pending changes that I'm working on right now, but I'm going to try to get it done this week.

[eikster-dk]

@dkershner6 It depends, I guess. If I understand the code correctly, you create one instance pr. service you have.. One service is connected to one shopify shop..

We are working on a MVP where we will be handling around 500-1500 shops. So that could potentially mean we would hit the conenction limit very fast..

For example, we have a reconciliation job that run every day, to make sure we received all events from the given webhooks.. We do this from a single function app, that function app shares the resources of the connection pool, so my guess would be that we would have failed reconciliation jobs due to conection limits.

[clement911]

Currently the HttpCclient is static so there is only 1 instance.

[clement911]

ShopifySharp/ShopifySharp/Services/ShopifyService.cs

Line 25 in 30a1928

private static HttpClient _Client { get; } = new HttpClient();

[eikster-dk]

Ahh yes! Sorry, my .NET is a little bit rusty when it comes to abstract classes and field initialization. So I guess this won't affect a function app's connection limits, since there is only 1 instance.

[dkershner6]

1 PER instance, but yes, this is the recommended pattern in Azure Functions (and elsewhere).

As a note, if you are running that many shops, you should prep to either split into multiple function apps or host in a more scalable way than consumption.

OR, you can just have it run slowly, synchronously.

[KorsG]

We ran into an issue today which seems to be caused by the 1 static instance of HttpClient, since it does not handle DNS changes (e.g. when Shopify changes the IP address of a shop).

By default the HttpClient does not respect DNS changes (https://nima-ara-blog.azurewebsites.net/beware-of-the-net-httpclient/), and since we running .NET 5 we cant set a global timeout value via ServicePointManager:
ServicePointManager.FindServicePoint(shopifyUrl).ConnectionLeaseTimeout = (int)TimeSpan.FromMinutes(1).TotalMilliseconds;

The solution for us would be to provide our own HttpClient with a SocketsHttpHandler and PooledConnectionLifetime set to an appropriate value, but since it is a private readonly property it is not possible - not even with reflection since it has no setter.

As a "quick-fix" I suggest one of the following:

1. Change the property to be public and settable
   public static HttpClient _Client { get; set; } = new HttpClient();

2. Change the property to have a private setter (then we can use reflection to set it)
   private static HttpClient _Client { get; private set; } = new HttpClient();

Would you be open to either of those? 🙂

[nozzlegear]

@KorsG Thanks for the report! I wasn't aware of any DNS issues but it makes sense. I think the easiest solution is to make the client mutable by the developer in some way. I'm working on ShopifySharp right now so I'll add a method to modify the HttpClient in the next release.

[nozzlegear]

Actually I think I'll go ahead and (finally) implement the IHttpClientFactory support while I'm at it. My plan is to add a ShopifyService.SetGlobalHttpClientFactory method and an instance-specific service.SetHttpClient method. So pretty similar to what we do with the execution policies.

Here's what I'm thinking:

public class StaticHttpClientFactory : IHttpFactory
{
    // Use a static HTTP Client
	private static HttpClient _Client = new HttpClient();
	
	public HttpClient CreateClient (string name)
	{
		return _Client;
	}
}

public abstract class ShopifyService
{
	// ...
	private static IHttpClientFactory _ClientFactory = new StaticHttpClientFactory();
	
	private HttpClient _Client = _ClientFactory.CreateClient("something");
	
	public static void SetGlobalHttpClientFactory(IHttpClientFactory factory)
	{
		_ClientFactory = factory;
	}
	
	public void SetHttpClient(HttpClient client)
	{
		_Client = client;
	}
}

@KorsG would this solve your issue?

[KorsG]

@nozzlegear I only hoped for a quick-fix, but this is awesome and would definitely solve our issue! 👍 many thanks 😃

[nozzlegear]

With ShopifySharp v5.14.0, it's now possible to set a global IHttpClientFactory and to change the instance-specific HttpClient. Thanks for the feedback everyone!