Code review #2 – remember about your awaits

This is another post about great code review feedback, that I either gave or received. It will always consist of 3 parts: context, review feedback and explanation. You can go ahead and read the previous ones here: https://www.michalbialecki.com/2019/06/21/code-reviews/. This time I’m going to show you a very simple bug I found.

The context

The problem occurred when I was investigating a code with multiple available calls. And after some refactoring, it came down to something like this.

public async Task InsertUser()
{
    try
    {
        // async stuff here

        client.PostAsync("http://localhost:49532/api/users/InsertMany", null).ConfigureAwait(false);

        // more async cally 
    }
    catch (Exception e)
    {
        Console.WriteLine(e);
        throw;
    }
}

The thing was, that my call to InsertMany endpoint was not working somehow, and I didn’t catch any exceptions in this code. Do you know what’s wrong with it?

Review feedback

Oh, you haven’t awaited this one!

Explanation

That’s correct. There is a try-catch block that should catch every exception, but when async call is not awaited, then a Task is returned. So as a result no call would be made. This is potentially dangerous, because code will compile. IDE will give you a hint, but it can be easily overlooked.

As a good practice you can check if a call was a success:

public async Task InsertUser()
{
    try
    {
        // async stuff here

        var response = await client.PostAsync("http://localhost:49532/api/users/InsertMany", null).ConfigureAwait(false);
        response.EnsureSuccessStatusCode();

        // more async cally 
    }
    catch (Exception e)
    {
        Console.WriteLine(e);
        throw;
    }
}

So take a good care about you async / await calls 🙂

Leave a Reply

Your email address will not be published. Required fields are marked *