Polly CircuitBreaker fallback not working

Question

I have the following policies:

var retryPolicy = Policy.Handle<Exception>(e => (e is HttpRequestException || e.InnerException is HttpRequestException)).WaitAndRetry(
                retryCount: maxRetryCount,
                sleepDurationProvider: attempt => TimeSpan.FromSeconds(Math.Pow(2, attempt)),
                onRetry: (exception, calculatedWaitDuration, retryCount, context) =>
                {
                    Log.Error($"Retry => Count: {retryCount}, Wait duration: {calculatedWaitDuration}, Policy Wrap: {context.PolicyWrapKey}, Policy: {context.PolicyKey}, Endpoint: {context.OperationKey}, Exception: {exception}.");
                });

var circuitBreaker = Policy.Handle<Exception>(e => (e is HttpRequestException || e.InnerException is HttpRequestException)).CircuitBreaker(maxExceptionsBeforeBreaking, TimeSpan.FromSeconds(circuitBreakDurationSeconds), onBreak, onReset);

var sharedBulkhead = Policy.Bulkhead(maxParallelizations, maxQueuingActions, onBulkheadRejected);

var fallbackForCircuitBreaker = Policy<bool>
             .Handle<BrokenCircuitException>()
             .Fallback(
                 fallbackValue: false,
                 onFallback: (b, context) =>
                 {
                     Log.Error($"Operation attempted on broken circuit => Policy Wrap: {context.PolicyWrapKey}, Policy: {context.PolicyKey}, Endpoint: {context.OperationKey}");
                 }
             );

            var fallbackForAnyException = Policy<bool>
                .Handle<Exception>()
                .Fallback(
                    fallbackAction: (context) => { return false; },
                    onFallback: (e, context) =>
                    {
                        Log.Error($"An unexpected error occured => Policy Wrap: {context.PolicyWrapKey}, Policy: {context.PolicyKey}, Endpoint: {context.OperationKey}");
                    }
                );

var resilienceStrategy = Policy.Wrap(retryPolicy, circuitBreaker, sharedBulkhead);
            var policyWrap = fallbackForAnyException.Wrap(fallbackForCircuitBreaker.Wrap(resilienceStrategy));

public bool CallApi(ChangeMapModel changeMessage)
    {
        var httpClient = new HttpClient();
        var endPoint = changeMessage.EndPoint;
        var headers = endPoint.Headers;
        if (headers != null)
        {
            foreach (var header in headers)
            {
                if (header.Contains(':'))
                {
                    var splitHeader = header.Split(':');
                    httpClient.DefaultRequestHeaders.Add(splitHeader[0], splitHeader[1]); 
                }
            } 
        }

        var res = httpClient.PostAsync(endPoint.Uri, null);
        var response = res.Result;
        response.EnsureSuccessStatusCode();
        return true;
    }

I execute the policy like so:

policyWrap.Execute((context) => CallApi(changeMessage), new Context(endPoint));

The problem is that I am not getting a hit in the CircuitBreaker callback when an action is executed on open circuit.

I want an API call to be placed through the policy with the exception type to be handled being HttpRequestException. Is there something wrong with the policy definitions? Why isn't the circuit breaker fallback called?

Answer 1

I created the following minimum, complete, verifiable example to help explore the problem:

Note: not necessarily a finished product; just some minor mods to the posted code and extra annotation, to help explore the question.

using Polly;
using Polly.CircuitBreaker;
using System;
using System.Net.Http;
using System.Threading.Tasks;

