Circuit breaker not handling exception at end points even when ExceptionsAllowedBeforeBreaking is set.

[anuviswan]

Expected Behavior / New Feature

As per the documentation of QoS functionality of Ocelot, it uses Polly to achieve Circuit Breaker among the other resilience features. From what I could understand, the following configuration should have triggered the circuit breaker after an endpoint raised exception more than 2 times.

"QoSOptions": {
   "ExceptionsAllowedBeforeBreaking": 2,
   "DurationOfBreak": 10000,
   "TimeoutValue": 2000
}

Actual Behavior / Motivation for New Feature

However, it looked like the exception raised by the end points isn't handled by the circuit breaker. Please note that the Timeout works perfectly and triggers the circuit breaker as expected. However the case wasn't same with the ExceptionsAllowedBeforeBreaking.

Steps to Reproduce the Problem

1. Create a route in the gateway with the above QoS configuration
2. The specified Endpoint should raise an exception
3. Raise the request for end point via the gateway.

The code sample in the following link demonstrates the issue
Demo Code

Similar problem is mentioned in the following Stackoverflow question too

• Ocelot QoS doesn't break circuit when n number of exception arises

Specifications

• Ocelot: 17.0.1
• Ocelot.Provider.Polly: 17.0.1

[ks1990cn]

Hi,

Thanks for reporting this bug @anuviswan.
@anuviswan Do you want to propose any solution or anything else you want to add on?

I am able to replicate and started looking into it. @raman-m

[ks1990cn]

Raised bug to Polly, App-vNext/Polly#1716.

We are in touch there. Thanks

@anuviswan @raman-m

[raman-m]

@ks1990cn Tanmay,
Are you sure it's a bug of Polly when posting on Polly boards?
I'm not sure... and the bug can belong to Ocelot, because of ugly Exception-to-Error design, see the HttpExceptionToErrorMapper class.

@ggnaegi Welcome! To have a fun! 😉

[raman-m]

@anuviswan Hey, Anu!
Thanks for reporting this bug!

Version: Ocelot - 17.0.1, Ocelot.Provider.Polly 17.0.1

Could you confirm please, that the bug exists in 19.x and 20.0 versions?

[ks1990cn]

@raman-m I've tried my own projects and created HttpClient call. After multiple errors from downstream I was unable to find circuitbreaker is working even in my case.
Its not about exception code, exception code is not changing at all in any way. You can see my GIF attached to Polly board.
Ocelot code looks fine to me.

Anyone else also can verify, I will retro inspect my things again.

[ks1990cn]

@ks1990cn Tanmay, Are you sure it's a bug of Polly when posting on Polly boards? I'm not sure... and the bug can belong to Ocelot, because of ugly Exception-to-Error design, see the HttpExceptionToErrorMapper class.

@ggnaegi Welcome! To have a fun! 😉

We don't get code to mapper part even, here is my debugger point

[ggnaegi]

@raman-m @ks1990cn @anuviswan Guys, I thought I was writing you rubbish. But, we have a bigger problem here:
First, as I wrote before, we shouldn't create a new CircuitBreaker per request / route. It will never work. I was suddenly unsure because in another project we had retry policies aswell.

I was able to make this acceptance test pass:

[Fact]
        public void should_open_circuit_breaker_after_two_exceptions()
        {
            var port = RandomPortFinder.GetRandomPort();

            var configuration = new FileConfiguration
            {
                Routes = new List<FileRoute>
                {
                    new()
                    {
                        DownstreamPathTemplate = "/",
                        DownstreamScheme = "http",
                        DownstreamHostAndPorts = new List<FileHostAndPort>
                        {
                            new()
                            {
                                Host = "localhost",
                                Port = port,
                            },
                        },
                        UpstreamPathTemplate = "/",
                        UpstreamHttpMethod = new List<string> { "Get" },
                        QoSOptions = new FileQoSOptions
                        {
                            ExceptionsAllowedBeforeBreaking = 2,
                            TimeoutValue = 5000,
                            DurationOfBreak = 1000,
                        },
                    },
                },
            };

            this.Given(x => x.GivenThereIsABrokenServiceRunningOn($"http://localhost:{port}"))
                .And(x => _steps.GivenThereIsAConfiguration(configuration))
                .And(x => _steps.GivenOcelotIsRunningWithPolly())
                .And(x => _steps.WhenIGetUrlOnTheApiGateway("/"))
                .And(x => _steps.WhenIGetUrlOnTheApiGateway("/"))
                .And(x => _steps.WhenIGetUrlOnTheApiGateway("/"))
                .Then(x => _steps.ThenTheStatusCodeShouldBe(HttpStatusCode.ServiceUnavailable))
                .BDDfy();
        }

But the code is quite ugly...

Second, in PollyCircuitBreakingDelegatingHandler, no exceptions are thrown here when the client return status 500. I was able to make it work with this terrible bit of code:

var policyResult = await policy.ExecuteAsync(async () =>
            {
                var result = await base.SendAsync(request, cancellationToken);
                if (result.StatusCode == HttpStatusCode.InternalServerError)
                {
                    throw new HttpRequestException("Service Unavailable");
                }

                return result;
            });

            return policyResult;

Then, after two tries, the Circuit Breaker kicks in, otherwise never...

Summary:

• We shouldn't create a Circuit Breaker per route / request, only maintain one Circuit Breaker per route
• The Circuit Breaker is Expecting an Exception to kick in, a response with status 500 is not an Exception

@ks1990cn is right when he says that the circuit breaker is not working, but it's not a bug in polly, but a bug in ocelot.

[ks1990cn]

@ggnaegi commented on Oct 23

Correct I agree on this, I made configuration issue too on my personal project. https://www.youtube.com/watch?v=z6YcU0PW_9E&t=1s helped me to configure it properly. I was doing same mistake. Closed bug on Polly.

[raman-m]

The issue has been accepted due to open PR #1753

• #1550 #1706 Addressing the QoS options ExceptionsAllowedBeforeBreaking issue #1753

[raman-m]

@ggnaegi commented on Oct 23

• But, we have a bigger problem here:
• the circuit breaker is not working, but it's not a bug in Polly, but a bug in ocelot.

Well... Has this problem already addressed in PR #1753, so is it included in PR?
Sorry, I ask strange questions sometimes

[ggnaegi]

@raman-m commented on Oct 30

Yes it's part of the PR

[raman-m]

@anuviswan Hey Anu,
Could you check & test the bug fix we've prepared as a gift for you, plz? 😉