Add RetryAfter to BrokenCircuitException. (#2324)

Add RetryAfter to BrokenCircuitException.

docs/strategies/circuit-breaker.md

@@ -14,7 +14,7 @@
 
 ---
 
-The circuit breaker **reactive** resilience strategy shortcuts the execution if the underlying resource is detected as unhealthy. The detection process is done via sampling. If the sampled executions' failure-success ratio exceeds a predefined threshold then a circuit breaker will prevent any new executions by throwing a `BrokenCircuitException`. After a preset duration the circuit breaker performs a probe, because the assumption is that this period was enough for the resource to self-heal. Depending on the outcome of the probe, the circuit will either allow new executions or continue to block them.
+The circuit breaker **reactive** resilience strategy shortcuts the execution if the underlying resource is detected as unhealthy. The detection process is done via sampling. If the sampled executions' failure-success ratio exceeds a predefined threshold then a circuit breaker will prevent any new executions by throwing a `BrokenCircuitException`. After a preset duration the circuit breaker performs a probe, because the assumption is that this period was enough for the resource to self-heal. Depending on the outcome of the probe, the circuit will either allow new executions or continue to block them. If an execution is blocked by the circuit breaker, the thrown exception may indicate the amount of time executions will continue to be blocked through its `RetryAfter` property.
 
 > [!NOTE]
 > Be aware that the Circuit Breaker strategy [rethrows all exceptions](https://github.com/App-vNext/Polly/wiki/Circuit-Breaker#exception-handling), including those that are handled. A Circuit Breaker's role is to monitor faults and break the circuit when a certain threshold is reached; it does not manage retries. Combine the Circuit Breaker with a Retry strategy if needed.

src/Polly.Core/CircuitBreaker/BrokenCircuitException.cs

@@ -22,6 +22,14 @@ public BrokenCircuitException()
     {
     }
 
+    /// <summary>
+    /// Initializes a new instance of the <see cref="BrokenCircuitException"/> class.
+    /// </summary>
+    /// <param name="retryAfter">The period after which the circuit will close.</param>
+    public BrokenCircuitException(TimeSpan retryAfter)
+        : base($"The circuit is now open and is not allowing calls. It can be retried after '{retryAfter}'.")
+        => RetryAfter = retryAfter;
+
     /// <summary>
     /// Initializes a new instance of the <see cref="BrokenCircuitException"/> class.
     /// </summary>
@@ -31,6 +39,14 @@ public BrokenCircuitException(string message)
     {
     }
 
+    /// <summary>
+    /// Initializes a new instance of the <see cref="BrokenCircuitException"/> class.
+    /// </summary>
+    /// <param name="message">The message that describes the error.</param>
+    /// <param name="retryAfter">The period after which the circuit will close.</param>
+    public BrokenCircuitException(string message, TimeSpan retryAfter)
+        : base(message) => RetryAfter = retryAfter;
+
     /// <summary>
     /// Initializes a new instance of the <see cref="BrokenCircuitException"/> class.
     /// </summary>
@@ -41,6 +57,15 @@ public BrokenCircuitException(string message, Exception inner)
     {
     }
 
+    /// <summary>
+    /// Initializes a new instance of the <see cref="BrokenCircuitException"/> class.
+    /// </summary>
+    /// <param name="message">The message that describes the error.</param>
+    /// <param name="retryAfter">The period after which the circuit will close.</param>
+    /// <param name="inner">The inner exception.</param>
+    public BrokenCircuitException(string message, TimeSpan retryAfter, Exception inner)
+        : base(message, inner) => RetryAfter = retryAfter;
+
 #pragma warning disable RS0016 // Add public types and members to the declared API
 #if !NETCOREAPP
     /// <summary>
@@ -51,7 +76,42 @@ public BrokenCircuitException(string message, Exception inner)
     protected BrokenCircuitException(SerializationInfo info, StreamingContext context)
         : base(info, context)
     {
+        Guard.NotNull(info);
+
+        // https://github.com/dotnet/runtime/issues/42460
+        SerializationInfoEnumerator enumerator = info.GetEnumerator();
+        while (enumerator.MoveNext())
+        {
+            SerializationEntry entry = enumerator.Current;
+            if (string.Equals(entry.Name, "RetryAfter", StringComparison.Ordinal))
+            {
+                var ticks = (long)entry.Value;
+                RetryAfter = new TimeSpan(ticks);
+                break;
+            }
+        }
+    }
+
+    /// <inheritdoc/>
+    public override void GetObjectData(SerializationInfo info, StreamingContext context)
+    {
+        Guard.NotNull(info);
+        if (RetryAfter.HasValue)
+        {
+            info.AddValue("RetryAfter", RetryAfter.Value.Ticks);
+        }
+
+        base.GetObjectData(info, context);
     }
 #endif
 #pragma warning restore RS0016 // Add public types and members to the declared API
+
+    /// <summary>
+    /// Gets the amount of time before the circuit can become closed, if known.
+    /// </summary>
+    /// <remarks>
+    /// This value is specified when the instance is constructed and may be inaccurate if consumed at a later time.
+    /// Can be <see langword="null"/> if not provided or if the circuit was manually isolated.
+    /// </remarks>
+    public TimeSpan? RetryAfter { get; }
 }

src/Polly.Core/CircuitBreaker/Controller/CircuitStateController.cs

@@ -305,11 +305,15 @@ private void SetLastHandledOutcome_NeedsLock(Outcome<T> outcome)
         _breakingException = outcome.Exception;
     }
 
