1

Wanted to verify if HttpCLient instance should be created outside method passed to polly for ExecuteAsync, or in?

My current usage varies between the two options and I am not sure which is the correct one?

Also, if it incurs some drawbacks, or possible memory leaks, etc. ?

Get:

var client = new HttpClient(new NativeMessageHandler()) { Timeout = new TimeSpan(0, 0, TimeOutSec) };

var httpResponse = await AuthenticationOnUnauthorizePolicy.ExecuteAsync(async () =>
{
    UpdateClientHeader(client, correlationId);
    return await client.GetAsync(url, token);
});

Post:

var httpResponse = await AuthenticationOnUnauthorizePolicy.ExecuteAsync(async () =>
{
    using (var client = new HttpClient(new NativeMessageHandler()) { Timeout = new TimeSpan(0, 0, TimeOutSec) })
    {
        UpdateClientHeader(client, correlationId);
        WriteNetworkAccessStatusToLog();
        return await client.PostAsync(url, content);
    }
});

The policy used here:

AuthenticationOnUnauthorizePolicy = Policy
     .HandleResult<HttpResponseMessage>(reposnse => reposnse.StatusCode == HttpStatusCode.Unauthorized)
     .RetryAsync(1, onRetryAsync:
     async (response, count, context) =>
     {
         _logger.Info("Unauthorized Response! Retrying Authentication...");
         await Authenticate();
     });

Appreciates any comments on the code above.

Is there a correct way?

Do I need to use the Context to get the client again, or is my usage okay?


Update:

Authenticate method:

public virtual async Task Authenticate()
{
    // lock it - only one request can request token
    if (Interlocked.CompareExchange(ref _isAuthenticated, 1, 0) == 0)
    {
        var result = new WebResult();
        var loginModel = new LoginModel
        {
            email = _settingService.Email,
            password = _settingService.Password
        };

        var url = ......
        var correlationId = Guid.NewGuid().ToString();

        try
        {
            var stringObj = JsonHelper.SerializeObject(loginModel);
            HttpContent content = new StringContent(stringObj, Encoding.UTF8, HttpConsts.JsonMediaType);

            using (var client = new HttpClient(new NativeMessageHandler()) { Timeout = new TimeSpan(0, 0, TimeOutSec) }
            )
            {
                UpdateClientHeader(client, correlationId, useToken: false); // not token, we need new one
                using (var httpResponse = await client.PostAsync(url, content))
                {
                    var sReader = await httpResponse.Content.ReadAsStringAsync();
                    await HandleRequest(result, sReader, httpResponse, correlationId, url, "result");
                }
            }
            if (result != null && !result.HasError)
            {
                _loginToken = result.Token;
            }
        }
        catch (Exception ex)
        {
            // Log error
        }
        finally
        {
            _isAuthenticated = 0;
        }
    }
}

Update client headr method:

if (_loginToken != null &&
                !client.DefaultRequestHeaders.Contains("Token"))
            {
                client.DefaultRequestHeaders.Add("Token", _loginToken );
            }
client.DefaultRequestHeaders.Accept.Add(new MediaTypeWithQualityHeaderValue(HttpConsts.JsonMediaType));
Peter Csala
  • 17,736
  • 16
  • 35
  • 75
o_w
  • 581
  • 3
  • 12
  • Can you please share with us the code of `Authenticate` and `UpdateClientHeader` as well? – Peter Csala Oct 18 '21 at 10:35
  • Updated with methods. – o_w Oct 18 '21 at 11:27
  • 1
    don't put a HttpClient in a using block. [It doesn't work that way](https://medium.com/@nuno.caneco/c-httpclient-should-not-be-disposed-or-should-it-45d2a8f568bc). Use a static instance. – JHBonarius Oct 18 '21 at 11:33
  • How do you get the `_loginToken` inside `UpdateClientHeader`? How is it updated? – Peter Csala Oct 18 '21 at 12:08
  • @JHBonarius Isn't static instance dangerous to use? Can't the client dispose after timeout or some error state, and I'll have to manage clients? create, remove, update and dispose them? – o_w Oct 19 '21 at 11:07
  • No, not for `HttpCient`. It's the suggested use as specified in [the MS docs](https://learn.microsoft.com/en-us/dotnet/api/system.net.http.httpclient) – JHBonarius Oct 19 '21 at 11:09
  • @PeterCsala I have edited the code to reflect this, sorry about that. – o_w Oct 19 '21 at 11:13
  • @JHBonarius Then what about thread safety, with only one instance? – o_w Oct 19 '21 at 11:17
  • Google it. e.g. [link](https://stackoverflow.com/a/11178252) – JHBonarius Oct 19 '21 at 11:27
  • @o_w Just to make sure that my understanding is correct: You want to have a `HttpClient` that is used against an API which requires the presence of a token. That token can expire / invalidated that's why you want to have a mechanism to request a new / refresh the token. You want to handle that with Polly. Is my understanding correct? – Peter Csala Oct 19 '21 at 15:24
  • @PeterCsala All of this coe was already present in the project before me. I am trying to understand if our use of HttpClient is correct, that is all. The Authentication part is less important in this context. – o_w Oct 21 '21 at 06:15
  • @JHBonarius Thank you, I'll check having my own pool of HttpClients for use across all requests. – o_w Oct 21 '21 at 06:17
  • @o_w If your question is concerned only about the HttpClient then please remove the `polly` tag and add `dotnet-httpclient` instead. I would also encourage you to read this excellent article (old but gold): [YOU'RE USING HTTPCLIENT WRONG AND IT IS DESTABILIZING YOUR SOFTWARE](https://www.aspnetmonsters.com/2016/08/2016-08-27-httpclientwrong/) – Peter Csala Oct 21 '21 at 10:09
  • @PeterCsala It still relates to Polly, since I don't know if including the client's creation in the return call method is crucial or not. – o_w Oct 24 '21 at 06:11
  • @o_w As it was already suggested you should not need to create a new HttpClient per request. You can and should reuse that client, which issues requests against the same domain. In other words you should have a client per domain. – Peter Csala Oct 25 '21 at 07:16
  • @PeterCsala What constitutes a domain in my case? GetAsync is one and SendAsync is one? Or is it by topic? – o_w Oct 25 '21 at 08:06
  • @o_w If you issue requests against `http://subdomainA.domain.com` and `http://subdomainB.domain.com` then they are under the same domain. If you issue requests against `http://domainA.com` and `http://domainB.com` then they are under different domains. The http VERB does not matter. – Peter Csala Oct 25 '21 at 08:14

0 Answers0