Async / Await Guidance: Intrinsic Signature Proliferation

Question

Consider the following class responsible for communicating with an API using the .NET HttpClient.

public class ApiServiceAgent
{
    public async Task<string> GetItem()
    {
        var client = new HttpClient();
        var uri = new Uri("http://stackoverflow.com");
        var response = await client.GetAsync(uri);
        return await response.Content.ReadAsStringAsync();
    }
}

Whilst simplified it is representative of how I have been writing code that communicates with HTTP APIs.

Now consider that I wish to consume this class within my own Web API project. For simplicity sake lets say I have two additional classes that (directly or otherwise) call this class.

• A Repository - responsible for my calling my Service Agent, reading the results and returning some form of Domain object.
• A Controller - An ASP.NET Web API controller - the entry point to my API and responsible for returning data to the calling API user.

Those two classes could look like this:

public class Controller : ApiController
{
    public async Task<string> Get()
    {
        var repository = new Repository();
        return await repository.GetItem();
    }
}

public class Repository
{
    public async Task<string> GetItem()
    {
        var serviceAgent = new ApiServiceAgent();
        var apiResponse = await serviceAgent.GetItem();
        //For simplicity/brevity my domain object is a lowercase string
        return apiResponse.ToLower();
    }
}

In the above two classes async / Task method signatures have quite intrinsically propagated all the way to the Controller... something quite common in most sample .NET code you see on the subject of async/await.

However in most async / await "best practice" articles / videos you see at the moment it is stressed that async / await should only be used for truly asynchronous operations.

I would argue that neither the Repository or the Controller do anything truly asynchronous - that is all done in the Service Agent.

Should I be preventing async / await from becoming so prolific in my code?

Would the following be more representative of "best practice" async / await?

public class Controller : ApiController
{
    public string Get()
    {
        var repository = new Repository();
        return repository.GetItem();
    }
}

public class Repository
{
    public string GetItem()
    {
        var serviceAgent = new ApiServiceAgent();
        var apiResponseTask = serviceAgent.GetItem();
        var apiResponse = apiResponse.GetAwaiter().GetResult();
        return apiResponse.ToLower();
    }
}

Am I way off-base? and if so... please do point me in the right direction.

Answer 1

I'd argue that those methods are asynchronous - but only by virtue of the API part. That asynchrony just propagates its way up.

Your second implementation of Repository.GetItem() is problematic, IMO:

• You should not call an awaiter's GetResult() method before the awaiter has completed. In fact, the only time it makes sense to use an awaiter directly in your own code is when you're implementing your own awaiter, in my view. (And that should be very rare.) In this case I think it'll do the same as using Task<T>.Result, but it's possible that the task awaiter validates that the task has completed (or faulted) before the call is made.
• You should not be using a blocking call on the API response to turn the asynchronous API into a synchronous API without a great deal of care. Depending on the exact nature of the code, it can very easily lead to deadlock... if that blocking call is made in a single-thread synchronization context and the asynchronous operation needs to get back to the same synchronization context, you'll end up with your code waiting for the task to complete, and the task waiting for the thread to become available.

That suggests that your methods should have an Async suffix, by the way - but it doesn't mean that they need to be async methods. The second probably does, but the first can be written simply as:

public class Controller : ApiController
{
    public Task<string> GetAsync()
    {
        var repository = new Repository();
        return repository.GetItemAsync();
    }
}

There's rarely a benefit from making a method async if it's just got a single await which is used directly for the return value.

For your Repository method, the await would be useful as you've got code to run after that completes - you could use ContinueWith directly, but that would be more painful. So I'd leave that as the async/await version, just renaming it to GetItemAsync... and possibly using ConfigureAwait(false) to indicate that you don't actually need that method to come back in the same context.

Answer 2

Should I be preventing async / await from becoming so prolific in my code?

No. I think your initial attempt is on the right track. If you invoke and asynchronous method and need to do something with the result, then using await is the best way to do that. async\await tend to propagate up the call stack like that.

However async\await has a cost, in that a state machine is generated for every async method. If you don't need the result of the asynchronous operation but are simply returning it to the caller, then you can just return the result of the called async method to avoid generating the state machine.

For example:

public class Repository
{
    public Task<string> GetItem()
    {
        var serviceAgent = new ApiServiceAgent();

        // Don't need to do anything with the result of GetItem, just return the task.
        return serviceAgent.GetItem();

        //var apiResponse = await serviceAgent.GetItem();
        //For simplicity/brevity my domain object is a lowercase string
        //return apiResponse.ToLower(); 
    }
}

Of course if you need to do something with the result of GetItem before returning it then the method has to be async.

However in most async / await "best practice" articles / videos you see at the moment it is stressed that async / await should only be used for truly asynchronous operations.

I think usually this advice is given to discourage developers from creating asynchronous methods with names ending in Async that simply call Task.Run or Task.Factory.StartNew inside. This is deceptive as it can start a new thread, whereas truly asynchronous operations do not start a new thread. This is what makes asynchrony so desirable, you can handle more asynchronous operations with fewer threads.

Would the following be more representative of "best practice" async / await?

No. Calling apiResponse.GetAwaiter().GetResult(); will block the calling thread until the result is available and it can cause a deadlock on the GUI thread.