-    private BrokenCircuitException CreateBrokenCircuitException() => _breakingException switch
+    private BrokenCircuitException CreateBrokenCircuitException()
     {
-        Exception exception => new BrokenCircuitException(BrokenCircuitException.DefaultMessage, exception),
-        _ => new BrokenCircuitException(BrokenCircuitException.DefaultMessage)
-    };
+        TimeSpan retryAfter = _blockedUntil - _timeProvider.GetUtcNow();
+        return _breakingException switch
+        {
+            Exception exception => new BrokenCircuitException(BrokenCircuitException.DefaultMessage, retryAfter, exception),
+            _ => new BrokenCircuitException(BrokenCircuitException.DefaultMessage, retryAfter)
+        };
+    }
 
     private void OpenCircuit_NeedsLock(Outcome<T> outcome, bool manual, ResilienceContext context, out Task? scheduledTask)
         => OpenCircuitFor_NeedsLock(outcome, _breakDuration, manual, context, out scheduledTask);

src/Polly.Core/PublicAPI.Unshipped.txt

@@ -1 +1,5 @@
 #nullable enable
+Polly.CircuitBreaker.BrokenCircuitException.BrokenCircuitException(string! message, System.TimeSpan retryAfter) -> void
+Polly.CircuitBreaker.BrokenCircuitException.BrokenCircuitException(string! message, System.TimeSpan retryAfter, System.Exception! inner) -> void
+Polly.CircuitBreaker.BrokenCircuitException.BrokenCircuitException(System.TimeSpan retryAfter) -> void
+Polly.CircuitBreaker.BrokenCircuitException.RetryAfter.get -> System.TimeSpan?

test/Polly.Core.Tests/CircuitBreaker/BrokenCircuitExceptionTests.cs

