From 29f6e842a6c35a7f5c9a8402d19962115da49298 Mon Sep 17 00:00:00 2001 From: David Lee Date: Fri, 4 Oct 2024 01:53:14 +0800 Subject: [PATCH 1/3] Add RetryAfter to BrokenCircuitException. --- docs/strategies/circuit-breaker.md | 2 +- .../CircuitBreaker/BrokenCircuitException.cs | 34 +++++++++++++++ .../Controller/CircuitStateController.cs | 12 ++++-- src/Polly.Core/PublicAPI.Unshipped.txt | 4 ++ .../BrokenCircuitExceptionTests.cs | 25 +++++++++-- ...itBreakerResiliencePipelineBuilderTests.cs | 42 ++++++++++++++----- .../CircuitBreakerResilienceStrategyTests.cs | 2 +- .../Controller/CircuitStateControllerTests.cs | 17 +++++--- .../IsolatedCircuitExceptionTests.cs | 14 +++++-- 9 files changed, 122 insertions(+), 30 deletions(-) diff --git a/docs/strategies/circuit-breaker.md b/docs/strategies/circuit-breaker.md index 4a2bef29d6d..1ceeeac3833 100644 --- a/docs/strategies/circuit-breaker.md +++ b/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. diff --git a/src/Polly.Core/CircuitBreaker/BrokenCircuitException.cs b/src/Polly.Core/CircuitBreaker/BrokenCircuitException.cs index d240e64702a..744bf1d31a8 100644 --- a/src/Polly.Core/CircuitBreaker/BrokenCircuitException.cs +++ b/src/Polly.Core/CircuitBreaker/BrokenCircuitException.cs @@ -22,6 +22,14 @@ public BrokenCircuitException() { } + /// + /// Initializes a new instance of the class. + /// + /// The period after which the circuit will close. + public BrokenCircuitException(TimeSpan retryAfter) + : base($"The circuit is now open and is not allowing calls. It can be retried after '{retryAfter}'.") + => RetryAfter = retryAfter; + /// /// Initializes a new instance of the class. /// @@ -31,6 +39,14 @@ public BrokenCircuitException(string message) { } + /// + /// Initializes a new instance of the class. + /// + /// The message that describes the error. + /// The period after which the circuit will close. + public BrokenCircuitException(string message, TimeSpan retryAfter) + : base(message) => RetryAfter = retryAfter; + /// /// Initializes a new instance of the class. /// @@ -41,6 +57,15 @@ public BrokenCircuitException(string message, Exception inner) { } + /// + /// Initializes a new instance of the class. + /// + /// The message that describes the error. + /// The period after which the circuit will close. + /// The inner exception. + 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 /// @@ -54,4 +79,13 @@ protected BrokenCircuitException(SerializationInfo info, StreamingContext contex } #endif #pragma warning restore RS0016 // Add public types and members to the declared API + + /// + /// Gets the amount of time before the circuit can become closed, if known. + /// + /// + /// This value is specified when the instance is constructed and may be inaccurate if consumed at a later time. + /// Can be if not provided or if the circuit was manually isolated. + /// + public TimeSpan? RetryAfter { get; } } diff --git a/src/Polly.Core/CircuitBreaker/Controller/CircuitStateController.cs b/src/Polly.Core/CircuitBreaker/Controller/CircuitStateController.cs index 40dd2c5e35a..9076965334d 100644 --- a/src/Polly.Core/CircuitBreaker/Controller/CircuitStateController.cs +++ b/src/Polly.Core/CircuitBreaker/Controller/CircuitStateController.cs @@ -305,11 +305,15 @@ private void SetLastHandledOutcome_NeedsLock(Outcome 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 outcome, bool manual, ResilienceContext context, out Task? scheduledTask) => OpenCircuitFor_NeedsLock(outcome, _breakDuration, manual, context, out scheduledTask); diff --git a/src/Polly.Core/PublicAPI.Unshipped.txt b/src/Polly.Core/PublicAPI.Unshipped.txt index ab058de62d4..f82785466ab 100644 --- a/src/Polly.Core/PublicAPI.Unshipped.txt +++ b/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? diff --git a/test/Polly.Core.Tests/CircuitBreaker/BrokenCircuitExceptionTests.cs b/test/Polly.Core.Tests/CircuitBreaker/BrokenCircuitExceptionTests.cs index ded9279f4d1..dc2211fa63e 100644 --- a/test/Polly.Core.Tests/CircuitBreaker/BrokenCircuitExceptionTests.cs +++ b/test/Polly.Core.Tests/CircuitBreaker/BrokenCircuitExceptionTests.cs @@ -7,10 +7,27 @@ public class BrokenCircuitExceptionTests [Fact] public void Ctor_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(); + var retryAfter = new TimeSpan(1, 0, 0); + var defaultException = new BrokenCircuitException(); + defaultException.Message.Should().Be("The circuit is now open and is not allowing calls."); + defaultException.RetryAfter.Should().BeNull(); + var retryAfterException = new BrokenCircuitException(retryAfter); + retryAfterException.Message.Should().Be($"The circuit is now open and is not allowing calls. It can be retried after '{retryAfter}'."); + retryAfterException.RetryAfter.Should().Be(retryAfter); + var dummyMessageException = new BrokenCircuitException("Dummy."); + dummyMessageException.Message.Should().Be("Dummy."); + dummyMessageException.RetryAfter.Should().BeNull(); + var dummyMessageWithRetryAfterException = new BrokenCircuitException("Dummy.", retryAfter); + dummyMessageWithRetryAfterException.Message.Should().Be("Dummy."); + dummyMessageWithRetryAfterException.RetryAfter.Should().Be(retryAfter); + var dummyMessageExceptionWithInnerException = new BrokenCircuitException("Dummy.", new InvalidOperationException()); + dummyMessageExceptionWithInnerException.Message.Should().Be("Dummy."); + dummyMessageExceptionWithInnerException.InnerException.Should().BeOfType(); + dummyMessageExceptionWithInnerException.RetryAfter.Should().BeNull(); + var dummyMessageExceptionWithInnerExceptionAndRetryAfter = new BrokenCircuitException("Dummy.", retryAfter, new InvalidOperationException()); + dummyMessageExceptionWithInnerExceptionAndRetryAfter.Message.Should().Be("Dummy."); + dummyMessageExceptionWithInnerExceptionAndRetryAfter.InnerException.Should().BeOfType(); + dummyMessageExceptionWithInnerExceptionAndRetryAfter.RetryAfter.Should().Be(retryAfter); } #if !NETCOREAPP diff --git a/test/Polly.Core.Tests/CircuitBreaker/CircuitBreakerResiliencePipelineBuilderTests.cs b/test/Polly.Core.Tests/CircuitBreaker/CircuitBreakerResiliencePipelineBuilderTests.cs index 5cee0687360..c5c1c809a83 100644 --- a/test/Polly.Core.Tests/CircuitBreaker/CircuitBreakerResiliencePipelineBuilderTests.cs +++ b/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(() => strategy.Execute(_ => 0)); + BrokenCircuitException exception = Assert.Throws(() => 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(() => strategy.Execute(_ => 0)); + exception.RetryAfter.Should().Be(halfBreakDuration); // Circuit Half Opened - timeProvider.Advance(breakDuration); + timeProvider.Advance(halfBreakDuration); strategy.Execute(_ => -1); - Assert.Throws(() => strategy.Execute(_ => 0)); + exception = Assert.Throws(() => 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(() => strategy.Execute(_ => 0)); + BrokenCircuitException exception = Assert.Throws(() => 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(() => strategy.Execute(_ => 0)); + exception.RetryAfter.Should().Be(halfBreakDuration); // Circuit Half Opened - timeProvider.Advance(breakDuration); + timeProvider.Advance(halfBreakDuration); strategy.Execute(_ => -1); - Assert.Throws(() => strategy.Execute(_ => 0)); + exception = Assert.Throws(() => 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(); - strategy2.Invoking(s => s.Execute(() => { })).Should().Throw(); + strategy1.Invoking(s => s.Execute(() => { })).Should().Throw().Where(e => e.RetryAfter == null); + strategy2.Invoking(s => s.Execute(() => { })).Should().Throw().Where(e => e.RetryAfter == null); await manualControl.CloseAsync(); diff --git a/test/Polly.Core.Tests/CircuitBreaker/CircuitBreakerResilienceStrategyTests.cs b/test/Polly.Core.Tests/CircuitBreaker/CircuitBreakerResilienceStrategyTests.cs index 500b830aee4..7a0748683d5 100644 --- a/test/Polly.Core.Tests/CircuitBreaker/CircuitBreakerResilienceStrategyTests.cs +++ b/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(); + strategy.Invoking(s => s.Execute(_ => 0)).Should().Throw().Where(e => e.RetryAfter == null); await _options.ManualControl.CloseAsync(CancellationToken.None); diff --git a/test/Polly.Core.Tests/CircuitBreaker/Controller/CircuitStateControllerTests.cs b/test/Polly.Core.Tests/CircuitBreaker/Controller/CircuitStateControllerTests.cs index dc993b44f32..7a623544629 100644 --- a/test/Polly.Core.Tests/CircuitBreaker/Controller/CircuitStateControllerTests.cs +++ b/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(); + outcome.Value.Exception.Should().BeOfType() + .And.Subject.As().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(); + error.Should().BeOfType().And.Subject.As().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(new InvalidOperationException())); var error = (BrokenCircuitException)(await controller.OnActionPreExecuteAsync(ResilienceContextPool.Shared.Get())).Value.Exception!; error.InnerException.Should().BeOfType(); + 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(); + error.Should().BeOfType().And.Subject.As().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(); + outcome.Value.Exception.Should().BeOfType().And.Subject.As().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(); + outcome.Value.Exception.Should().BeOfType() + .And.Subject.As().RetryAfter.Should().Be(_options.BreakDuration - advanceTimeRejected); // wait and try, transition to half open AdvanceTime(_options.BreakDuration + _options.BreakDuration); diff --git a/test/Polly.Core.Tests/CircuitBreaker/IsolatedCircuitExceptionTests.cs b/test/Polly.Core.Tests/CircuitBreaker/IsolatedCircuitExceptionTests.cs index 5e381a06e3b..6698f8148cc 100644 --- a/test/Polly.Core.Tests/CircuitBreaker/IsolatedCircuitExceptionTests.cs +++ b/test/Polly.Core.Tests/CircuitBreaker/IsolatedCircuitExceptionTests.cs @@ -7,10 +7,16 @@ public class IsolatedCircuitExceptionTests [Fact] public void Ctor_Ok() { - new IsolatedCircuitException("Dummy.").Message.Should().Be("Dummy."); - new IsolatedCircuitException().Message.Should().Be("The circuit is manually held open and is not allowing calls."); - new IsolatedCircuitException("Dummy.", new InvalidOperationException()).Message.Should().Be("Dummy."); - new IsolatedCircuitException("Dummy.", new InvalidOperationException()).InnerException.Should().BeOfType(); + var dummyMessageException = new IsolatedCircuitException("Dummy."); + dummyMessageException.Message.Should().Be("Dummy."); + dummyMessageException.RetryAfter.Should().BeNull(); + var defaultException = new IsolatedCircuitException(); + defaultException.Message.Should().Be("The circuit is manually held open and is not allowing calls."); + defaultException.RetryAfter.Should().BeNull(); + var dummyMessageExceptionWithInnerException = new IsolatedCircuitException("Dummy.", new InvalidOperationException()); + dummyMessageExceptionWithInnerException.Message.Should().Be("Dummy."); + dummyMessageExceptionWithInnerException.InnerException.Should().BeOfType(); + dummyMessageExceptionWithInnerException.RetryAfter.Should().BeNull(); } #if !NETCOREAPP From cd57a4e08beb0869b9959562ca5bba73e2ec7dd9 Mon Sep 17 00:00:00 2001 From: David Lee Date: Sat, 5 Oct 2024 00:00:25 +0800 Subject: [PATCH 2/3] Refactor circuit breaker exception constructor tests. --- .../BrokenCircuitExceptionTests.cs | 71 +++++++++++++------ .../IsolatedCircuitExceptionTests.cs | 34 ++++++--- 2 files changed, 72 insertions(+), 33 deletions(-) diff --git a/test/Polly.Core.Tests/CircuitBreaker/BrokenCircuitExceptionTests.cs b/test/Polly.Core.Tests/CircuitBreaker/BrokenCircuitExceptionTests.cs index dc2211fa63e..d386e4288bf 100644 --- a/test/Polly.Core.Tests/CircuitBreaker/BrokenCircuitExceptionTests.cs +++ b/test/Polly.Core.Tests/CircuitBreaker/BrokenCircuitExceptionTests.cs @@ -5,29 +5,53 @@ namespace Polly.Core.Tests.CircuitBreaker; public class BrokenCircuitExceptionTests { [Fact] - public void Ctor_Ok() + public void Ctor_Default_Ok() { - var retryAfter = new TimeSpan(1, 0, 0); - var defaultException = new BrokenCircuitException(); - defaultException.Message.Should().Be("The circuit is now open and is not allowing calls."); - defaultException.RetryAfter.Should().BeNull(); - var retryAfterException = new BrokenCircuitException(retryAfter); - retryAfterException.Message.Should().Be($"The circuit is now open and is not allowing calls. It can be retried after '{retryAfter}'."); - retryAfterException.RetryAfter.Should().Be(retryAfter); - var dummyMessageException = new BrokenCircuitException("Dummy."); - dummyMessageException.Message.Should().Be("Dummy."); - dummyMessageException.RetryAfter.Should().BeNull(); - var dummyMessageWithRetryAfterException = new BrokenCircuitException("Dummy.", retryAfter); - dummyMessageWithRetryAfterException.Message.Should().Be("Dummy."); - dummyMessageWithRetryAfterException.RetryAfter.Should().Be(retryAfter); - var dummyMessageExceptionWithInnerException = new BrokenCircuitException("Dummy.", new InvalidOperationException()); - dummyMessageExceptionWithInnerException.Message.Should().Be("Dummy."); - dummyMessageExceptionWithInnerException.InnerException.Should().BeOfType(); - dummyMessageExceptionWithInnerException.RetryAfter.Should().BeNull(); - var dummyMessageExceptionWithInnerExceptionAndRetryAfter = new BrokenCircuitException("Dummy.", retryAfter, new InvalidOperationException()); - dummyMessageExceptionWithInnerExceptionAndRetryAfter.Message.Should().Be("Dummy."); - dummyMessageExceptionWithInnerExceptionAndRetryAfter.InnerException.Should().BeOfType(); - dummyMessageExceptionWithInnerExceptionAndRetryAfter.RetryAfter.Should().Be(retryAfter); + 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(); + 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(); + exception.RetryAfter.Should().Be(TestRetryAfter); } #if !NETCOREAPP @@ -35,4 +59,7 @@ public void Ctor_Ok() public void BinarySerialization_Ok() => BinarySerializationUtil.SerializeAndDeserializeException(new BrokenCircuitException()).Should().NotBeNull(); #endif + + private const string TestMessage = "Dummy."; + private static readonly TimeSpan TestRetryAfter = TimeSpan.FromHours(1); } diff --git a/test/Polly.Core.Tests/CircuitBreaker/IsolatedCircuitExceptionTests.cs b/test/Polly.Core.Tests/CircuitBreaker/IsolatedCircuitExceptionTests.cs index 6698f8148cc..09413c8a1ee 100644 --- a/test/Polly.Core.Tests/CircuitBreaker/IsolatedCircuitExceptionTests.cs +++ b/test/Polly.Core.Tests/CircuitBreaker/IsolatedCircuitExceptionTests.cs @@ -5,18 +5,28 @@ namespace Polly.Core.Tests.CircuitBreaker; public class IsolatedCircuitExceptionTests { [Fact] - public void Ctor_Ok() + public void Ctor_Default_Ok() { - var dummyMessageException = new IsolatedCircuitException("Dummy."); - dummyMessageException.Message.Should().Be("Dummy."); - dummyMessageException.RetryAfter.Should().BeNull(); - var defaultException = new IsolatedCircuitException(); - defaultException.Message.Should().Be("The circuit is manually held open and is not allowing calls."); - defaultException.RetryAfter.Should().BeNull(); - var dummyMessageExceptionWithInnerException = new IsolatedCircuitException("Dummy.", new InvalidOperationException()); - dummyMessageExceptionWithInnerException.Message.Should().Be("Dummy."); - dummyMessageExceptionWithInnerException.InnerException.Should().BeOfType(); - dummyMessageExceptionWithInnerException.RetryAfter.Should().BeNull(); + var exception = new IsolatedCircuitException(); + exception.Message.Should().Be("The circuit is manually held open and is not allowing calls."); + exception.RetryAfter.Should().BeNull(); + } + + [Fact] + public void Ctor_Message_Ok() + { + var exception = new IsolatedCircuitException(TestMessage); + exception.Message.Should().Be(TestMessage); + exception.RetryAfter.Should().BeNull(); + } + + [Fact] + public void Ctor_Message_InnerException_Ok() + { + var exception = new IsolatedCircuitException(TestMessage, new InvalidOperationException()); + exception.Message.Should().Be(TestMessage); + exception.InnerException.Should().BeOfType(); + exception.RetryAfter.Should().BeNull(); } #if !NETCOREAPP @@ -24,4 +34,6 @@ public void Ctor_Ok() public void BinarySerialization_Ok() => BinarySerializationUtil.SerializeAndDeserializeException(new IsolatedCircuitException("dummy")).Should().NotBeNull(); #endif + + private const string TestMessage = "Dummy."; } From 9a938452c424be1069235aed8ac448b855235d7b Mon Sep 17 00:00:00 2001 From: David Lee Date: Sat, 5 Oct 2024 00:04:25 +0800 Subject: [PATCH 3/3] Add binary serialization support for circuit breaker exception RetryAfter property. --- .../CircuitBreaker/BrokenCircuitException.cs | 26 +++++++++++++++++++ .../BrokenCircuitExceptionTests.cs | 22 ++++++++++++++-- .../IsolatedCircuitExceptionTests.cs | 11 ++++++-- 3 files changed, 55 insertions(+), 4 deletions(-) diff --git a/src/Polly.Core/CircuitBreaker/BrokenCircuitException.cs b/src/Polly.Core/CircuitBreaker/BrokenCircuitException.cs index 744bf1d31a8..6238950613c 100644 --- a/src/Polly.Core/CircuitBreaker/BrokenCircuitException.cs +++ b/src/Polly.Core/CircuitBreaker/BrokenCircuitException.cs @@ -76,6 +76,32 @@ public BrokenCircuitException(string message, TimeSpan retryAfter, Exception inn 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; + } + } + } + + /// + 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 diff --git a/test/Polly.Core.Tests/CircuitBreaker/BrokenCircuitExceptionTests.cs b/test/Polly.Core.Tests/CircuitBreaker/BrokenCircuitExceptionTests.cs index d386e4288bf..70f9dc128bc 100644 --- a/test/Polly.Core.Tests/CircuitBreaker/BrokenCircuitExceptionTests.cs +++ b/test/Polly.Core.Tests/CircuitBreaker/BrokenCircuitExceptionTests.cs @@ -56,8 +56,26 @@ public void Ctor_Message_RetryAfter_InnerException_Ok() #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(); + 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(); + roundtripResult.RetryAfter.Should().BeNull(); + } #endif private const string TestMessage = "Dummy."; diff --git a/test/Polly.Core.Tests/CircuitBreaker/IsolatedCircuitExceptionTests.cs b/test/Polly.Core.Tests/CircuitBreaker/IsolatedCircuitExceptionTests.cs index 09413c8a1ee..7a76a623229 100644 --- a/test/Polly.Core.Tests/CircuitBreaker/IsolatedCircuitExceptionTests.cs +++ b/test/Polly.Core.Tests/CircuitBreaker/IsolatedCircuitExceptionTests.cs @@ -31,8 +31,15 @@ public void Ctor_Message_InnerException_Ok() #if !NETCOREAPP [Fact] - public void BinarySerialization_Ok() => - BinarySerializationUtil.SerializeAndDeserializeException(new IsolatedCircuitException("dummy")).Should().NotBeNull(); + public void BinarySerialization_Ok() + { + var exception = new IsolatedCircuitException(TestMessage, new InvalidOperationException()); + IsolatedCircuitException roundtripResult = BinarySerializationUtil.SerializeAndDeserializeException(exception); + roundtripResult.Should().NotBeNull(); + roundtripResult.Message.Should().Be(TestMessage); + roundtripResult.InnerException.Should().BeOfType(); + roundtripResult.RetryAfter.Should().BeNull(); + } #endif private const string TestMessage = "Dummy.";