C# Exceptions In ContinueWith With Class Extension

I created a class extension for manage the exceptions with ContinueWith.

public static class ClassExtensions
{
    public static Task<T> ValidateInnerException<T>(this Task<T> task) => task.Exception != null ? throw task.Exception.InnerException ?? task.Exception : task;
}

There are two possible implementations using this class extension. The difference is the position of the ValidateInnerException call.

public Task<Response> ValidateCall(long periodId)
{
   return _client.Validate(periodId)
    .ContinueWith(t =>
    {
       return t.ValidateInnerException().Result.Adapt<Response>();
    });
}

And

public Task<Response> ValidateCall(long periodId)
{
   return _client.Validate(periodId)
    .ValidateInnerException()
    .ContinueWith(t =>
    {
        return t.Result.Adapt<Response>();
    });
}

The two implementations pass the Unit Test related with throw exception

[Fact]
public async Task Validate_Throw_Exception()
{
    _ = _client
       .Setup(x => x.Validate(It.IsAny<long>()))
       .ThrowsAsync(new Exception("Exception"));

    _ = await Assert.ThrowsAsync<Exception>(() => _manager.ValidateCall(1));
}

The first implementation has a problem, the Sonar marks it as an Code Smell. enter image description here

My questions are:

Thanks,

Upvotes: 1

Views: 100

Answers (1)

Johnathan Barclay
Johnathan Barclay

Reputation: 20353

Here Sonar knows 100% that t is a completed Task, and there is no issue accessing Result on a completed Task:

return _client.Validate(periodId)
    .ValidateInnerException()
    .ContinueWith(t =>
    {
        return t.Result.Adapt<Response>();
    });

That's because ContinueWith is only invoked on completion.

In this example, Sonar is not so sure, because ValidateInnerException is a Task-returning method, and that Task is not necessarily complete:

public Task<Response> ValidateCall(long periodId)
{
   return _client.Validate(periodId)
    .ContinueWith(t =>
    {
       return t.ValidateInnerException().Result.Adapt<Response>();
    });
}

That would be a code-smell, because it's potentially blocking.

Whilst in practice calling ValidateInnerException() here will always return a complete Task (if no exception is thrown) the tool clearly isn't able to determine that.

Upvotes: 1

Related Questions