@@ -5,17 +5,79 @@ namespace Polly.Core.Tests.CircuitBreaker;
 public class BrokenCircuitExceptionTests
 {
     [Fact]
-    public void Ctor_Ok()
+    public void Ctor_Default_Ok()
     {
-        new BrokenCircuitException().Message.Should().Be("The circuit is now open and is not allowing calls.");
-        new BrokenCircuitException("Dummy.").Message.Should().Be("Dummy.");
-        new BrokenCircuitException("Dummy.", new InvalidOperationException()).Message.Should().Be("Dummy.");
-        new BrokenCircuitException("Dummy.", new InvalidOperationException()).InnerException.Should().BeOfType<InvalidOperationException>();
+        var exception = new BrokenCircuitException();
+        exception.Message.Should().Be("The circuit is now open and is not allowing calls.");
+        exception.RetryAfter.Should().BeNull();
+    }
+
+    [Fact]
+    public void Ctor_Message_Ok()
+    {
+        var exception = new BrokenCircuitException(TestMessage);
+        exception.Message.Should().Be(TestMessage);
+        exception.RetryAfter.Should().BeNull();
+    }
+
+    [Fact]
+    public void Ctor_RetryAfter_Ok()
+    {
+        var exception = new BrokenCircuitException(TestRetryAfter);
+        exception.Message.Should().Be($"The circuit is now open and is not allowing calls. It can be retried after '{TestRetryAfter}'.");
+        exception.RetryAfter.Should().Be(TestRetryAfter);
+    }
+
+    [Fact]
+    public void Ctor_Message_RetryAfter_Ok()
+    {
+        var exception = new BrokenCircuitException(TestMessage, TestRetryAfter);
+        exception.Message.Should().Be(TestMessage);
+        exception.RetryAfter.Should().Be(TestRetryAfter);
+    }
+
+    [Fact]
+    public void Ctor_Message_InnerException_Ok()
+    {
+        var exception = new BrokenCircuitException(TestMessage, new InvalidOperationException());
+        exception.Message.Should().Be(TestMessage);
+        exception.InnerException.Should().BeOfType<InvalidOperationException>();
+        exception.RetryAfter.Should().BeNull();
+    }
+
+    [Fact]
+    public void Ctor_Message_RetryAfter_InnerException_Ok()
+    {
+        var exception = new BrokenCircuitException(TestMessage, TestRetryAfter, new InvalidOperationException());
+        exception.Message.Should().Be(TestMessage);
+        exception.InnerException.Should().BeOfType<InvalidOperationException>();
+        exception.RetryAfter.Should().Be(TestRetryAfter);
     }
 
 #if !NETCOREAPP
     [Fact]
-    public void BinarySerialization_Ok() =>
-        BinarySerializationUtil.SerializeAndDeserializeException(new BrokenCircuitException()).Should().NotBeNull();
+    public void BinarySerialization_NonNullRetryAfter_Ok()
+    {
+        var exception = new BrokenCircuitException(TestMessage, TestRetryAfter, new InvalidOperationException());
+        BrokenCircuitException roundtripResult = BinarySerializationUtil.SerializeAndDeserializeException(exception);
+        roundtripResult.Should().NotBeNull();
+        roundtripResult.Message.Should().Be(TestMessage);
+        roundtripResult.InnerException.Should().BeOfType<InvalidOperationException>();
+        roundtripResult.RetryAfter.Should().Be(TestRetryAfter);
+    }
+
+    [Fact]
+    public void BinarySerialization_NullRetryAfter_Ok()
+    {
+        var exception = new BrokenCircuitException(TestMessage, new InvalidOperationException());
+        BrokenCircuitException roundtripResult = BinarySerializationUtil.SerializeAndDeserializeException(exception);
+        roundtripResult.Should().NotBeNull();
+        roundtripResult.Message.Should().Be(TestMessage);
+        roundtripResult.InnerException.Should().BeOfType<InvalidOperationException>();
+        roundtripResult.RetryAfter.Should().BeNull();
+    }
 #endif
+
+    private const string TestMessage = "Dummy.";
+    private static readonly TimeSpan TestRetryAfter = TimeSpan.FromHours(1);
 }

test/Polly.Core.Tests/CircuitBreaker/CircuitBreakerResiliencePipelineBuilderTests.cs

@@ -68,7 +68,8 @@ public void AddCircuitBreaker_IntegrationTest()
         int closed = 0;
         int halfOpened = 0;
 
