[Bug]: HedgingResilienceStrategyTests.ExecuteAsync_OnHedgingEventThrows_EnsureExceptionRethrown fails on debug mode

[gabidabet]

Describe the bug

When I run tests from Visual Studio with debug configuration, the tests Polly.Core.Tests.Hedging.HedgingResilienceStrategyTests.ExecuteAsync_OnHedgingEventThrows_EnsureExceptionRethrown fails.

Expected behavior

All tests must pass.

Actual behavior

the test Polly.Core.Tests.Hedging.HedgingResilienceStrategyTests.ExecuteAsync_OnHedgingEventThrows_EnsureExceptionRethrown fails with an exception on debug mode

Steps to reproduce

Use Visual Studio Runner to run tests with debug configuration.

Exception(s) (if any)

Message: 
Expected a <System.InvalidOperationException> to be thrown, but found <Microsoft.VisualStudio.TestPlatform.TestHost.DebugAssertException>:
Microsoft.VisualStudio.TestPlatform.TestHost.DebugAssertException: Method Debug.Fail failed with 'There must be exactly one accepted outcome for hedging. Found 0.
', and was translated to Microsoft.VisualStudio.TestPlatform.TestHost.DebugAssertException to avoid terminating the process hosting the test.
   at Microsoft.VisualStudio.TestPlatform.TestHost.TestHostTraceListener.GetException(String message) in /_/src/testhost.x86/TestHostTraceListener.cs:line 48
   at Microsoft.VisualStudio.TestPlatform.TestHost.TestHostTraceListener.Fail(String message, String detailMessage) in /_/src/testhost.x86/TestHostTraceListener.cs:line 43
   at System.Diagnostics.TraceInternal.Fail(String message, String detailMessage)
   at System.Diagnostics.Debug.Fail(String message, String detailMessage)
   at Polly.Hedging.Utils.HedgingExecutionContext`1.UpdateOriginalContext() in C:\Users\pc\source\repos\Polly\src\Polly.Core\Hedging\Controller\HedgingExecutionContext.cs:line 202
   at Polly.Hedging.Utils.HedgingExecutionContext`1.DisposeAsync() in C:\Users\pc\source\repos\Polly\src\Polly.Core\Hedging\Controller\HedgingExecutionContext.cs:line 78
   at System.Runtime.CompilerServices.AsyncMethodBuilderCore.Start[TStateMachine](TStateMachine& stateMachine)
   at Polly.Hedging.Utils.HedgingExecutionContext`1.DisposeAsync()
   at Polly.Hedging.HedgingResilienceStrategy`1.ExecuteCore[TState](Func`3 callback, ResilienceContext context, TState state) in C:\Users\pc\source\repos\Polly\src\Polly.Core\Hedging\HedgingResilienceStrategy.cs:line 49
   at System.Runtime.CompilerServices.AsyncMethodBuilderCore.Start[TStateMachine](TStateMachine& stateMachine)
   at Polly.Hedging.HedgingResilienceStrategy`1.ExecuteCore[TState](Func`3 callback, ResilienceContext context, TState state)
   at Polly.Utils.Pipeline.BridgeComponent`1.ExecuteCore[TResult,TState](Func`3 callback, ResilienceContext context, TState state) in C:\Users\pc\source\repos\Polly\src\Polly.Core\Utils\Pipeline\BridgeComponent.TResult.cs:line 17
   at Polly.Utils.Pipeline.CompositeComponent.ExecuteCoreWithoutTelemetry[TResult,TState](Func`3 callback, ResilienceContext context, TState state) in C:\Users\pc\source\repos\Polly\src\Polly.Core\Utils\Pipeline\CompositeComponent.cs:line 96
   at Polly.Utils.Pipeline.CompositeComponent.ExecuteCore[TResult,TState](Func`3 callback, ResilienceContext context, TState state) in C:\Users\pc\source\repos\Polly\src\Polly.Core\Utils\Pipeline\CompositeComponent.cs:line 79
   at Polly.ResiliencePipeline.ExecuteAsync[TResult](Func`2 callback, CancellationToken cancellationToken)
   at System.Runtime.CompilerServices.AsyncMethodBuilderCore.Start[TStateMachine](TStateMachine& stateMachine)
   at Polly.ResiliencePipeline.ExecuteAsync[TResult](Func`2 callback, CancellationToken cancellationToken)
   at Polly.ResiliencePipeline`1.ExecuteAsync[TResult](Func`2 callback, CancellationToken cancellationToken) in C:\Users\pc\source\repos\Polly\src\Polly.Core\ResiliencePipelineT.Async.cs:line 84
   at Polly.Core.Tests.Hedging.HedgingResilienceStrategyTests.<>c.<ExecuteAsync_OnHedgingEventThrows_EnsureExceptionRethrown>b__31_3(ResiliencePipeline`1 s) in C:\Users\pc\source\repos\Polly\test\Polly.Core.Tests\Hedging\HedgingResilienceStrategyTests.cs:line 528
   at FluentAssertions.AssertionExtensions.<>c__DisplayClass2_0`2.<Invoking>b__0()
   at FluentAssertions.Specialized.AsyncFunctionAssertions`2.InvokeWithInterceptionAsync(Func`1 action)
   at System.Runtime.CompilerServices.AsyncMethodBuilderCore.Start[TStateMachine](TStateMachine& stateMachine)
   at FluentAssertions.Specialized.AsyncFunctionAssertions`2.InvokeWithInterceptionAsync(Func`1 action)
   at FluentAssertions.Specialized.AsyncFunctionAssertions`2.ThrowAsync[TException](String because, Object[] becauseArgs)
   at System.Runtime.CompilerServices.AsyncMethodBuilderCore.Start[TStateMachine](TStateMachine& stateMachine)
   at FluentAssertions.Specialized.AsyncFunctionAssertions`2.ThrowAsync[TException](String because, Object[] becauseArgs)
   at Polly.Core.Tests.Hedging.HedgingResilienceStrategyTests.ExecuteAsync_OnHedgingEventThrows_EnsureExceptionRethrown() in C:\Users\pc\source\repos\Polly\test\Polly.Core.Tests\Hedging\HedgingResilienceStrategyTests.cs:line 527
   at System.Runtime.CompilerServices.AsyncMethodBuilderCore.Start[TStateMachine](TStateMachine& stateMachine)
   at Polly.Core.Tests.Hedging.HedgingResilienceStrategyTests.ExecuteAsync_OnHedgingEventThrows_EnsureExceptionRethrown()
   at System.RuntimeMethodHandle.InvokeMethod(Object target, Void** arguments, Signature sig, Boolean isConstructor)
   at System.Reflection.MethodBaseInvoker.InvokeWithNoArgs(Object obj, BindingFlags invokeAttr)
   at System.Reflection.MethodBase.Invoke(Object obj, Object[] parameters)
   at Xunit.Sdk.TestInvoker`1.CallTestMethod(Object testClassInstance) in /_/src/xunit.execution/Sdk/Frameworks/Runners/TestInvoker.cs:line 149
   at Xunit.Sdk.TestInvoker`1.<>c__DisplayClass47_0.<<InvokeTestMethodAsync>b__1>d.MoveNext() in /_/src/xunit.execution/Sdk/Frameworks/Runners/TestInvoker.cs:line 256
   at System.Runtime.CompilerServices.AsyncMethodBuilderCore.Start[TStateMachine](TStateMachine& stateMachine)
   at Xunit.Sdk.TestInvoker`1.<>c__DisplayClass47_0.<InvokeTestMethodAsync>b__1()
   at Xunit.Sdk.ExecutionTimer.AggregateAsync(Func`1 asyncAction) in /_/src/xunit.execution/Sdk/Frameworks/ExecutionTimer.cs:line 48
   at System.Runtime.CompilerServices.AsyncMethodBuilderCore.Start[TStateMachine](TStateMachine& stateMachine)
   at Xunit.Sdk.ExecutionTimer.AggregateAsync(Func`1 asyncAction)
   at Xunit.Sdk.TestInvoker`1.<>c__DisplayClass47_0.<InvokeTestMethodAsync>b__0() in /_/src/xunit.execution/Sdk/Frameworks/Runners/TestInvoker.cs:line 215
   at Xunit.Sdk.ExceptionAggregator.RunAsync(Func`1 code) in /_/src/xunit.core/Sdk/ExceptionAggregator.cs:line 90
   at System.Runtime.CompilerServices.AsyncMethodBuilderCore.Start[TStateMachine](TStateMachine& stateMachine)
   at Xunit.Sdk.ExceptionAggregator.RunAsync(Func`1 code)
   at Xunit.Sdk.TestInvoker`1.InvokeTestMethodAsync(Object testClassInstance) in /_/src/xunit.execution/Sdk/Frameworks/Runners/TestInvoker.cs:line 214
   at System.Runtime.CompilerServices.AsyncMethodBuilderCore.Start[TStateMachine](TStateMachine& stateMachine)
   at Xunit.Sdk.TestInvoker`1.InvokeTestMethodAsync(Object testClassInstance)
   at Xunit.Sdk.XunitTestInvoker.InvokeTestMethodAsync(Object testClassInstance) in /_/src/xunit.execution/Sdk/Frameworks/Runners/XunitTestInvoker.cs:line 112
   at Xunit.Sdk.TestInvoker`1.<RunAsync>b__46_0() in /_/src/xunit.execution/Sdk/Frameworks/Runners/TestInvoker.cs:line 180
   at System.Runtime.CompilerServices.AsyncMethodBuilderCore.Start[TStateMachine](TStateMachine& stateMachine)
   at Xunit.Sdk.TestInvoker`1.<RunAsync>b__46_0()
   at Xunit.Sdk.ExceptionAggregator.RunAsync[T](Func`1 code) in /_/src/xunit.core/Sdk/ExceptionAggregator.cs:line 107
   at System.Runtime.CompilerServices.AsyncMethodBuilderCore.Start[TStateMachine](TStateMachine& stateMachine)
   at Xunit.Sdk.ExceptionAggregator.RunAsync[T](Func`1 code)
   at Xunit.Sdk.TestInvoker`1.RunAsync() in /_/src/xunit.execution/Sdk/Frameworks/Runners/TestInvoker.cs:line 163
   at Xunit.Sdk.XunitTestRunner.InvokeTestMethodAsync(ExceptionAggregator aggregator) in /_/src/xunit.execution/Sdk/Frameworks/Runners/XunitTestRunner.cs:line 88
   at Xunit.Sdk.XunitTestRunner.InvokeTestAsync(ExceptionAggregator aggregator) in /_/src/xunit.execution/Sdk/Frameworks/Runners/XunitTestRunner.cs:line 70
   at System.Runtime.CompilerServices.AsyncMethodBuilderCore.Start[TStateMachine](TStateMachine& stateMachine)
   at Xunit.Sdk.XunitTestRunner.InvokeTestAsync(ExceptionAggregator aggregator)
   at Xunit.Sdk.TestRunner`1.<>c__DisplayClass43_0.<RunAsync>b__0() in /_/src/xunit.execution/Sdk/Frameworks/Runners/TestRunner.cs:line 149
   at Xunit.Sdk.ExceptionAggregator.RunAsync[T](Func`1 code) in /_/src/xunit.core/Sdk/ExceptionAggregator.cs:line 107
   at System.Runtime.CompilerServices.AsyncMethodBuilderCore.Start[TStateMachine](TStateMachine& stateMachine)
   at Xunit.Sdk.ExceptionAggregator.RunAsync[T](Func`1 code)
   at Xunit.Sdk.TestRunner`1.RunAsync() in /_/src/xunit.execution/Sdk/Frameworks/Runners/TestRunner.cs:line 149
   at System.Runtime.CompilerServices.AsyncMethodBuilderCore.Start[TStateMachine](TStateMachine& stateMachine)
   at Xunit.Sdk.TestRunner`1.RunAsync()
   at Xunit.Sdk.XunitTestCaseRunner.RunTestAsync() in /_/src/xunit.execution/Sdk/Frameworks/Runners/XunitTestCaseRunner.cs:line 140
   at Xunit.Sdk.TestCaseRunner`1.RunAsync() in /_/src/xunit.execution/Sdk/Frameworks/Runners/TestCaseRunner.cs:line 82
   at System.Runtime.CompilerServices.AsyncMethodBuilderCore.Start[TStateMachine](TStateMachine& stateMachine)
   at Xunit.Sdk.TestCaseRunner`1.RunAsync()
   at Xunit.Sdk.XunitTestCase.RunAsync(IMessageSink diagnosticMessageSink, IMessageBus messageBus, Object[] constructorArguments, ExceptionAggregator aggregator, CancellationTokenSource cancellationTokenSource) in /_/src/xunit.execution/Sdk/Frameworks/XunitTestCase.cs:line 170
   at Xunit.Sdk.XunitTestMethodRunner.RunTestCaseAsync(IXunitTestCase testCase) in /_/src/xunit.execution/Sdk/Frameworks/Runners/XunitTestMethodRunner.cs:line 45
   at Xunit.Sdk.TestMethodRunner`1.RunTestCasesAsync() in /_/src/xunit.execution/Sdk/Frameworks/Runners/TestMethodRunner.cs:line 136
   at System.Runtime.CompilerServices.AsyncMethodBuilderCore.Start[TStateMachine](TStateMachine& stateMachine)
   at Xunit.Sdk.TestMethodRunner`1.RunTestCasesAsync()
   at Xunit.Sdk.TestMethodRunner`1.RunAsync() in /_/src/xunit.execution/Sdk/Frameworks/Runners/TestMethodRunner.cs:line 106
   at System.Runtime.CompilerServices.AsyncMethodBuilderCore.Start[TStateMachine](TStateMachine& stateMachin
... <truncated>

  Stack Trace: 
XUnit2TestFramework.Throw(String message)
TestFrameworkProvider.Throw(String message)
DefaultAssertionStrategy.HandleFailure(String message)
AssertionScope.FailWith(Func`1 failReasonFunc)
AssertionScope.FailWith(Func`1 failReasonFunc)
AssertionScope.FailWith(String message, Object[] args)
DelegateAssertionsBase`2.ThrowInternal[TException](Exception exception, String because, Object[] becauseArgs)
AsyncFunctionAssertions`2.ThrowAsync[TException](String because, Object[] becauseArgs)
HedgingResilienceStrategyTests.ExecuteAsync_OnHedgingEventThrows_EnsureExceptionRethrown() line 527
--- End of stack trace from previous location ---

  (result has additional output)

Polly version

main

.NET Version

net8.0 (sdk: 8.0.402)

Anything else?

No response

[peter-csala]

@martincostello please assign to me.

[peter-csala]

The good part

This happens only if you run the test against non-release code. For release code the strategy works as expected (you will see explanation bellow):

var pipeline = new ResiliencePipelineBuilder<string>()
.AddHedging(new HedgingStrategyOptions<string>()
{
	ShouldHandle = new PredicateBuilder<string>().HandleResult("foo"),
	OnHedging = _ => throw new InvalidOperationException("hey hi")
}).Build();

await pipeline.ExecuteAsync<string>(ct => ValueTask.FromResult<string>("foo"));

will output

Unhandled exception. System.InvalidOperationException: hey hi
   at Program.<>c.<Main>b__0_1(OnHedgingArguments`1 _)
   at Polly.Hedging.Controller.TaskExecution`1.HandleOnHedgingAsync(ResilienceContext primaryContext, Int32 attemptNumber) in /_/src/Polly.Core/Hedging/Controller/TaskExecution.cs:line 150
   at Polly.Hedging.Controller.TaskExecution`1.InitializeAsync[TState](HedgedTaskType type, ResilienceContext primaryContext, Func`3 primaryCallback, TState state, Int32 attemptNumber) in /_/src/Polly.Core/Hedging/Controller/TaskExecution.cs:line 127
   at Polly.Hedging.Utils.HedgingExecutionContext`1.LoadExecutionAsync[TState](Func`3 primaryCallback, TState state) in /_/src/Polly.Core/Hedging/Controller/HedgingExecutionContext.cs:line 62
   at Polly.Hedging.HedgingResilienceStrategy`1.ExecuteCoreAsync[TState](HedgingExecutionContext`1 hedgingContext, Func`3 callback, ResilienceContext context, TState state) in /_/src/Polly.Core/Hedging/HedgingResilienceStrategy.cs:line 72
   at Polly.Hedging.HedgingResilienceStrategy`1.ExecuteCore[TState](Func`3 callback, ResilienceContext context, TState state) in /_/src/Polly.Core/Hedging/HedgingResilienceStrategy.cs:line 45
   at Polly.Hedging.HedgingResilienceStrategy`1.ExecuteCore[TState](Func`3 callback, ResilienceContext context, TState state) in /_/src/Polly.Core/Hedging/HedgingResilienceStrategy.cs:line 49
   at Polly.Utils.Pipeline.BridgeComponent`1.<ConvertValueTask>g__ConvertValueTaskAsync|5_0[TTo](ValueTask`1 valueTask, ResilienceContext resilienceContext) in /_/src/Polly.Core/Utils/Pipeline/BridgeComponent.TResult.cs:line 51
   at Polly.ResiliencePipeline.ExecuteAsync[TResult](Func`2 callback, CancellationToken cancellationToken) in /_/src/Polly.Core/ResiliencePipeline.AsyncT.cs:line 170
   ...

The bad part

The happy path

Let's amend the OnHedging delegate to the following:

_options.OnHedging = _ => default;

1. After the OnHedging has been successfully executed the TaskExecution finishes the HandleOnHedgingAsync method.
2. Its caller the InitializeAsync continues the execution
3. Its caller the HedgingExecutionContext's LoadExecutionAsync continues the execution
4. And the execution continues its journey inside the HedgingResilienceStrategy's ExecuteCoreAsync

Finally we reach this code where we mark the TaskExecution as Accepted.

Polly/src/Polly.Core/Hedging/HedgingResilienceStrategy.cs

Lines 88 to 92 in 25e57e4

	if (!execution.IsHandled)
	{
	execution.AcceptOutcome();
	return outcome;
	}

The unhappy path

Let's run the original OnHedging delegate which throws an InvalidOperationException

1. There is no explicit exception handling inside the TaskExecution's HandleOnHedgingAsync and InitializeAsync methods so, the exception bubbles up to theHedgingExecutionContext
2. There is no explicit exception handling inside the HedgingExecutionContext's LoadExecutionAsync method so, the exception bubbles up to the HedgingResilienceStrategy
3. There is no explicit exception handling inside the ExecuteCoreAsync BUT there is a try-finally block inside the ExecuteCore which is its caller

Polly/src/Polly.Core/Hedging/HedgingResilienceStrategy.cs

Lines 43 to 50 in 25e57e4

	try
	{
	return await ExecuteCoreAsync(hedgingContext, callback, context, state).ConfigureAwait(context.ContinueOnCapturedContext);
	}
	finally
	{
	await hedgingContext.DisposeAsync().ConfigureAwait(context.ContinueOnCapturedContext);
	}

4. Inside the finally block we dispose the HedgingExecutionContext. Its DisposeAsync calls the UpdateOriginalContext
5. And since we did not have any AcceptOutcome calls on this execution path, that's why the test fails with a Microsoft.VisualStudio.TestPlatform.TestHost.DebugAssertException.

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

Line 202 in 25e57e4

Debug.Assert(Tasks.Count(t => t.IsAccepted) == 1, $"There must be exactly one accepted outcome for hedging. Found {Tasks.Count(t => t.IsAccepted)}.");

If we comment out the Debug.Assert then the test passes as expected.

@martintmk, @martincostello Are you guys okay to delete that assertion?