Reputation: 141
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.
My questions are:
Thanks,
Upvotes: 1
Views: 100
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