Less logs for circuit breakers (and other Polly exceptions)

[RaynaldM]

We'd like to know more about the exceptions raised in PollyPoliciesDelegatingHandler, in order to get the origin of the log like this:

try
 {
     // at least one policy (timeout) will be returned
     // AsyncPollyPolicy can't be null
     // AsyncPollyPolicy constructor will throw if no policy is provided
     var policy = qoSProvider.GetPollyPolicyWrapper(_route).AsyncPollyPolicy;
     return await policy.ExecuteAsync(async () => await base.SendAsync(request, cancellationToken));
 }
 catch (BrokenCircuitException ex)
 {
     _logger.LogError($"Reached to allowed number of exceptions. Circuit is open for {request.RequestUri}", ex);
     throw;
 }
 catch (HttpRequestException ex)
 {
     _logger.LogError($"Error in {nameof(PollyPoliciesDelegatingHandler)}.{nameof(SendAsync)} for {request.RequestUri}", ex);
     throw;
 }

[raman-m]

@ggnaegi ❓ 🤣

[RaynaldM]

in fact catch (BrokenCircuitException ex) { _logger.LogError($"Reached to allowed number of exceptions. Circuit is open for {request.RequestUri}", ex); throw; } is useless because we have almost the same log here

 exceptionsAllowedBeforeBreakingPolicy = Policy
      .HandleResult<HttpResponseMessage>(r => _serverErrorCodes.Contains(r.StatusCode))
      .Or<TimeoutRejectedException>()
      .Or<TimeoutException>()
      .CircuitBreakerAsync(route.QosOptions.ExceptionsAllowedBeforeBreaking,
          durationOfBreak: TimeSpan.FromMilliseconds(route.QosOptions.DurationOfBreak),
          onBreak: (ex, breakDelay) => _logger.LogError(info + $"Breaking the circuit for {breakDelay.TotalMilliseconds} ms!", ex.Exception),
          onReset: () => _logger.LogDebug(info + "Call OK! Closed the circuit again."),
          onHalfOpen: () => _logger.LogDebug(info + "Half-open; Next call is a trial."));

@raman-m @ggnaegi agree with that ?

[ggnaegi]

@RaynaldM well, probably one or two test cases will fail but imho we can remove the whole try/catch, BrokenCircuitException is thrown and handled later in ErrorMessages. BrokenCircuitException is thrown by policy.ExecuteAsync, when conditions for circuit breaker are met. As for the HttpRequestException I don't think that this exception is thrown anyway. Soo... You know how it is, you don't want to mess with code that was already in place. It can be removed though. As you wrote BrokenCircuitException handling is equivalent to onBreak, yes.

[ggnaegi]

@RaynaldM So, both are useless... HttpRequestException can't be thrown since we are working with HandleResult<HttpResponseMessage>(r => _serverErrorCodes.Contains(r.StatusCode))

[raman-m]

I agree, this logic can be optimized.

[raman-m]

Guys, do you understand that if we removed try-catch block then all exceptions not being processed by Polly will go to Error mapping logic? Will it be OK ?

[RaynaldM]

Right, this try-catch (and associated logs) is useless

[raman-m]

I have a bad joke regarding Polly but I'm not going to say it now, after release... how is it useful or useless.

[raman-m]

"More accurate logs" Cannot get the phrase. What does accuracy mean here?

• Less log records
• Efficient code to process exceptions
• something else

What is the goal, @RaynaldM ?