Polly: Policy.Handle invoked twice for same exception in conjunction with AsyncPolicy.ExecuteAndCaptureAsync

[JustinKaffenberger]

Summary:

When using AsyncPolicy.ExecuteAndCaptureAsync in conjunction with Policy.Handle<TException>, the exception handling predicate (Func<TException, bool>) is invoked twice.

Expected behavior:

When the exception handling predicate returns false, it does not get invoked again for the same exception.

Actual behaviour:

When the exception handling predicate returns false, it does get invoked one additional time for the same exception.

Steps / Code to reproduce the problem:

// MSTest Sample
var exceptionsHandled = 0;
var policy = Polly.Policy.Handle<InvalidOperationException>(exception => {
    exceptionsHandled++;
    return false;
}).RetryForeverAsync();

var result = await policy.ExecuteAndCaptureAsync(async () =>
{
    throw new InvalidOperationException();
});

Microsoft.VisualStudio.TestTools.UnitTesting.Assert.AreEqual(1, exceptionsHandled);

[JustinKaffenberger]

After looking at the code-base, it seems the way it is currently designed, would make fixing this issue quite complicated. The issue is that, at its core, ExecuteAndCaptureAsync will end up calling ExecuteAsync. ExecuteAsync already evaluates the ExceptionPredicates across any exception that occur, and then rethrows the exception when it isn't successfully handled.

Then, ExecuteAndCaptureAsync captures the 'rethrown' exception, and evaluates the ExceptionPredicates again in order to accurately populate the result.

So all in all, some nasty workarounds would be required for this issue, so I'm going to close it.

Thanks!

[reisenberger]

Hi @JustinKaffenberger Yes, the .Handle<>() predicates are designed to be lightweight predicates which it is idempotent to evaluate more than once without causing side-effects.

The intentional place to hook in additional behaviour is the onRetry delegate of the retry policies (and similar delegates for other policies). If the goal is to count the number of exceptions handled, this can readily be done in the onRetry delegate.

[JustinKaffenberger]

That makes sense to me. I managed to find an alternative by using ExecuteAsync but I think I'll switch to using the onRetry delegate since that seems more appropriate. Thanks!