Validate parameters in async method

Starting from C# 7.0, you can use use local function to reduce noise in code but still be compliant with argument check practice from sonar rule S4457. For example, this code will throw an ArgumentNullException in both cases: if you call it with await or without.

private Task WaitSeconds(int? durationInSeconds)
{
    if(durationInSeconds == null) throw new ArgumentNullException(nameof(durationInSeconds));
    async Task WaitSecondsInternal()
    {
        await Task.Delay(TimeSpan.FromSeconds(durationInSeconds.Value));
    }
    return WaitSecondsInternal();
}

As per sonar rule S4457

Because of the way async/await methods are rewritten by the compiler, any exceptions thrown during the parameters check will happen only when the task is observed. That could happen far away from the source of the buggy code or never happen for fire-and-forget tasks.

Therefore it is recommended to split the method into two: an outer method handling the parameter checks (without being async/await) and an inner method to handle the iterator block with the async/await pattern.

This rule raises an issue when an async method throws any exception derived from ArgumentException and contains await keyword.

Noncompliant Code Example

public static async Task SkipLinesAsync(this TextReader reader, int linesToSkip) // Noncompliant
{
    if (reader == null) { throw new ArgumentNullException(nameof(reader)); }
    if (linesToSkip < 0) { throw new ArgumentOutOfRangeException(nameof(linesToSkip)); }

    for (var i = 0; i < linesToSkip; ++i)
    {
        var line = await reader.ReadLineAsync().ConfigureAwait(false);
        if (line == null) { break; }
    }
}

---

Compliant Solution

public static Task SkipLinesAsync(this TextReader reader, int linesToSkip)
{
    if (reader == null) { throw new ArgumentNullException(nameof(reader)); }
    if (linesToSkip < 0) { throw new ArgumentOutOfRangeException(nameof(linesToSkip)); }

    return reader.SkipLinesInternalAsync(linesToSkip);
}

private static async Task SkipLinesInternalAsync(this TextReader reader, int linesToSkip)
{
    for (var i = 0; i < linesToSkip; ++i)
    {
        var line = await reader.ReadLineAsync().ConfigureAwait(false);
        if (line == null) { break; }
    }
}

That depends a bit on when you want the error to be raised - i.e. eagerly, or as part of the awaitable. As with iterator blocks, if you want eager error checks, you need two methods, for example:

public Task<int> SomeMethod(..args..) {
    if(..args fail..) throw new InvalidOperationException(...);
    return SomeMethodImpl(...args...);
}
private async Task<int> SomeMethodImpl(...args...)
{
    ... await etc ...
}

This will then perform any argument checking as part of the initial call, not the awaitable. If you want the exception to be part of the awaitable, you can just throw it:

public async Task<int> SomeMethod(..args..) {
    if(..args fail..) throw new InvalidOperationException(...);
    ... await etc ...
}

However, in your example, the fact that you are returning a Task suggests that this is not actually an async method, but is an async (but not async) method. You can't just do:

return new Task(() => { throw new ArgumentNullException("argument"); });

because that Task will never have been started - and never will be. I suspect you would need to do something like:

try {
    throw new InvalidArgumentException(...); // need to throw to get stacktrace
} catch(Exception ex) {
    var source = new TaskCompletionSource<int>();
    source.SetException(ex);
    return source.Task;
}

which is... a bit of a mouthful and could probably be encapsulated a bit better. This will return a Task that indicates it is in the Faulted state.

Simply throw it like you did in the sync method, the TPL has various mechanisms in place to re-throw exception, for example when you read the .Result property or the access .Exception property.