Using HttpClient in the synchronous class library

Question

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:

Using async/await all the way down to the client project
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)?

"First one would be less reusable than having synchronous methods"... I would argue fairly strongly that this is not true. Somewhere along the line you are doing some IO operations, async/await is definitely the easiest way to handle this without blocking. Otherwise you will be left having to either write it "APM style" and build your state machine manually or block on the IO. — Scott Perham
– Scott Perham, Commented Jan 28, 2018 at 18:22
HttpClient is an async API and meant to be used asynchronously. Long story short is that you should not use it async all the way down and avoid blocking synchronous calls or risk deadlocks. — Nkosi
– Nkosi, Commented Jan 28, 2018 at 18:25
@ScottPerham As I see it now, that approach would make every system, which depends on such lib, fully async or they will be dealing with the exact same problem that I am trying to find solution for. — gosferano
– gosferano, Commented Jan 28, 2018 at 18:27
Yes, but then they have the option to either use "async all the way down" (as McGuireV10 said), wrap it in a Task.Run, call Wait(), ... which one to use depends on the use case. As a library author you need to be consistent and using a "known" asynchronous concept such as Task is a good way to go (in my opinion of course!) — Scott Perham
– Scott Perham, Commented Jan 28, 2018 at 18:41
@gosferano Let's put it this way: If you go async only and a user of your library uses Wait() and they get a deadlock, that's their fault. But if you use Wait() inside your library and there's a deadlock, that's your fault. :) — Todd Menier
– Todd Menier, Commented Jan 28, 2018 at 19:05

Todd Menier · Accepted Answer · 2018-01-28 18:58:48Z

2

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:

Support async only.
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.

answered Jan 28, 2018 at 18:58

Todd Menier

39.7k18 gold badges156 silver badges185 bronze badges

Sign up to request clarification or add additional context in comments.

2 Comments

gosferano Over a year ago

I fully understand that. That's why I'm making asynchronous calls when requesting multiple URLs. My concern is, that it will also make my application, also fully async, while not interacting with HttpClient directly, and only via wrapper methods. But maybe that's just my faulty thinking.

Todd Menier Over a year ago

To do it right means that if your wrapper methods are making async calls, then those wrapper methods themselves must be async as well. That is, return a Task or Task<T>, and to follow the standard pattern, their names should end in Async. For example, if you have a wrapper method that gets a Thing by ID, then that method's signature should be public async Task<T> GetThingAsync(id) (or similar). That's a user-friendly library - experienced devs will know exactly how to call that.

McGuireV10 · Accepted Answer · 2018-01-28 18:21:39Z

0

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) { ... }

answered Jan 28, 2018 at 18:21

McGuireV10

10.1k5 gold badges54 silver badges65 bronze badges

1 Comment

McGuireV10 Over a year ago

I'm said the same thing Todd said in the answer you accepted: async is the right way to do it.

Collectives™ on Stack Overflow

Using HttpClient in the synchronous class library

2 Answers 2

2 Comments

1 Comment

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

2 Answers 2

2 Comments

1 Comment

Your Answer

Sign up or log in

Post as a guest

Related