Fix stack trace growing for opened circuit breaker (#1878)

App-vNext · Jan 3, 2024 · b145338 · b145338
1 parent 502094f
commit b145338
Show file tree

Hide file tree

Showing 3 changed files with 49 additions and 12 deletions.
diff --git a/src/Polly.Core/CircuitBreaker/Controller/CircuitStateController.cs b/src/Polly.Core/CircuitBreaker/Controller/CircuitStateController.cs
@@ -20,7 +20,7 @@ internal sealed class CircuitStateController<T> : IDisposable
     private DateTimeOffset _blockedUntil;
     private CircuitState _circuitState = CircuitState.Closed;
     private Outcome<T>? _lastOutcome;
-    private BrokenCircuitException _breakingException = new();
+    private Exception? _breakingException;
     private bool _disposed;
 
 #pragma warning disable S107
@@ -139,8 +139,8 @@ public ValueTask CloseCircuitAsync(ResilienceContext context)
 
             exception = _circuitState switch
             {
-                CircuitState.Open => _breakingException,
-                CircuitState.HalfOpen when !isHalfOpen => _breakingException,
+                CircuitState.Open => CreateBrokenCircuitException(),
+                CircuitState.HalfOpen when !isHalfOpen => CreateBrokenCircuitException(),
                 CircuitState.Isolated => new IsolatedCircuitException(),
                 _ => null
             };
@@ -302,17 +302,15 @@ private bool PermitHalfOpenCircuitTest_NeedsLock()
     private void SetLastHandledOutcome_NeedsLock(Outcome<T> outcome)
     {
         _lastOutcome = outcome;
-
-        if (outcome.Exception is Exception exception)
-        {
-            _breakingException = new BrokenCircuitException(BrokenCircuitException.DefaultMessage, exception);
-        }
-        else
-        {
-            _breakingException = new BrokenCircuitException(BrokenCircuitException.DefaultMessage);
-        }
+        _breakingException = outcome.Exception;
     }
 
+    private BrokenCircuitException CreateBrokenCircuitException() => _breakingException switch
+    {
+        Exception exception => new BrokenCircuitException(BrokenCircuitException.DefaultMessage, exception),
+        _ => new BrokenCircuitException(BrokenCircuitException.DefaultMessage)
+    };
+
     private void OpenCircuit_NeedsLock(Outcome<T> outcome, bool manual, ResilienceContext context, out Task? scheduledTask)
     {
         OpenCircuitFor_NeedsLock(outcome, _breakDuration, manual, context, out scheduledTask);

diff --git a/test/Polly.Core.Tests/CircuitBreaker/BrokenCircuitExceptionTests.cs b/test/Polly.Core.Tests/CircuitBreaker/BrokenCircuitExceptionTests.cs
@@ -7,6 +7,7 @@ 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<InvalidOperationException>();

diff --git a/test/Polly.Core.Tests/CircuitBreaker/Controller/CircuitStateControllerTests.cs b/test/Polly.Core.Tests/CircuitBreaker/Controller/CircuitStateControllerTests.cs
@@ -124,6 +124,44 @@ public async Task OnActionPreExecute_CircuitOpenedByValue()
         GetBlockedTill(controller).Should().Be(_timeProvider.GetUtcNow() + _options.BreakDuration);
     }
 
+    [InlineData(true)]
+    [InlineData(false)]
+    [Theory]
+    public async Task OnActionPreExecute_CircuitOpened_EnsureExceptionStackTraceDoesNotGrow(bool innerException)
+    {
+        var stacks = new List<string>();
+        var context = ResilienceContextPool.Shared.Get();
+        using var controller = CreateController();
+
+        await OpenCircuit(
+            controller,
+            innerException ? Outcome.FromException<int>(new InvalidOperationException()) : Outcome.FromResult(99));
+
+        for (int i = 0; i < 100; i++)
+        {
+            try
+            {
+                (await controller.OnActionPreExecuteAsync(context)).Value.ThrowIfException();
+            }
+            catch (BrokenCircuitException e)
+            {
+                stacks.Add(e.StackTrace!);
+                e.Message.Should().Be("The circuit is now open and is not allowing calls.");
+
+                if (innerException)
+                {
+                    e.InnerException.Should().BeOfType<InvalidOperationException>();
+                }
+                else
+                {
+                    e.InnerException.Should().BeNull();
+                }
+            }
+        }
+
+        stacks.Distinct().Should().HaveCount(1);
+    }
+
     [Fact]
     public async Task HalfOpen_EnsureBreakDuration()
     {