-        var breakDuration = TimeSpan.FromSeconds(1);
+        var halfBreakDuration = TimeSpan.FromMilliseconds(500);
+        var breakDuration = halfBreakDuration + halfBreakDuration;
 
         var options = new CircuitBreakerStrategyOptions
         {
@@ -94,15 +95,25 @@ public void AddCircuitBreaker_IntegrationTest()
         opened.Should().Be(1);
         halfOpened.Should().Be(0);
         closed.Should().Be(0);
-        Assert.Throws<BrokenCircuitException>(() => strategy.Execute(_ => 0));
+        BrokenCircuitException exception = Assert.Throws<BrokenCircuitException>(() => strategy.Execute(_ => 0));
+        exception.RetryAfter.Should().Be(breakDuration);
+
+        // Circuit still open after some time
+        timeProvider.Advance(halfBreakDuration);
+        opened.Should().Be(1);
+        halfOpened.Should().Be(0);
+        closed.Should().Be(0);
+        exception = Assert.Throws<BrokenCircuitException>(() => strategy.Execute(_ => 0));
+        exception.RetryAfter.Should().Be(halfBreakDuration);
 
         // Circuit Half Opened
-        timeProvider.Advance(breakDuration);
+        timeProvider.Advance(halfBreakDuration);
         strategy.Execute(_ => -1);
-        Assert.Throws<BrokenCircuitException>(() => strategy.Execute(_ => 0));
+        exception = Assert.Throws<BrokenCircuitException>(() => strategy.Execute(_ => 0));
         opened.Should().Be(2);
         halfOpened.Should().Be(1);
         closed.Should().Be(0);
+        exception.RetryAfter.Should().Be(breakDuration);
 
         // Now close it
         timeProvider.Advance(breakDuration);
@@ -119,7 +130,8 @@ public void AddCircuitBreaker_IntegrationTest_WithBreakDurationGenerator()
         int closed = 0;
         int halfOpened = 0;
 
-        var breakDuration = TimeSpan.FromSeconds(1);
+        var halfBreakDuration = TimeSpan.FromMilliseconds(500);
+        var breakDuration = halfBreakDuration + halfBreakDuration;
 
         var options = new CircuitBreakerStrategyOptions
         {
@@ -146,15 +158,25 @@ public void AddCircuitBreaker_IntegrationTest_WithBreakDurationGenerator()
         opened.Should().Be(1);
         halfOpened.Should().Be(0);
         closed.Should().Be(0);
-        Assert.Throws<BrokenCircuitException>(() => strategy.Execute(_ => 0));
+        BrokenCircuitException exception = Assert.Throws<BrokenCircuitException>(() => strategy.Execute(_ => 0));
+        exception.RetryAfter.Should().Be(breakDuration);
+
+        // Circuit still open after some time
+        timeProvider.Advance(halfBreakDuration);
+        opened.Should().Be(1);
+        halfOpened.Should().Be(0);
+        closed.Should().Be(0);
+        exception = Assert.Throws<BrokenCircuitException>(() => strategy.Execute(_ => 0));
+        exception.RetryAfter.Should().Be(halfBreakDuration);
 
         // Circuit Half Opened
-        timeProvider.Advance(breakDuration);
+        timeProvider.Advance(halfBreakDuration);
         strategy.Execute(_ => -1);
-        Assert.Throws<BrokenCircuitException>(() => strategy.Execute(_ => 0));
+        exception = Assert.Throws<BrokenCircuitException>(() => strategy.Execute(_ => 0));
         opened.Should().Be(2);
         halfOpened.Should().Be(1);
         closed.Should().Be(0);
+        exception.RetryAfter.Should().Be(breakDuration);
 
         // Now close it
         timeProvider.Advance(breakDuration);
@@ -178,8 +200,8 @@ public async Task AddCircuitBreakers_WithIsolatedManualControl_ShouldBeIsolated(
             .AddCircuitBreaker(new() { ManualControl = manualControl })
             .Build();
 
-        strategy1.Invoking(s => s.Execute(() => { })).Should().Throw<IsolatedCircuitException>();
-        strategy2.Invoking(s => s.Execute(() => { })).Should().Throw<IsolatedCircuitException>();
+        strategy1.Invoking(s => s.Execute(() => { })).Should().Throw<IsolatedCircuitException>().Where(e => e.RetryAfter == null);
+        strategy2.Invoking(s => s.Execute(() => { })).Should().Throw<IsolatedCircuitException>().Where(e => e.RetryAfter == null);
 
         await manualControl.CloseAsync();
 

test/Polly.Core.Tests/CircuitBreaker/CircuitBreakerResilienceStrategyTests.cs

@@ -53,7 +53,7 @@ public async Task Ctor_ManualControl_EnsureAttached()
         var strategy = Create();
 
         await _options.ManualControl.IsolateAsync(CancellationToken.None);
-        strategy.Invoking(s => s.Execute(_ => 0)).Should().Throw<IsolatedCircuitException>();
+        strategy.Invoking(s => s.Execute(_ => 0)).Should().Throw<IsolatedCircuitException>().Where(e => e.RetryAfter == null);
 
         await _options.ManualControl.CloseAsync(CancellationToken.None);
 

test/Polly.Core.Tests/CircuitBreaker/Controller/CircuitStateControllerTests.cs

@@ -52,7 +52,8 @@ public async Task IsolateAsync_Ok()
         called.Should().BeTrue();
 
         var outcome = await controller.OnActionPreExecuteAsync(ResilienceContextPool.Shared.Get());
-        outcome.Value.Exception.Should().BeOfType<IsolatedCircuitException>();
+        outcome.Value.Exception.Should().BeOfType<IsolatedCircuitException>()
+            .And.Subject.As<IsolatedCircuitException>().RetryAfter.Should().BeNull();
 
         // now close it
         await controller.CloseCircuitAsync(ResilienceContextPool.Shared.Get());
@@ -119,7 +120,7 @@ public async Task OnActionPreExecute_CircuitOpenedByValue()
 
         await OpenCircuit(controller, Outcome.FromResult(99));
         var error = (BrokenCircuitException)(await controller.OnActionPreExecuteAsync(ResilienceContextPool.Shared.Get())).Value.Exception!;
-        error.Should().BeOfType<BrokenCircuitException>();
+        error.Should().BeOfType<BrokenCircuitException>().And.Subject.As<BrokenCircuitException>().RetryAfter.Should().NotBeNull();
 
         GetBlockedTill(controller).Should().Be(_timeProvider.GetUtcNow() + _options.BreakDuration);
     }
@@ -147,6 +148,7 @@ await OpenCircuit(
             {
                 stacks.Add(e.StackTrace!);
                 e.Message.Should().Be("The circuit is now open and is not allowing calls.");
+                e.RetryAfter.Should().NotBeNull();
 
                 if (innerException)
                 {
@@ -206,6 +208,7 @@ public async Task OnActionPreExecute_CircuitOpenedByException()
         await OpenCircuit(controller, Outcome.FromException<int>(new InvalidOperationException()));
         var error = (BrokenCircuitException)(await controller.OnActionPreExecuteAsync(ResilienceContextPool.Shared.Get())).Value.Exception!;
         error.InnerException.Should().BeOfType<InvalidOperationException>();
+        error.RetryAfter.Should().NotBeNull();
     }
 
     [Fact]
@@ -258,7 +261,7 @@ public async Task OnActionPreExecute_HalfOpen()
         // act
         await controller.OnActionPreExecuteAsync(ResilienceContextPool.Shared.Get());
         var error = (await controller.OnActionPreExecuteAsync(ResilienceContextPool.Shared.Get())).Value.Exception;
-        error.Should().BeOfType<BrokenCircuitException>();
+        error.Should().BeOfType<BrokenCircuitException>().And.Subject.As<BrokenCircuitException>().RetryAfter.Should().NotBeNull();
 
         // assert
         controller.CircuitState.Should().Be(CircuitState.HalfOpen);
@@ -462,7 +465,7 @@ public async Task OnActionFailureAsync_VoidResult_EnsureBreakingExceptionNotSet(
         // assert
         controller.LastException.Should().BeNull();
         var outcome = await controller.OnActionPreExecuteAsync(ResilienceContextPool.Shared.Get());
-        outcome.Value.Exception.Should().BeOfType<BrokenCircuitException>();
+        outcome.Value.Exception.Should().BeOfType<BrokenCircuitException>().And.Subject.As<BrokenCircuitException>().RetryAfter.Should().NotBeNull();
     }
 
     [Fact]
@@ -495,9 +498,11 @@ public async Task Flow_Closed_HalfOpen_Open_HalfOpen_Closed()
         controller.CircuitState.Should().Be(CircuitState.Open);
 
         // execution rejected
-        AdvanceTime(TimeSpan.FromMilliseconds(1));
+        TimeSpan advanceTimeRejected = TimeSpan.FromMilliseconds(1);
+        AdvanceTime(advanceTimeRejected);
         var outcome = await controller.OnActionPreExecuteAsync(ResilienceContextPool.Shared.Get());
-        outcome.Value.Exception.Should().BeOfType<BrokenCircuitException>();
+        outcome.Value.Exception.Should().BeOfType<BrokenCircuitException>()
+            .And.Subject.As<BrokenCircuitException>().RetryAfter.Should().Be(_options.BreakDuration - advanceTimeRejected);
 
         // wait and try, transition to half open
         AdvanceTime(_options.BreakDuration + _options.BreakDuration);