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 🙂