Document outcome strategy anti-patterns

docs/chaos/outcome.md

@@ -9,19 +9,18 @@
 
 ---
 
-The outcome chaos strategy is designed to inject or substitute fake results into system operations. This allows testing how an application behaves when it receives different types of responses, like successful results, errors, or exceptions.
+The outcome chaos strategy is designed to inject or substitute fake results into system operations. This allows testing how an application behaves when it receives different types of responses, like successful or error results.
 
 ## Usage
 
 <!-- snippet: chaos-outcome-usage -->
 ```cs
-// To use OutcomeGenerator<T> to register the results and exceptions to be injected (equal probability)
+// To use OutcomeGenerator<T> to register the results to be injected (equal probability)
 var optionsWithResultGenerator = new ChaosOutcomeStrategyOptions<HttpResponseMessage>
 {
     OutcomeGenerator = new OutcomeGenerator<HttpResponseMessage>()
         .AddResult(() => new HttpResponseMessage(HttpStatusCode.TooManyRequests))
         .AddResult(() => new HttpResponseMessage(HttpStatusCode.InternalServerError))
-        .AddException(() => new HttpRequestException("Chaos request exception.")),
     InjectionRate = 0.1
 };
 
@@ -132,24 +131,23 @@ To generate a faulted outcome (result or exception), you need to specify a `Outc
 
 ### Use `OutcomeGenerator<T>` class to generate outcomes
 
-The `OutcomeGenerator<T>` is a convenience API that allows you to specify what outcomes (results or exceptions) are to be injected. Additionally, it also allows assigning weight to each registered outcome.
+The `OutcomeGenerator<T>` is a convenience API that allows you to specify what outcomes are to be injected. Additionally, it also allows assigning weight to each registered outcome.
 
 <!-- snippet: chaos-outcome-generator-class -->
 ```cs
 new ResiliencePipelineBuilder<HttpResponseMessage>()
     .AddChaosOutcome(new ChaosOutcomeStrategyOptions<HttpResponseMessage>
     {
-        // Use OutcomeGenerator<T> to register the results and exceptions to be injected
+        // Use OutcomeGenerator<T> to register the results to be injected
         OutcomeGenerator = new OutcomeGenerator<HttpResponseMessage>()
             .AddResult(() => new HttpResponseMessage(HttpStatusCode.InternalServerError)) // Result generator
             .AddResult(() => new HttpResponseMessage(HttpStatusCode.TooManyRequests), weight: 50) // Result generator with weight
             .AddResult(context => CreateResultFromContext(context)) // Access the ResilienceContext to create result
-            .AddException<HttpRequestException>(), // You can also register exceptions
     });
 ```
 <!-- endSnippet -->
 
-### Use delegates to generate faults
+### Use delegates to generate outcomes
 
 Delegates give you the most flexibility at the expense of slightly more complicated syntax. Delegates also support asynchronous outcome generation, if you ever need that possibility.
 
