Reputation: 43
Please tell me, is the asynchronous model correctly implemented in this method?
I need to implement a method that expects the condition to be met, with the possibility of a timeout.
FYI: there is no need to make a generic method
var timeout = timeout == default ? TimeSpan.FromSeconds(30) : timeout;
var stopwatch = new Stopwatch();
stopwatch.Start();
var delayTimeout = 0;
do
{
delayTimeout += 1000;
var report = await MethodAsync();
if (report.Active == true)
{
return report;
}
await Task.Delay(delayTimeout);
}
while (stopwatch.Elapsed < timeout);
throw new TimeoutException($"Timeout.");
Upvotes: 0
Views: 253
Reputation: 43384
The retry method could benefit from a CancellationToken
argument, so that is can be canceled at any moment. Also the last Task.Delay
should be omitted in case the timeout condition has been met. No need to delay the throwing of the TimeoutException
for some extra seconds without reason.
private async Task<Report> MethodWithRetryAsync(TimeSpan timeout = default,
CancellationToken cancellationToken = default)
{
timeout = timeout == default ? TimeSpan.FromSeconds(30) : timeout;
var stopwatch = Stopwatch.StartNew();
int delayTimeout = 0;
while (true)
{
var report = await MethodAsync();
if (report.Active == true) return report;
delayTimeout += 1000;
if (stopwatch.Elapsed + TimeSpan.FromMilliseconds(delayTimeout) > timeout)
{
throw new TimeoutException();
}
await Task.Delay(delayTimeout, cancellationToken);
}
}
Another issue is that in case of an exception in the awaiting of MethodAsync
, the exception will be propagated instantly and no retry will be attempted. This may be desirable or not, depending on the case at hand.
You could also add an optional argument maxRetries
to the method, to allow the caller to limit the number of retries. Also parameterizing the initialDelay
and the delayIncreament
would make the method more flexible, at the cost of making the API more verbose.
Upvotes: 1