Proper implementation of async retry method

Question

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.");

I would recommend using Polly. It allows different retry policies and handles exceptions, timeouts etc all via simple configuration. — Neil
– Neil, Commented Jul 7, 2020 at 19:35
If you want code to be reviewed then codeReview stackexchange is probably a better place: codereview.stackexchange.com — JonasH
– JonasH, Commented Jul 7, 2020 at 19:36
Thank you, but I want to understand if this method is implemented correctly — Alexander
– Alexander, Commented Jul 7, 2020 at 19:37
in the first line you are both declaring a variable and at the same time using it in the assigment to the new variable, which would not compile. Apart from it, the method looks ok. As an alternative to using Stopwatch class for measuring elapsed time, you might also use just plain DateTime.UtcNow comparison. — Martin Staufcik
– Martin Staufcik, Commented Jul 7, 2020 at 19:40

Theodor Zoulias · Accepted Answer · 2020-07-07 21:08:39Z

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.

Collectives™ on Stack Overflow

Proper implementation of async retry method

1 Answer 1

Comments

Your Answer

Linked

Hot Network Questions

Collectives™ on Stack Overflow

1 Answer 1

Comments

Your Answer

Sign up or log in

Post as a guest

Linked

Related