Polly: Retry only if a certain condition has been met after some time has elapsed

[codingmoh]

Hello there,

Unfortunately, we're dealing with a legacy system (via http), that behaves unstable and nondeterministic, i.e. it could happen, that internal server errors or similar ones occure, but a request managed to perform the intended operation, after some time has elapsed.

So what we are trying to do, is to setup a retry policy, which, in case of any error response,
waits a certain amount of time and then checks ( orderCreated() ) whether the initial operation did go through.
If the order was not successful (orderCreated() returns false), and __only then__ retry again.
If the ordered was successfully created, (orderCreated() returns true) we are done, no retries.

So what we are trying to achieve: In case of any expected error, wait some time, check whether a condition is met, if not, retry again, otherwise continue.

I am wondering whether this is the correct way of dealing with our issue. I have already written some unit tests and in seems to work as expected. Are there any issues, we should consider? Are we messing things up? What do you guys think?

Thank you so much for your help!

bool orderCreated()
{
       // everytime 
    Thread.Sleep(20 * 1000);
    return GetOrderId(orderId, user, capacity).Result != null;
}

return Policy
    .Handle<HttpRequestException>(_ => !orderCreated())
    .Or<TimeoutRejectedException>(r => !orderCreated()) // thrown by Polly's TimeoutPolicy if the inner call times out
    .OrResult<HttpResponseMessage>(r => !orderCreated() && ((int)r.StatusCode >= 500 || r.StatusCode == HttpStatusCode.RequestTimeout))
    .Retry(2,
    onRetry: (ctx, span) =>
    {
        // logging
    }).ExecuteAndCaptureAsync(() => _legacyHttpClient.RequestOrder(user, order, capacity));

[reisenberger]

Hi @codingmoh . You can certainly make a .Handle<>() clause call out to another method, to include more complex logic.

Concerns about the posted code could be:

• (a) GetOrderId(...).Result is sync-over-async blocking, which can be a common cause of deadlocks;
• (b) Thread.Sleep(...) blocks a thread; await Task.Delay() would be preferable in an async call environment.

That said, Polly intentionally does not offer an async version of the .Handle<TException>(predicate) clauses (because having async I/O-style calls in handle clauses makes it hard to reason about the source of onward error - handle-clause or executed-delegate? - when the policy is part of a wider PolicyWrap). So you would not be able to convert the problem areas (a) and (b) to async code, within a Polly .Handle<>(...)clause.

---

A different approach:

Part of the logic is to re-interpret that what initially looks like a failure is in fact a success. Fallback policy is close to this: designed for substituting a different response and/or taking an alternative action, in the event of certain exceptions or responses.

You could move your "reinterpret failure as success" logic into a FallbackPolicy, and consider actually substituting a success response, so that the code following the .ExecuteAsync() need not even need to be concerned with handling the special case. This does explicitly mask the original failure with a success (as far as calling code is concerned), so you would want to log that somewhere so that you had a record it happened.

Something like:

var errorsToHandle = Policy
    .Handle<HttpRequestException>()
    .Or<TimeoutRejectedException>()
    .OrResult<HttpResponseMessage>(((int)r.StatusCode >= 500 || r.StatusCode == HttpStatusCode.RequestTimeout));

var waitAndCheckIfErrorIsReallySuccess = errorsToHandle.FallbackAsync(
            onFallbackAsync: (outcome, context) => Task.Delay(TimeSpan.FromSeconds(20)), // Delay, then:
            fallbackAction: (outcome, context, cancellationToken) => async
            {
                var orderId = await GetOrderId(orderId, user, capacity);
                if (orderId != null)
                {
                    // This substitutes a success result when you detect the original call succeeded.  
                    // Recommend logging the original failure and the fact that the substitution was made, 
                    // otherwise you will have no record of the difference between that and a genuine first-time success.
                    // You may want to substitute something as close as possible to what the original 
                    // _legacyHttpClient.RequestOrder(...) call would have returned if it was successful.  
                    // This is just an example:
                    return new HttpResponseMessage(HttpStatuscode.OK); 
                }
                else
                {
                    return outcome.Result ?? throw outcome.Exception; // return original result or rethrow exception
                }
            });

var retry = errorsToHandle.RetryAsync(2,
    onRetry: (ctx, span) =>
    {
        // logging
    });

return await retry.WrapAsync(waitAndCheckIfErrorIsReallySuccess)
        .ExecuteAndCaptureAsync(() => _legacyHttpClient.RequestOrder(user, order, capacity));

This approach also only executes the delay once per exception thrown.

[codingmoh]

Hello @reisenberger, I can't thank you enough. Your provided solution seems like a great fit! :-) - We just started to work on the implementation of it. Hopefully, I'll return with great accomplishment. :-)

Using a fallback policy in a "reinterpret failure as success" approach, didn't occur to us at all - and yes, we've been trying to get rid of sync over async and thread.sleep, but discovered, that there are no async versions.

Now I understand, why the api doesn't offer async operations - Thank you so much for covering that as well.

[reisenberger]

Hi @codingmoh ! Closing this as think we are covered here. However, if you do have further questions, please do not hesitate to post again, and we can re-open.

[codingmoh]

hello @reisenberger
That's fine!
The retry policy is already running on production. :-)

No issues so far!
Thank you so much!