public class Program
{
    public static void Main()
    {
        int maxRetryCount = 6;
        double circuitBreakDurationSeconds = 0.2 /* experiment with effect of shorter or longer here, eg: change to = 1, and the fallbackForCircuitBreaker is correctly invoked */ ;
        int maxExceptionsBeforeBreaking = 4; /* experiment with effect of fewer here, eg change to = 1, and the fallbackForCircuitBreaker is correctly invoked */
        int maxParallelizations = 2;
        int maxQueuingActions = 2;

        var retryPolicy = Policy.Handle<Exception>(e => (e is HttpRequestException || (/*!(e is BrokenCircuitException) &&*/ e.InnerException is HttpRequestException))) // experiment with introducing the extra (!(e is BrokenCircuitException) && ) clause here, if necessary/desired, depending on goal
            .WaitAndRetry(
                retryCount: maxRetryCount,
                sleepDurationProvider: attempt => TimeSpan.FromMilliseconds(50 * Math.Pow(2, attempt)),
                onRetry: (ex, calculatedWaitDuration, retryCount, context) =>
                {
                    Console.WriteLine(String.Format("Retry => Count: {0}, Wait duration: {1}, Policy Wrap: {2}, Policy: {3}, Endpoint: {4}, Exception: {5}", retryCount, calculatedWaitDuration, context.PolicyWrapKey, context.PolicyKey, context.OperationKey, ex.Message));
                });

        var circuitBreaker = Policy.Handle<Exception>(e => (e is HttpRequestException || e.InnerException is HttpRequestException))
            .CircuitBreaker(maxExceptionsBeforeBreaking,
                TimeSpan.FromSeconds(circuitBreakDurationSeconds),
                onBreak: (ex, breakDuration) => {
                    Console.WriteLine(String.Format("Circuit breaking for {0} ms due to {1}", breakDuration.TotalMilliseconds, ex.Message));
                },
                onReset: () => {
                    Console.WriteLine("Circuit closed again.");
                },
                onHalfOpen: () => { Console.WriteLine("Half open."); });

        var sharedBulkhead = Policy.Bulkhead(maxParallelizations, maxQueuingActions);

        var fallbackForCircuitBreaker = Policy<bool>
             .Handle<BrokenCircuitException>()
            /* .OrInner<BrokenCircuitException>() */ // Consider this if necessary.
            /* .Or<Exception>(e => circuitBreaker.State != CircuitState.Closed) */ // This check will also detect the circuit in anything but healthy state, regardless of the final exception thrown.
             .Fallback(
                 fallbackValue: false,
                 onFallback: (b, context) =>
                 {
                     Console.WriteLine(String.Format("Operation attempted on broken circuit => Policy Wrap: {0}, Policy: {1}, Endpoint: {2}", context.PolicyWrapKey, context.PolicyKey, context.OperationKey));
                 }
             );

        var fallbackForAnyException = Policy<bool>
                .Handle<Exception>()
                .Fallback<bool>(
                    fallbackAction: (context) => { return false; },
                    onFallback: (e, context) =>
                    {
                        Console.WriteLine(String.Format("An unexpected error occured => Policy Wrap: {0}, Policy: {1}, Endpoint: {2}, Exception: {3}", context.PolicyWrapKey, context.PolicyKey, context.OperationKey, e.Exception.Message));
                    }
                );

        var resilienceStrategy = Policy.Wrap(retryPolicy, circuitBreaker, sharedBulkhead);
        var policyWrap = fallbackForAnyException.Wrap(fallbackForCircuitBreaker.Wrap(resilienceStrategy));

        bool outcome = policyWrap.Execute((context) => CallApi("http://www.doesnotexistattimeofwriting.com/"), new Context("some endpoint info"));
    }

    public static bool CallApi(string uri)
    {
        using (var httpClient = new HttpClient() { Timeout = TimeSpan.FromSeconds(1) }) // Consider HttpClient lifetimes and disposal; this pattern is for minimum change from original posted code, not a recommendation.
        {
            Task<HttpResponseMessage> res = httpClient.GetAsync(uri);
            var response = res.Result; // Consider async/await rather than blocking on the returned Task.
            response.EnsureSuccessStatusCode();
            return true;
        }
    }
}

---

More than one factor could be causing the fallbackForCircuitBreaker not to be invoked:

1. The circuitBreakDurationSeconds may be set shorter than the overall time taken by the various tries and waits between retries.

If so, the circuit may revert to half-open state. In half-open state or closed state, an exception which causes the circuit to break is rethrown as-is. BrokenCircuitException is only thrown when an call is prevented from being attempted by a (fully) open circuit.

Thus, if your circuit has reverted to half-open by the time retries exhaust, the exception thrown back on to the wrapping fallback policies will be an HttpRequestException, not BrokenCircuitException.

2. .Handle<Exception>(e => (e is HttpRequestException || e.InnerException is HttpRequestException)) clauses may catch a CircuitBreakerExceptions having InnerException is HttpRequestException

A CircuitBreakerException contains the exception which caused the circuit to break as its InnerException. So an overly greedy/looser check of e.InnerException is HttpRequestException could also catch a CircuitBreakerException having InnerException is HttpRequestException. This may or may not be desired depending on your goal.

I believe this not to be happening for the original posted code because of the particular way it is constructed. Blocking on the Task returned by HttpClient.DoSomethingAsync(...) causes already an AggregateException->HttpRequestException, meaning a resulting CircuitBreakerException nests the HttpRequestException two-deep:

CircuitBreakerException -> AggregateException -> HttpRequestException

So this doesn't fall in to the one-deep check in the posted code. However, be aware that a CircuitBreakerException contains the exception which caused the circuit to break as its InnerException. This could cause a handle clause checking only e.InnerException is HttpRequestException to unwantedly (unexpectedly, if it is not your goal) retry for a CircuitBreakerException, if either:

(a) the code is changed to async/await, which will remove the AggregateException and so cause the nesting to be only one-deep

(b) the code is changed to Polly's .HandleInner<HttpRequestException>() syntax, which is recursively-greedy, and so would catch a nested-two-deep CircuitBreakerException->AggregateException->HttpRequestException.

---

Suggestions /* commented out */ // with additional explanation in the above code suggest how the code-as-posted may be adjusted so that fallbackForCircuitBreaker invokes as expected.

---

Two other thoughts:

3. Consider changing to async/await throughout if possible.

Blocking on HttpClient.DoSomethingAsync() by calling .Result can impact performance, or risk deadlocks if mixed with other async code, and is bringing in the whole AggregateException-with-InnerException pain.

4. Consider disposal and lifetime of HttpClient instances.

(Kept these points 3 and 4 intentionally brief as widely discussed elsewhere.)