Alpha fixes and improvements (#1319)

bench/BenchmarkDotNet.Artifacts/results/Polly.Core.Benchmarks.MultipleStrategiesBenchmark-report-github.md

@@ -1,15 +1,15 @@
 ``` ini
 
-BenchmarkDotNet=v0.13.5, OS=Windows 11 (10.0.22621.1702/22H2/2022Update/SunValley2)
-Intel Core i9-10885H CPU 2.40GHz, 1 CPU, 16 logical and 8 physical cores
-.NET SDK=7.0.203
-  [Host] : .NET 7.0.5 (7.0.523.17405), X64 RyuJIT AVX2
+BenchmarkDotNet=v0.13.5, OS=Windows 11 (10.0.22621.1848/22H2/2022Update/SunValley2), VM=Hyper-V
+Intel Xeon Platinum 8370C CPU 2.80GHz, 1 CPU, 16 logical and 8 physical cores
+.NET SDK=7.0.304
+  [Host] : .NET 7.0.7 (7.0.723.27404), X64 RyuJIT AVX2
 
 Job=MediumRun  Toolchain=InProcessEmitToolchain  IterationCount=15  
 LaunchCount=2  WarmupCount=10  
 
 ```
-|                     Method |     Mean |     Error |    StdDev | Ratio | RatioSD |   Gen0 | Allocated | Alloc Ratio |
-|--------------------------- |---------:|----------:|----------:|------:|--------:|-------:|----------:|------------:|
-| ExecuteStrategyPipeline_V7 | 2.027 μs | 0.0282 μs | 0.0413 μs |  1.00 |    0.00 | 0.3357 |    2824 B |        1.00 |
-| ExecuteStrategyPipeline_V8 | 1.783 μs | 0.0254 μs | 0.0372 μs |  0.88 |    0.03 | 0.0038 |      40 B |        0.01 |
+|                     Method |     Mean |     Error |    StdDev | Ratio |   Gen0 | Allocated | Alloc Ratio |
+|--------------------------- |---------:|----------:|----------:|------:|-------:|----------:|------------:|
+| ExecuteStrategyPipeline_V7 | 2.227 μs | 0.0077 μs | 0.0116 μs |  1.00 | 0.1106 |    2824 B |        1.00 |
+| ExecuteStrategyPipeline_V8 | 1.750 μs | 0.0060 μs | 0.0084 μs |  0.79 |      - |      40 B |        0.01 |

bench/Polly.Core.Benchmarks/Utils/Helper.Retry.cs

@@ -21,7 +21,7 @@ public static object CreateRetry(PollyVersion technology)
                     RetryCount = 3,
                     BackoffType = RetryBackoffType.Constant,
                     BaseDelay = delay,
-                    ShouldRetry = args => args switch
+                    ShouldHandle = args => args switch
                     {
                         { Exception: InvalidOperationException } => PredicateResult.True,
                         { Result: string result } when result == Failure => PredicateResult.True,

src/Polly.Core/CircuitBreaker/CircuitBreakerStrategyOptions.cs

@@ -41,12 +41,7 @@ public abstract class CircuitBreakerStrategyOptions<TResult> : ResilienceStrateg
     /// This property is required.
     /// </remarks>
     [Required]
-    public Func<OutcomeArguments<TResult, CircuitBreakerPredicateArguments>, ValueTask<bool>> ShouldHandle { get; set; } = args => args.Exception switch
-    {
-        OperationCanceledException => PredicateResult.False,
-        Exception => PredicateResult.True,
-        _ => PredicateResult.False
-    };
+    public Func<OutcomeArguments<TResult, CircuitBreakerPredicateArguments>, ValueTask<bool>> ShouldHandle { get; set; } = DefaultPredicates<CircuitBreakerPredicateArguments, TResult>.HandleOutcome;
 
     /// <summary>
     /// Gets or sets the event that is raised when the circuit resets to a <see cref="CircuitState.Closed"/> state.

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

@@ -1,4 +1,3 @@
-using System.Runtime.ExceptionServices;
 using Polly.Telemetry;
 
 namespace Polly.CircuitBreaker;
@@ -151,7 +150,7 @@ public ValueTask CloseCircuitAsync(ResilienceContext context)
 
         if (exception is not null)
         {
-            return new Outcome<TResult>(ExceptionDispatchInfo.Capture(exception));
+            return new Outcome<TResult>(exception);
         }
 
         return null;
@@ -305,6 +304,8 @@ private void SetLastHandledOutcome_NeedsLock<TResult>(Outcome<TResult> outcome)
         {
             _breakingException = new BrokenCircuitException<TResult>(BrokenCircuitException.DefaultMessage, result!);
         }
+
+        _breakingException?.TrySetStackTrace();
     }
 
     private BrokenCircuitException GetBreakingException_NeedsLock() => _breakingException ?? new BrokenCircuitException();

src/Polly.Core/CircuitBreaker/OnCircuitClosedArguments.cs

@@ -4,7 +4,4 @@ namespace Polly.CircuitBreaker;
 /// Arguments used by <see cref="CircuitBreakerStrategyOptions{TResult}.OnClosed"/> event.
 /// </summary>
 /// <param name="IsManual">Indicates whether the circuit was closed manually by using <see cref="CircuitBreakerManualControl"/>.</param>
-/// <remarks>
-/// Always use the constructor when creating this struct, otherwise we do not guarantee binary compatibility.
-/// </remarks>
-public readonly record struct OnCircuitClosedArguments(bool IsManual);
+public record OnCircuitClosedArguments(bool IsManual);

src/Polly.Core/CircuitBreaker/OnCircuitHalfOpenedArguments.cs

@@ -3,7 +3,4 @@ namespace Polly.CircuitBreaker;
 /// <summary>
 /// Arguments used by <see cref="CircuitBreakerStrategyOptions{TResult}.OnHalfOpened"/> event.
 /// </summary>
-/// <remarks>
-/// Always use the constructor when creating this struct, otherwise we do not guarantee binary compatibility.
-/// </remarks>
-public readonly record struct OnCircuitHalfOpenedArguments();
+public record OnCircuitHalfOpenedArguments();

src/Polly.Core/CircuitBreaker/OnCircuitOpenedArguments.cs

@@ -5,7 +5,4 @@ namespace Polly.CircuitBreaker;
 /// </summary>
 /// <param name="BreakDuration">The duration of break.</param>
 /// <param name="IsManual">Indicates whether the circuit was opened manually by using <see cref="CircuitBreakerManualControl"/>.</param>
-/// <remarks>
-/// Always use the constructor when creating this struct, otherwise we do not guarantee binary compatibility.
-/// </remarks>
-public readonly record struct OnCircuitOpenedArguments(TimeSpan BreakDuration, bool IsManual);
+public record OnCircuitOpenedArguments(TimeSpan BreakDuration, bool IsManual);

src/Polly.Core/Fallback/FallbackHandler.cs

@@ -1,8 +1,8 @@
 namespace Polly.Fallback;
 
 internal sealed record class FallbackHandler<T>(
-    PredicateInvoker<HandleFallbackArguments> ShouldHandle,
-    Func<OutcomeArguments<T, HandleFallbackArguments>, ValueTask<Outcome<T>>> ActionGenerator,
+    PredicateInvoker<FallbackPredicateArguments> ShouldHandle,
+    Func<OutcomeArguments<T, FallbackPredicateArguments>, ValueTask<Outcome<T>>> ActionGenerator,
     bool IsGeneric)
 {
     public bool HandlesFallback<TResult>() => IsGeneric switch
@@ -11,9 +11,9 @@ internal sealed record class FallbackHandler<T>(
         false => true
     };
 
-    public async ValueTask<Outcome<TResult>> GetFallbackOutcomeAsync<TResult>(OutcomeArguments<TResult, HandleFallbackArguments> args)
+    public async ValueTask<Outcome<TResult>> GetFallbackOutcomeAsync<TResult>(OutcomeArguments<TResult, FallbackPredicateArguments> args)
     {
-        var copiedArgs = new OutcomeArguments<T, HandleFallbackArguments>(
+        var copiedArgs = new OutcomeArguments<T, FallbackPredicateArguments>(
             args.Context,
             args.Outcome.AsOutcome<T>(),
             args.Arguments);

src/Polly.Core/Fallback/HandleFallbackArguments.cs → src/Polly.Core/Fallback/FallbackPredicateArguments.cs

@@ -6,4 +6,4 @@ namespace Polly.Fallback;
 /// <remarks>
 /// Always use the constructor when creating this struct, otherwise we do not guarantee binary compatibility.
 /// </remarks>
-public readonly record struct HandleFallbackArguments();
+public readonly record struct FallbackPredicateArguments();

src/Polly.Core/Fallback/FallbackResilienceStrategy.cs

@@ -1,4 +1,3 @@
-using System.Runtime.ExceptionServices;
 using Polly.Telemetry;
 
 namespace Polly.Fallback;
@@ -29,7 +28,7 @@ protected internal override async ValueTask<Outcome<TResult>> ExecuteCoreAsync<T
         }
 
         var outcome = await ExecuteCallbackSafeAsync(callback, context, state).ConfigureAwait(context.ContinueOnCapturedContext);
-        var handleFallbackArgs = new OutcomeArguments<TResult, HandleFallbackArguments>(context, outcome, new HandleFallbackArguments());
+        var handleFallbackArgs = new OutcomeArguments<TResult, FallbackPredicateArguments>(context, outcome, new FallbackPredicateArguments());
         if (!await _handler.ShouldHandle.HandleAsync(handleFallbackArgs).ConfigureAwait(context.ContinueOnCapturedContext))
         {
             return outcome;
@@ -50,7 +49,7 @@ protected internal override async ValueTask<Outcome<TResult>> ExecuteCoreAsync<T
         }
         catch (Exception e)
         {
-            return new Outcome<TResult>(ExceptionDispatchInfo.Capture(e));
+            return new Outcome<TResult>(e);
         }
     }
 }

src/Polly.Core/Fallback/FallbackResilienceStrategyBuilderExtensions.cs

@@ -20,7 +20,7 @@ public static class FallbackResilienceStrategyBuilderExtensions
     public static ResilienceStrategyBuilder<TResult> AddFallback<TResult>(
         this ResilienceStrategyBuilder<TResult> builder,
         Action<PredicateBuilder<TResult>> shouldHandle,
-        Func<OutcomeArguments<TResult, HandleFallbackArguments>, ValueTask<Outcome<TResult>>> fallbackAction)
+        Func<OutcomeArguments<TResult, FallbackPredicateArguments>, ValueTask<Outcome<TResult>>> fallbackAction)
     {
         Guard.NotNull(builder);
         Guard.NotNull(shouldHandle);
@@ -34,7 +34,7 @@ public static ResilienceStrategyBuilder<TResult> AddFallback<TResult>(
         var predicateBuilder = new PredicateBuilder<TResult>();
         shouldHandle(predicateBuilder);
 
-        options.ShouldHandle = predicateBuilder.CreatePredicate<HandleFallbackArguments>();
+        options.ShouldHandle = predicateBuilder.CreatePredicate<FallbackPredicateArguments>();
 
         return builder.AddFallback(options);
     }

src/Polly.Core/Fallback/FallbackStrategyOptions.TResult.cs

@@ -18,10 +18,11 @@ public class FallbackStrategyOptions<TResult> : ResilienceStrategyOptions
     /// Gets or sets the outcome predicate for determining whether a fallback should be executed.
     /// </summary>
     /// <remarks>
-    /// This property is required. Defaults to <see langword="null"/>.
+    /// Defaults to a delegate that hedges on any exception except <see cref="OperationCanceledException"/>.
+    /// This property is required.
     /// </remarks>
     [Required]
-    public Func<OutcomeArguments<TResult, HandleFallbackArguments>, ValueTask<bool>>? ShouldHandle { get; set; }
+    public Func<OutcomeArguments<TResult, FallbackPredicateArguments>, ValueTask<bool>> ShouldHandle { get; set; } = DefaultPredicates<FallbackPredicateArguments, TResult>.HandleOutcome;
 
     /// <summary>
     /// Gets or sets the fallback action to be executed when the <see cref="ShouldHandle"/> predicate evaluates as true.
@@ -30,7 +31,7 @@ public class FallbackStrategyOptions<TResult> : ResilienceStrategyOptions
     /// This property is required. Defaults to <see langword="null"/>.
     /// </remarks>
     [Required]
-    public Func<OutcomeArguments<TResult, HandleFallbackArguments>, ValueTask<Outcome<TResult>>>? FallbackAction { get; set; }
+    public Func<OutcomeArguments<TResult, FallbackPredicateArguments>, ValueTask<Outcome<TResult>>>? FallbackAction { get; set; }
 
     /// <summary>
     /// Gets or sets the outcome event instance responsible for triggering fallback events.

src/Polly.Core/Fallback/OnFallbackArguments.cs

@@ -3,7 +3,4 @@ namespace Polly.Fallback;
 /// <summary>
 /// Represents arguments used in fallback handling scenarios.
 /// </summary>
-/// <remarks>
-/// Always use the constructor when creating this struct, otherwise we do not guarantee binary compatibility.
-/// </remarks>
-public readonly record struct OnFallbackArguments();
+public record OnFallbackArguments();

src/Polly.Core/Hedging/Controller/HedgingExecutionContext.cs

@@ -8,7 +8,7 @@ namespace Polly.Hedging.Utils;
 /// The context associated with an execution of hedging resilience strategy.
 /// It holds the resources for all executed hedged tasks (primary + secondary) and is responsible for resource disposal.
 /// </summary>
-internal sealed class HedgingExecutionContext<T>
+internal sealed class HedgingExecutionContext<T> : IAsyncDisposable
 {
     public readonly record struct ExecutionInfo<TResult>(TaskExecution<T>? Execution, bool Loaded, Outcome<TResult>? Outcome);
 
@@ -81,7 +81,7 @@ public async ValueTask<ExecutionInfo<TResult>> LoadExecutionAsync<TResult, TStat
         }
     }
 
