2

I have a static helper method class and at the base of it, I have this async method:

private static async Task<string> SendRequest(HttpMethod method, string apiUrl, string json, ILogger logger)
{
    try
    {
        using var request = new HttpRequestMessage()
        {
            Method = method,
            RequestUri = new Uri(apiUrl, UriKind.Absolute),
        };

        if (!string.IsNullOrWhiteSpace(json))
        {
            request.Content = new StringContent(json, Encoding.UTF8, "application/json");
            request.Content.Headers.ContentType.MediaType = MediaTypeNames.Application.Json;
        }

        using var client = new HttpClient();
        HttpResponseMessage result = await client.SendAsync(request);

        if (result.IsSuccessStatusCode)
        {
            return await result.Content.ReadAsStringAsync();
        }
        else
        {
             return $"{apiUrl}, {result.StatusCode}: Error from {method} request";
        }
    }
        catch (Exception ex)
    {
        logger.LogCritical(ex, $"{method} request failed: {apiUrl}");

        return $"{apiUrl}, {ex.Message}";
    }
}

Does it matter if the methods that call this are async or not it seems I can do either of the following:

internal static Task<string> SendPutRequest(string apiUrl, string json, ILogger logger)
{
    return SendRequest(HttpMethod.Put, apiUrl, json, logger);
}

or

internal static async Task<string> SendPutRequest(string apiUrl, string json, ILogger logger)
{
    return await SendRequest(HttpMethod.Put, apiUrl, json, logger);
}

It has no difference to my end call which is in an async method and calls await SendPutRequest so I was wondering if it there was any point if the SendPutRequest was marked as async or not.

Ivan Gechev
  • 706
  • 3
  • 9
  • 21
Pete
  • 57,112
  • 28
  • 117
  • 166
  • 2
    https://blog.stephencleary.com/2016/12/eliding-async-await.html – Hans Kesting Oct 24 '22 at 14:58
  • 2
    I'd be more interested in the serious bug in this code: `using var client = new HttpClient();` HttpClient is thread-safe and *meant* to be reused. Disposing it like this leaks sockets. This is explained in [You're using HttpClient wrong and its destabilizing your software](https://www.aspnetmonsters.com/2016/08/2016-08-27-httpclientwrong/). Most of the things done by this method are the defaults used by `HttpClient` or extension methods like [PostAsJsonAsync](https://learn.microsoft.com/en-us/dotnet/api/system.net.http.json.httpclientjsonextensions.postasjsonasync?view=net-7.0) – Panagiotis Kanavos Oct 24 '22 at 15:01
  • It would be better to use HttpClientFactory and typed HttpClient instances through [AddHttpClient](https://learn.microsoft.com/en-us/aspnet/core/fundamentals/http-requests?view=aspnetcore-6.0). This allows configuring clients per type or name, caching *and* recycling them as needed to take care of DNS changes. It even allows easy retrying through Polly retry policies, without modifying the clients – Panagiotis Kanavos Oct 24 '22 at 15:03
  • @PanagiotisKanavos never knew that about the http client, it was really interesting, thanks. I've changed to using the factory and made the above into an extension method on the client (without the using) – Pete Oct 25 '22 at 09:37

0 Answers0