Use HttpClientFactory

[vertonghenb]

Currently we're using using statements to create HttpClients. It would be better/less socket intensive to use a Static HttpClient or HttpClientFactory.

As a first issue, while this class is disposable, using it with the using statement is not the best choice because even when you dispose HttpClient object, the underlying socket is not immediately released and can cause a serious issue named ‘sockets exhaustion’. For more information about this issue, see You're using HttpClient wrong and it's destabilizing your software blog post.

Therefore, HttpClient is intended to be instantiated once and reused throughout the life of an application. Instantiating an HttpClient class for every request will exhaust the number of sockets available under heavy loads. That issue will result in SocketException errors. Possible approaches to solve that problem are based on the creation of the HttpClient object as singleton or static, as explained in this Microsoft article on HttpClient usage.

But there’s a second issue with HttpClient that you can have when you use it as singleton or static object. In this case, a singleton or static HttpClient doesn't respect DNS changes, as explained in this issue at the .NET Core GitHub repo.

https://docs.microsoft.com/en-us/dotnet/standard/microservices-architecture/implement-resilient-applications/use-httpclientfactory-to-implement-resilient-http-requests

[GordonBlahut]

Looking into adding HttpClientFactory support but I want to better understand your code base first.

1. Is there a reason some methods use GetHttpResponseMessage and others use ExecuteRequest. Is this a refactoring that was never completed for all methods?
2. I also see a lot of non-awaited async calls e.g. res.Content.ReadAsStringAsync().Result instead of await res.Content.ReadAsStringAsync(). Is there a reason these aren't awaited?

[perfahlen]

@GordonBlahut, awesome and thanks in advance.

1. No, no refactorization made. ExecuteRequest makes all the GET requests and GetHttpResponseMessage POST, PUT, DELETE etc. The two methods should be merged into one if possible.

2. No, not really. That should be done in any case. Thanks for finding that!