-    public void Complete()
+    public async ValueTask DisposeAsync()
     {
         UpdateOriginalContext();
 
@@ -91,10 +91,14 @@ public void Complete()
             pair.Cancel();
         }
 
-        // We are intentionally doing the cleanup in the background as we do not want to
-        // delay the hedging.
-        // The background cleanup is safe. All exceptions are handled.
-        _ = CleanupInBackgroundAsync();
+        foreach (var task in _tasks)
+        {
+            await task.ExecutionTaskSafe!.ConfigureAwait(false);
+            await task.ResetAsync().ConfigureAwait(false);
+            _executionPool.Return(task);
+        }
+
+        Reset();
     }
 
     public async ValueTask<TaskExecution<T>?> TryWaitForCompletedExecutionAsync(TimeSpan hedgingDelay)
@@ -226,18 +230,6 @@ private void UpdateOriginalContext()
         }
     }
 
-    private async Task CleanupInBackgroundAsync()
-    {
-        foreach (var task in _tasks)
-        {
-            await task.ExecutionTaskSafe!.ConfigureAwait(false);
-            await task.ResetAsync().ConfigureAwait(false);
-            _executionPool.Return(task);
-        }
-
-        Reset();
-    }
-
     private void Reset()
     {
         _replacedProperties.Clear();

src/Polly.Core/Hedging/Controller/HedgingHandler.cs

@@ -1,7 +1,7 @@
 namespace Polly.Hedging.Utils;
 
 internal sealed record class HedgingHandler<T>(
-    PredicateInvoker<HandleHedgingArguments> ShouldHandle,
+    PredicateInvoker<HedgingPredicateArguments> ShouldHandle,
     Func<HedgingActionGeneratorArguments<T>, Func<ValueTask<Outcome<T>>>?> ActionGenerator,
     bool IsGeneric)
 {

src/Polly.Core/Hedging/Controller/TaskExecution.cs

@@ -208,7 +208,7 @@ private async Task ExecutePrimaryActionAsync<TResult, TState>(Func<ResilienceCon
 
     private async Task UpdateOutcomeAsync<TResult>(Outcome<TResult> outcome)
     {
-        var args = new OutcomeArguments<TResult, HandleHedgingArguments>(Context, outcome, new HandleHedgingArguments());
+        var args = new OutcomeArguments<TResult, HedgingPredicateArguments>(Context, outcome, new HedgingPredicateArguments());
         Outcome = outcome.AsOutcome();
         IsHandled = await _handler.ShouldHandle.HandleAsync(args).ConfigureAwait(Context.ContinueOnCapturedContext);
     }

src/Polly.Core/Hedging/HandleHedgingArguments.cs → src/Polly.Core/Hedging/HedgingPredicateArguments.cs

@@ -6,4 +6,4 @@ namespace Polly.Hedging;
 /// <remarks>
 /// Always use the constructor when creating this struct, otherwise we do not guarantee binary compatibility.
 /// </remarks>
-public readonly record struct HandleHedgingArguments();
+public readonly record struct HedgingPredicateArguments();