gosferano
gosferano

Reputation: 35

Using HttpClient in the synchronous class library

I am writing a class library wrapper for a 3rd party API, using .Net Standard and later on I'm planning to use this wrapper in my other project. While looking around the web, I've found out that general concern is that one should use HttpClient class for making HTTP requests.

I am aware of two approaches that I could take:

  1. Using async/await all the way down to the client project
  2. Using Task.Wait() method

So far I am going for the 1st approach. But both approaches seem rather problematic. First one would be less reusable than having synchronous methods, which return their type and not a Task object. I'd rather take the second approach, but it's prone to deadlocks.

My code for making a single request (HttpClient initialized with parameterless constructor):

    protected async Task<JObject> Request(string url)
    {
        Uri uri = BuildUrl(url);
        HttpResponseMessage response = await HttpClient.GetAsync(uri);
        if (response.IsSuccessStatusCode)
        {
            string result = await response.Content.ReadAsStringAsync();
            return JObject.Parse(result);
        }

        return new JObject();
    }

For multiple requests:

    protected async Task<JObject[]> Request(IEnumerable<string> urls)
    {
        var requests = urls.Select(Request);
        return await Task.WhenAll(requests);
    }

And usage in the wrapper class:

    protected async Task<JObject> RequestGet(string id, bool byUrl)
    {
        if (IsBulk && !byUrl)
            return await Request($"{Url}?id={id}");

        if (byUrl)
            return await Request(Url + id);

        return await Request(Url);
    }

How could I modify the code (1st and 2nd snippet) so that it wouldn't cause any deadlocks and async usage in every caller method (3rd snippet)?

Upvotes: 1

Views: 2468

Answers (2)

Todd Menier
Todd Menier

Reputation: 39289

Using HttpClient synchronously is unsupported and deadlock prone, and there's nothing you can do (including using Task.Wait) to change that fact. You have 2 options:

  1. Support async only.

  2. Use the older WebRequest APIs instead and support synchronous calls that way.

I would opt for option 1. There's a reason that the latest and greatest HTTP libraries don't even support synchronous I/O anymore. The newer wave of multi-core processors and distributed architectures have triggered a paradigm shift in the programming world, where tying up threads while waiting on I/O (especially longer-running network calls like HTTP) is a total waste of resources. And when languages like C# provide incredible support for async programming out of the box, there's virtually no good reason to do synchronous HTTP anymore. Most developers understand this I don't think you should be concerned about abandoning users by going async only. Instead, encourage them to get up to speed on doing it the right way.

Upvotes: 2

McGuireV10
McGuireV10

Reputation: 9926

Using async all the way down is the right way, and you can return results from a Task method (in other words -- as your own code shows -- it isn't right to say they don't return anything other than Task):

public async Task<string> GetString(int value)
{
    return value.ToString();
}

Obviously that doesn't need to be async, it doesn't need to await anything, but the point is Task can return anything thanks to generics. You can even use the fancy new C# 7 value-tuples:

public async Task<(string name, int age)> GetUserInfo(int userId) { ... }

Upvotes: 0

Related Questions