Async / Await Guidance: Intrinsic Signature Proliferation

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.

Jon Skeet
people
quotationmark

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.

people

See more on this question at Stackoverflow