Alexander
Alexander

Reputation: 43

Proper implementation of async retry method

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

Answers (1)

Theodor Zoulias
Theodor Zoulias

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

Related Questions