@@ -160,18 +158,117 @@ new ResiliencePipelineBuilder<HttpResponseMessage>()
     {
         // The same behavior can be achieved with delegates
         OutcomeGenerator = args =>
+        {
+            Outcome<HttpResponseMessage>? outcome = Random.Shared.Next(350) switch
+            {
+                < 100 => Outcome.FromResult(new HttpResponseMessage(HttpStatusCode.InternalServerError)),
+                < 150 => Outcome.FromResult(new HttpResponseMessage(HttpStatusCode.TooManyRequests)),
+                < 350 => Outcome.FromResult(CreateResultFromContext(args.Context)),
+                _ => Outcome.FromResult(new HttpResponseMessage(HttpStatusCode.OK))
+            };
+
+            return ValueTask.FromResult(outcome);
+        }
+    });
+```
+<!-- endSnippet -->
+
+## Anti-patterns
+
+### Injecting faults (exceptions)
+
+❌ DON'T
+
+Use outcome strategies to inject faults in advanced scenarios which you need to inject outcomes using delegates. This is an opinionated anti-pattern since you can consider an exception as a result/outcome, however, there might be undesired implications when doing so, one of them is the telemetry events, which might end up affecting your metrics as the `ChaosOutcomeStrategy` reports both result and exceptions in the same way, and this could pose a problem for instrumentation purposes since it's clearer looking for fault injected events to be 100% sure where/when exceptions were injected, rather than have them mixed in the same "bag".
+
+Also, you end up losing control of how/when to inject outcomes vs faults since this way does not allow you to control separately when to inject a fault vs an outcome.
+
+<!-- snippet: chaos-outcome-anti-pattern-inject-fault -->
+```cs
+var pipeline = new ResiliencePipelineBuilder<HttpResponseMessage>()
+    .AddChaosOutcome(new ChaosOutcomeStrategyOptions<HttpResponseMessage>
+    {
+        InjectionRate = 0.5, // same injection rate for both fault and outcome
+        OutcomeGenerator = args =>
         {
             Outcome<HttpResponseMessage>? outcome = Random.Shared.Next(350) switch
             {
                 < 100 => Outcome.FromResult(new HttpResponseMessage(HttpStatusCode.InternalServerError)),
                 < 150 => Outcome.FromResult(new HttpResponseMessage(HttpStatusCode.TooManyRequests)),
                 < 250 => Outcome.FromResult(CreateResultFromContext(args.Context)),
-                < 350 => Outcome.FromException<HttpResponseMessage>(new TimeoutException()),
+                < 350 => Outcome.FromException<HttpResponseMessage>(new HttpRequestException("Chaos request exception.")),
+                _ => Outcome.FromResult(new HttpResponseMessage(HttpStatusCode.OK))
+            };
+
+            return ValueTask.FromResult(outcome);
+        },
+        OnOutcomeInjected = static args =>
+        {
+            // You might have to put some logic here to determine what kind of output was injected. 😕
+            if (args.Outcome.Exception != null)
+            {
+                Console.WriteLine($"OnBehaviorInjected, Exception: {args.Outcome.Exception.Message}, Operation: {args.Context.OperationKey}.");
+            }
+            else
+            {
+                Console.WriteLine($"OnBehaviorInjected, Outcome: {args.Outcome.Result}, Operation: {args.Context.OperationKey}.");
+            }
+
+            return default;
+        }
+    })
+    .Build();
+```
+<!-- endSnippet -->
+
+✅ DO
+
+The previous approach is tempting since it looks more succinct, but use the fault chaos instead as the [`ChaosFaultStrategy`](fault.md) correctly tracks telemetry events effectively as faults not just as any other outcome, also by separating them you can control the injection rate and enable/disable them separately which gives you more control when it comes to injecting chaos dynamically and in a controlled manner.
+
+<!-- snippet: chaos-outcome-pattern-inject-fault -->
+```cs
+var randomThreshold = Random.Shared.Next(350);
+var pipeline = new ResiliencePipelineBuilder<HttpResponseMessage>()
+    .AddChaosFault(new ChaosFaultStrategyOptions
+    {
+        InjectionRate = 0.1, // different injection rate for faults
+        FaultGenerator = args =>
+        {
+            Exception? exception = randomThreshold switch
+            {
+                >= 250 and < 350 => new HttpRequestException("Chaos request exception."),
                 _ => null
             };
 
+            return new ValueTask<Exception?>(exception);
+        },
+        OnFaultInjected = static args =>
+        {
+            Console.WriteLine($"OnFaultInjected, Exception: {args.Fault.Message}, Operation: {args.Context.OperationKey}.");
+            return default;
+        }
+    })
+    .AddChaosOutcome(new ChaosOutcomeStrategyOptions<HttpResponseMessage>
+    {
+        InjectionRate = 0.5, // different injection rate for outcomes
+        OutcomeGenerator = args =>
+        {
+            Outcome<HttpResponseMessage>? outcome = randomThreshold switch
+            {
+                < 100 => Outcome.FromResult(new HttpResponseMessage(HttpStatusCode.InternalServerError)),
+                < 150 => Outcome.FromResult(new HttpResponseMessage(HttpStatusCode.TooManyRequests)),
+                < 250 => Outcome.FromResult(CreateResultFromContext(args.Context)),
+                _ => Outcome.FromResult(new HttpResponseMessage(HttpStatusCode.OK))
+            };
+
             return ValueTask.FromResult(outcome);
+        },
+        OnOutcomeInjected = static args =>
+        {
+            Console.WriteLine($"OnBehaviorInjected, Outcome: {args.Outcome.Result}, Operation: {args.Context.OperationKey}.");
+            return default;
         }
-    });
+    })
+    .Build();
 ```
 <!-- endSnippet -->

src/Polly.Core/Simmy/Outcomes/OutcomeGenerator.cs

@@ -27,7 +27,7 @@
     }
 
     internal OutcomeGenerator(Func<int, int> weightGenerator)
         => _helper = new GeneratorHelper<TResult>(weightGenerator);
 
     /// <summary>
     /// Registers an exception generator delegate.
@@ -35,6 +35,9 @@
     /// <param name="generator">The delegate that generates the exception.</param>
     /// <param name="weight">The weight assigned to this generator. Defaults to <c>100</c>.</param>
     /// <returns>The current instance of <see cref="OutcomeGenerator{TResult}"/>.</returns>
+#pragma warning disable S1133 // Deprecated code should be removed
+    [Obsolete("This method is deprecated and will be removed in the next version. Use Chaos fault strategy instead.")]
+#pragma warning restore S1133 // Deprecated code should be removed
     public OutcomeGenerator<TResult> AddException(Func<Exception> generator, int weight = DefaultWeight)
     {
         Guard.NotNull(generator);
@@ -42,7 +45,7 @@
         _helper.AddOutcome(_ => Outcome.FromException<TResult>(generator()), weight);
 
         return this;
     }
 
     /// <summary>
     /// Registers an exception generator delegate that accepts a <see cref="ResilienceContext"/>.
@@ -50,6 +53,9 @@
     /// <param name="generator">The delegate that generates the exception, accepting a <see cref="ResilienceContext"/>.</param>
     /// <param name="weight">The weight assigned to this generator. Defaults to <c>100</c>.</param>
     /// <returns>The current instance of <see cref="OutcomeGenerator{TResult}"/>.</returns>
+#pragma warning disable S1133 // Deprecated code should be removed
+    [Obsolete("This method is deprecated and will be removed in the next version. Use Chaos fault strategy instead.")]
+#pragma warning restore S1133 // Deprecated code should be removed
     public OutcomeGenerator<TResult> AddException(Func<ResilienceContext, Exception> generator, int weight = DefaultWeight)
     {
         Guard.NotNull(generator);
@@ -57,7 +63,7 @@
         _helper.AddOutcome(context => Outcome.FromException<TResult>(generator(context)), weight);
 
         return this;
     }
 
     /// <summary>
     /// Registers an exception generator for a specific exception type, using the default constructor of that exception.
@@ -65,6 +71,9 @@
     /// <typeparam name="TException">The type of the exception to generate.</typeparam>
     /// <param name="weight">The weight assigned to this generator. Defaults to <c>100</c>.</param>
     /// <returns>The current instance of <see cref="OutcomeGenerator{TResult}"/>.</returns>
+#pragma warning disable S1133 // Deprecated code should be removed
+    [Obsolete("This method is deprecated and will be removed in the next version. Use Chaos fault strategy instead.")]
+#pragma warning restore S1133 // Deprecated code should be removed
     public OutcomeGenerator<TResult> AddException<TException>(int weight = DefaultWeight)
         where TException : Exception, new()
     {

src/Snippets/Docs/Chaos.Outcome.cs

@@ -1,7 +1,8 @@
-using System.Net;
+using System.Net;
 using System.Net.Http;
 using Polly.Retry;
 using Polly.Simmy;
+using Polly.Simmy.Fault;
 using Polly.Simmy.Outcomes;
 
 namespace Snippets.Docs;
@@ -18,8 +19,7 @@
         {
             OutcomeGenerator = new OutcomeGenerator<HttpResponseMessage>()
                 .AddResult(() => new HttpResponseMessage(HttpStatusCode.TooManyRequests))
                 .AddResult(() => new HttpResponseMessage(HttpStatusCode.InternalServerError))
-                .AddException(() => new HttpRequestException("Chaos request exception.")),
             InjectionRate = 0.1
         };
 
@@ -83,7 +83,6 @@
                     .AddResult(() => new HttpResponseMessage(HttpStatusCode.InternalServerError)) // Result generator
                     .AddResult(() => new HttpResponseMessage(HttpStatusCode.TooManyRequests), weight: 50) // Result generator with weight
                     .AddResult(context => CreateResultFromContext(context)) // Access the ResilienceContext to create result
-                    .AddException<HttpRequestException>(), // You can also register exceptions
             });
 
         #endregion
@@ -105,7 +104,7 @@
                         < 150 => Outcome.FromResult(new HttpResponseMessage(HttpStatusCode.TooManyRequests)),
                         < 250 => Outcome.FromResult(CreateResultFromContext(args.Context)),
                         < 350 => Outcome.FromException<HttpResponseMessage>(new TimeoutException()),
-                        _ => null
+                        _ => Outcome.FromResult(new HttpResponseMessage(HttpStatusCode.OK))
                     };
 
                     return ValueTask.FromResult(outcome);
@@ -115,5 +114,94 @@
         #endregion
     }
 
+    public static void AntiPattern_InjectFault()
+    {
+        #region chaos-outcome-anti-pattern-inject-fault
+        var pipeline = new ResiliencePipelineBuilder<HttpResponseMessage>()
+            .AddChaosOutcome(new ChaosOutcomeStrategyOptions<HttpResponseMessage>
+            {
+                InjectionRate = 0.5, // same injection rate for both fault and outcome
+                OutcomeGenerator = args =>
+                {
+                    Outcome<HttpResponseMessage>? outcome = Random.Shared.Next(350) switch
+                    {
+                        < 100 => Outcome.FromResult(new HttpResponseMessage(HttpStatusCode.InternalServerError)),
+                        < 150 => Outcome.FromResult(new HttpResponseMessage(HttpStatusCode.TooManyRequests)),
+                        < 250 => Outcome.FromResult(CreateResultFromContext(args.Context)),
+                        < 350 => Outcome.FromException<HttpResponseMessage>(new HttpRequestException("Chaos request exception.")),
+                        _ => Outcome.FromResult(new HttpResponseMessage(HttpStatusCode.OK))
+                    };
+
+                    return ValueTask.FromResult(outcome);
+                },
+                OnOutcomeInjected = static args =>
+                {
+                    // You might have to put some logic here to determine what kind of output was injected. 😕
+                    if (args.Outcome.Exception != null)
+                    {
+                        Console.WriteLine($"OnBehaviorInjected, Exception: {args.Outcome.Exception.Message}, Operation: {args.Context.OperationKey}.");
+                    }
+                    else
+                    {
+                        Console.WriteLine($"OnBehaviorInjected, Outcome: {args.Outcome.Result}, Operation: {args.Context.OperationKey}.");
+                    }
+
+                    return default;
+                }
+            })
+            .Build();
+
+        #endregion
+    }
+
+    public static void Pattern_InjectFault()
+    {
+        #region chaos-outcome-pattern-inject-fault
+        var randomThreshold = Random.Shared.Next(350);
+        var pipeline = new ResiliencePipelineBuilder<HttpResponseMessage>()
+            .AddChaosFault(new ChaosFaultStrategyOptions
+            {
+                InjectionRate = 0.1, // different injection rate for faults
+                FaultGenerator = args =>
+                {
+                    Exception? exception = randomThreshold switch
+                    {
+                        >= 250 and < 350 => new HttpRequestException("Chaos request exception."),
+                        _ => null
+                    };
+
+                    return new ValueTask<Exception?>(exception);
+                },
+                OnFaultInjected = static args =>
+                {
+                    Console.WriteLine($"OnFaultInjected, Exception: {args.Fault.Message}, Operation: {args.Context.OperationKey}.");
+                    return default;
+                }
+            })
+            .AddChaosOutcome(new ChaosOutcomeStrategyOptions<HttpResponseMessage>
+            {
+                InjectionRate = 0.5, // different injection rate for outcomes
+                OutcomeGenerator = args =>
+                {
+                    Outcome<HttpResponseMessage>? outcome = randomThreshold switch
+                    {
+                        < 100 => Outcome.FromResult(new HttpResponseMessage(HttpStatusCode.InternalServerError)),
+                        < 150 => Outcome.FromResult(new HttpResponseMessage(HttpStatusCode.TooManyRequests)),
+                        < 250 => Outcome.FromResult(CreateResultFromContext(args.Context)),
+                        _ => Outcome.FromResult(new HttpResponseMessage(HttpStatusCode.OK))
+                    };
+
+                    return ValueTask.FromResult(outcome);
+                },
+                OnOutcomeInjected = static args =>
+                {
+                    Console.WriteLine($"OnBehaviorInjected, Outcome: {args.Outcome.Result}, Operation: {args.Context.OperationKey}.");
+                    return default;
+                }
+            })
+            .Build();
+        #endregion
+    }
+
     private static HttpResponseMessage CreateResultFromContext(ResilienceContext context) => new(HttpStatusCode.TooManyRequests);
 }