AG0019 - Do not await what does not need to be awaited

[jbizkit]

For example in the code below, the await _service.FetchSomethingAsync() expression forces compiler to create the state machine that’s behind this feature.

public class Something : ISomething
{
	readonly IService _service;

	public Something(IService service)
	{
		_service = service;
	}

	public async Task<string> GetSomethingAsync() => await _service.FetchSomethingAsync();
}

It can be easily fixed by

public class Something : ISomething
{
	readonly IService _service;

	public Something(IService service)
	{
		_service = service;
	}

	public Task<string> GetSomethingAsync() => _service.FetchSomethingAsync();
}

For reference: Link with measurements

[szaboopeeter]

While the rule itself sounds, I probably wouldn't want this to be enforced as a blocker. You're right that in the quoted case - and in general pass-through cases await can be omitted without issues (though there's something about weird exception stack traces, I think GetSomethingAsync() will not be appear if you omit await which can make tracking down issues a little bit harder).

But the bigger issue in my point of view is for the analyser to decide the cases when it's safe to omit. I guess we can start with very simple cases... But as soon as you do anything else than simple passthrough, you should also await. Consider:

public Task<string> GetSomethingAsync(string numericString) => _service.FetchSomethingAsync(Int32.Parse(numericString));

The parsing can throw so you probably want the await here.

Also need to keep in mind that it's very easy to forget to add await in when you're doing a change and your method gets more complicated. I'd say the overhead of the async statemachine will be insignificant in 99.9% of our cases, so we need to compare the risk against the benefit.

[inemtsev]

I don't think we raised this issue because of statemachine impact, but because it takes away the ability of the caller to decide when to await. Taking away opportunities to parallelize api/database calls. But, I agree that this should be a recommendation and not a blocker.

[szaboopeeter]

@inemtsev Not sure if I get what you mean by

it takes away the ability of the caller to decide when to await

The caller still has control, it can await the returned async call even in an async chain whenever it wants. Parallelization should not be a problem; as long as you're using async through the whole call-chain you can still do something like this:

public async Task Main() {
    var dbTask = GetDataFromDatabase();
    var apiTask = GetDataFromApi();
    await Task.WhenAll(dbTask, apiTask);
    /* ... */
}

public async Task<int> GetDataFromDatabase => await GetFromDatabaseImplementation(); // Internally awaits for DB

public async Task<int> GetDataFromApi => await GetFromApiImplementation(); // Internally awaits for API

Or did I completely miss the point you are trying to make?

[inemtsev]

@szaboopeeter Never mind that point, you are correct. I forgot that await does not block in outer context.