[Docs] Add antipatterns to circuit breaker documentation page (#1621)

docs/strategies/circuit-breaker.md

@@ -99,3 +99,234 @@ await manualControl.CloseAsync();
 - [Circuit Breaker by Martin Fowler](https://martinfowler.com/bliki/CircuitBreaker.html)
 - [Circuit Breaker Pattern by Microsoft](https://msdn.microsoft.com/en-us/library/dn589784.aspx)
 - [Original Circuit Breaking Article](https://web.archive.org/web/20160106203951/http://thatextramile.be/blog/2008/05/the-circuit-breaker)
+
+
+## Patterns and Anti-patterns
+Throughout the years many people have used Polly in so many different ways. Some reoccuring patterns are suboptimal. So, this section shows the donts and dos.
+
+### 1 - Using different sleep duration between retry attempts based on Circuit Breaker state
+
+Imagine that we have an inner Circuit Breaker and an outer Retry strategies.
+We would like to define the retry in a way that the sleep duration calculation is taking into account the Circuit Breaker's state.
+
+❌ DON'T
+
+Use closure to branch based on circuit breaker state:
+
+<!-- snippet: circuit-breaker-anti-pattern-1 -->
+```cs
+var stateProvider = new CircuitBreakerStateProvider();
+var circuitBreaker = new ResiliencePipelineBuilder()
+    .AddCircuitBreaker(new()
+    {
+        ShouldHandle = new PredicateBuilder().Handle<HttpRequestException>(),
+        BreakDuration = TimeSpan.FromSeconds(5),
+        StateProvider = stateProvider
+    })
+    .Build();
+
+var retry = new ResiliencePipelineBuilder()
+    .AddRetry(new()
+    {
+        ShouldHandle = new PredicateBuilder()
+            .Handle<HttpRequestException>()
+            .Handle<BrokenCircuitException>(),
+        DelayGenerator = args =>
+        {
+            TimeSpan? delay = TimeSpan.FromSeconds(1);
+            if (stateProvider.CircuitState == CircuitState.Open)
+            {
+                delay = TimeSpan.FromSeconds(5);
+            }
+
+            return ValueTask.FromResult(delay);
+        }
+    })
+    .Build();
+```
+<!-- endSnippet -->
+
+**Reasoning**:
+
+- By default, each strategy is independent and has no any reference to other strategies.
+- We use the (`stateProvider`) to access the Circuit Breaker's state. However, this approach is not optimal as the retry strategy's `DelayGenerator` varies based on state.
+- This solution is delicate because the break duration and the sleep duration aren't linked:
+- If a future code maintainer modifies the `circuitBreaker`'s `BreakDuration`, they might overlook adjusting the sleep duration.
+
+✅ DO
+
+Use `Context` to pass information between strategies:
+
+<!-- snippet: circuit-breaker-pattern-1 -->
+```cs
+var circuitBreaker = new ResiliencePipelineBuilder()
+    .AddCircuitBreaker(new()
+    {
+        ShouldHandle = new PredicateBuilder().Handle<HttpRequestException>(),
+        BreakDuration = TimeSpan.FromSeconds(5),
+        OnOpened = static args =>
+        {
+            args.Context.Properties.Set(SleepDurationKey, args.BreakDuration);
+            return ValueTask.CompletedTask;
+        },
+        OnClosed = args =>
+        {
+            args.Context.Properties.Set(SleepDurationKey, null);
+            return ValueTask.CompletedTask;
+        }
+    })
+    .Build();
+
+var retry = new ResiliencePipelineBuilder()
+    .AddRetry(new()
+    {
+        ShouldHandle = new PredicateBuilder()
+            .Handle<HttpRequestException>()
+            .Handle<BrokenCircuitException>(),
+        DelayGenerator = static args =>
+        {
+            _ = args.Context.Properties.TryGetValue(SleepDurationKey, out var delay);
+            delay ??= TimeSpan.FromSeconds(1);
+            return ValueTask.FromResult(delay);
+        }
+    })
+    .Build();
+```
+<!-- endSnippet -->
+
+**Reasoning**:
+
+- Both strategies are less coupled in this approach since they rely on the context and the `sleepDurationKey` components.
+- The Circuit Breaker shares the `BreakDuration` through the context when it breaks.
+When it transitions back to Closed, the sharring is revoked.
+- The Retry strategy fetches the sleep duration dyanmically whithout knowing any specific knowledge about the Circuit Breaker.
+- If adjustments are needed for the `BreakDuration`, they can be made in one place.
+
+### 2 - Using different duration for breaks
+
+In case of Retry you can specify dynamically the sleep duration via the `DelayGenerator`.
+In case of Circuit Breaker the `BreakDuration` is considered constant (can't be changed between breaks).
+
+❌ DON'T
+
+Use `Task.Delay` inside `OnOpened`:
+
+<!-- snippet: circuit-breaker-anti-pattern-2 -->
+```cs
+static IEnumerable<TimeSpan> GetSleepDuration()
+{
+    for (int i = 1; i < 10; i++)
+    {
+        yield return TimeSpan.FromSeconds(i);
+    }
+}
+
+var sleepDurationProvider = GetSleepDuration().GetEnumerator();
+sleepDurationProvider.MoveNext();
+
+var circuitBreaker = new ResiliencePipelineBuilder()
+    .AddCircuitBreaker(new()
+    {
+        ShouldHandle = new PredicateBuilder().Handle<HttpRequestException>(),
+        BreakDuration = TimeSpan.FromSeconds(0.5),
+        OnOpened = async args =>
+        {
+            await Task.Delay(sleepDurationProvider.Current);
+            sleepDurationProvider.MoveNext();
+        }
+
+    })
+    .Build();
+```
+<!-- endSnippet -->
+
+**Reasoning**:
+
+- The minimum break duration value is half second. This implies that each sleep lasts for `sleepDurationProvider.Current` plus an additional half second.
+- One might think that setting the `BreakDuration` to `sleepDurationProvider.Current` would addres this, but it doesn't. This is because the `BreakDuration` is established only once and  isn't reassessed during each break.
+
+<!-- snippet: circuit-breaker-anti-pattern-2-ext -->
+```cs
+circuitBreaker = new ResiliencePipelineBuilder()
+    .AddCircuitBreaker(new()
+    {
+        ShouldHandle = new PredicateBuilder().Handle<HttpRequestException>(),
+        BreakDuration = sleepDurationProvider.Current,
+        OnOpened = async args =>
+        {
+            Console.WriteLine($"Break: {sleepDurationProvider.Current}");
+            sleepDurationProvider.MoveNext();
+        }
+
+    })
+    .Build();
+```
+<!-- endSnippet -->
+
+✅ DO
+
+The `CircuitBreakerStartegyOptions` currently do not support defining break durations dynamically. This may be re-evaluted in the future. For now, refer to the first example for a potential workaround. However, please use it with caution.
+
+### 3 - Wrapping each endpoint with a circuit breaker
+
+Imagine that you have to call N number of services via `HttpClient`s.
+You want to decorate all downstream calls with the service-aware Circuit Breaker.
+
+❌ DON'T
+
+Use a collection of Circuit Breakers and explicitly call `ExecuteAsync`:
+
+<!-- snippet: circuit-breaker-anti-pattern-3 -->
+```cs
+// Defined in a common place
+var uriToCbMappings = new Dictionary<Uri, ResiliencePipeline>
+{
+    { new Uri("https://downstream1.com"), GetCircuitBreaker() },
+    // ...
+    { new Uri("https://downstreamN.com"), GetCircuitBreaker() }
+};
+
+// Used in the downstream 1 client
+var downstream1Uri = new Uri("https://downstream1.com");
+await uriToCbMappings[downstream1Uri].ExecuteAsync(CallXYZOnDownstream1, CancellationToken.None);
+```
+<!-- endSnippet -->
+
+**Reasoning**:
+
+- Whenever you use an `HttpClient`, you must have a reference to the  `uriToCbMappings`.
+- It's your responsibility to decorate each network call with the corresponding circuit breaker.
+
+✅ DO
+
+Use named and typed `HttpClient`s:
+
+```cs
+foreach (string uri in uris)
+{
+    builder.Services
+      .AddHttpClient<IResilientClient, ResilientClient>(uri, client => client.BaseAddress = new Uri(uri))
+      .AddPolicyHandler(GetCircuitBreaker().AsAsyncPolicy<HttpResponseMessage>());
+}
+
+...
+private const string serviceUrl = "https://downstream1.com";
+public Downstream1Client(
+   IHttpClientFactory namedClientFactory,
+   ITypedHttpClientFactory<ResilientClient> typedClientFactory)
+{
+    var namedClient = namedClientFactory.CreateClient(serviceUrl);
+    var namedTypedClient = typedClientFactory.CreateClient(namedClient);
+    ...
+}
+```
+
+**Reasoning**:
+
+- The `HttpClient` integrates with Circuit Breaker during startup.
+- There's no need to call `ExecuteAsync` directly. The `DelegatingHandler` handles it automatically.
+
+> [!NOTE]
+> The above sample code used the `AsAsyncPolicy<HttpResponseMessage>()` method to convert the `ResiliencePipeline<HttpResponseMessage>` to `IAsyncPolicy<HttpResponseMessage>`.
+> It is required because the `AddPolicyHandler` anticipates an `IAsyncPolicy<HttpResponse>` parameter.
+> Please be aware that, later an `AddResilienceHandler` will be introduced in the `Microsoft.Extensions.Http.Resilience` package which is the successor of the `Microsoft.Extensions.Http.Polly`.

src/Snippets/Docs/CircuitBreaker.cs

@@ -66,4 +66,148 @@ public static async Task Usage()
 
         #endregion
     }
+
+    public static void AntiPattern_1()
+    {
+        #region circuit-breaker-anti-pattern-1
+        var stateProvider = new CircuitBreakerStateProvider();
+        var circuitBreaker = new ResiliencePipelineBuilder()
+            .AddCircuitBreaker(new()
+            {
+                ShouldHandle = new PredicateBuilder().Handle<HttpRequestException>(),
+                BreakDuration = TimeSpan.FromSeconds(5),
+                StateProvider = stateProvider
+            })
+            .Build();
+
+        var retry = new ResiliencePipelineBuilder()
+            .AddRetry(new()
+            {
+                ShouldHandle = new PredicateBuilder()
+                    .Handle<HttpRequestException>()
+                    .Handle<BrokenCircuitException>(),
+                DelayGenerator = args =>
+                {
+                    TimeSpan? delay = TimeSpan.FromSeconds(1);
+                    if (stateProvider.CircuitState == CircuitState.Open)
+                    {
+                        delay = TimeSpan.FromSeconds(5);
+                    }
+
+                    return ValueTask.FromResult(delay);
+                }
+            })
+            .Build();
+
+        #endregion
+    }
+
+    private static readonly ResiliencePropertyKey<TimeSpan?> SleepDurationKey = new("sleep_duration");
+    public static void Pattern_1()
+    {
+        #region circuit-breaker-pattern-1
+        var circuitBreaker = new ResiliencePipelineBuilder()
+            .AddCircuitBreaker(new()
+            {
+                ShouldHandle = new PredicateBuilder().Handle<HttpRequestException>(),
+                BreakDuration = TimeSpan.FromSeconds(5),
+                OnOpened = static args =>
+                {
+                    args.Context.Properties.Set(SleepDurationKey, args.BreakDuration);
+                    return ValueTask.CompletedTask;
+                },
+                OnClosed = args =>
+                {
+                    args.Context.Properties.Set(SleepDurationKey, null);
+                    return ValueTask.CompletedTask;
+                }
+            })
+            .Build();
+
+        var retry = new ResiliencePipelineBuilder()
+            .AddRetry(new()
+            {
+                ShouldHandle = new PredicateBuilder()
+                    .Handle<HttpRequestException>()
+                    .Handle<BrokenCircuitException>(),
+                DelayGenerator = static args =>
+                {
+                    _ = args.Context.Properties.TryGetValue(SleepDurationKey, out var delay);
+                    delay ??= TimeSpan.FromSeconds(1);
+                    return ValueTask.FromResult(delay);
+                }
+            })
+            .Build();
+
+        #endregion
+    }
+
+    public static void AntiPattern_2()
+    {
+        #region circuit-breaker-anti-pattern-2
+        static IEnumerable<TimeSpan> GetSleepDuration()
+        {
+            for (int i = 1; i < 10; i++)
+            {
+                yield return TimeSpan.FromSeconds(i);
+            }
+        }
+
+        var sleepDurationProvider = GetSleepDuration().GetEnumerator();
+        sleepDurationProvider.MoveNext();
+
+        var circuitBreaker = new ResiliencePipelineBuilder()
+            .AddCircuitBreaker(new()
+            {
+                ShouldHandle = new PredicateBuilder().Handle<HttpRequestException>(),
+                BreakDuration = TimeSpan.FromSeconds(0.5),
+                OnOpened = async args =>
+                {
+                    await Task.Delay(sleepDurationProvider.Current);
+                    sleepDurationProvider.MoveNext();
+                }
+
+            })
+            .Build();
+
+        #endregion
+
+        #region circuit-breaker-anti-pattern-2-ext
+
+        circuitBreaker = new ResiliencePipelineBuilder()
+            .AddCircuitBreaker(new()
+            {
+                ShouldHandle = new PredicateBuilder().Handle<HttpRequestException>(),
+                BreakDuration = sleepDurationProvider.Current,
+                OnOpened = async args =>
+                {
+                    Console.WriteLine($"Break: {sleepDurationProvider.Current}");
+                    sleepDurationProvider.MoveNext();
+                }
+
+            })
+            .Build();
+
+        #endregion
+    }
+
+    public static async ValueTask AntiPattern_3()
+    {
+        static ValueTask CallXYZOnDownstream1(CancellationToken ct) => ValueTask.CompletedTask;
+        static ResiliencePipeline GetCircuitBreaker() => ResiliencePipeline.Empty;
+
+        #region circuit-breaker-anti-pattern-3
+        // Defined in a common place
+        var uriToCbMappings = new Dictionary<Uri, ResiliencePipeline>
+        {
+            { new Uri("https://downstream1.com"), GetCircuitBreaker() },
+            // ...
+            { new Uri("https://downstreamN.com"), GetCircuitBreaker() }
+        };
+
+        // Used in the downstream 1 client
+        var downstream1Uri = new Uri("https://downstream1.com");
+        await uriToCbMappings[downstream1Uri].ExecuteAsync(CallXYZOnDownstream1, CancellationToken.None);
+        #endregion
+    }
 }

src/Snippets/Docs/Fallback.cs

@@ -200,7 +200,7 @@ private static ValueTask<HttpResponseMessage> ActionCore()
     }
 
     private static ValueTask<HttpResponseMessage> CallExternalSystem(CancellationToken ct) => ValueTask.FromResult(new HttpResponseMessage());
-    public static async ValueTask<HttpResponseMessage?> Anti_Pattern_3()
+    public static async ValueTask<HttpResponseMessage?> AntiPattern_3()
     {
         var timeout = ResiliencePipeline<HttpResponseMessage>.Empty;
         var fallback = ResiliencePipeline<HttpResponseMessage>.Empty;