diff --git a/src/Polly.Core/Simmy/Outcomes/ChaosOutcomeStrategy.cs b/src/Polly.Core/Simmy/Outcomes/ChaosOutcomeStrategy.cs index 1a6c075ff0..0f9a2e44d0 100644 --- a/src/Polly.Core/Simmy/Outcomes/ChaosOutcomeStrategy.cs +++ b/src/Polly.Core/Simmy/Outcomes/ChaosOutcomeStrategy.cs @@ -5,35 +5,38 @@ namespace Polly.Simmy.Outcomes; internal class ChaosOutcomeStrategy : ChaosStrategy { private readonly ResilienceStrategyTelemetry _telemetry; + private readonly Func, ValueTask>? _onOutcomeInjected; + private readonly Func?>> _outcomeGenerator; public ChaosOutcomeStrategy(ChaosOutcomeStrategyOptions options, ResilienceStrategyTelemetry telemetry) : base(options) { _telemetry = telemetry; - OnOutcomeInjected = options.OnOutcomeInjected; - OutcomeGenerator = options.OutcomeGenerator; + _onOutcomeInjected = options.OnOutcomeInjected; + _outcomeGenerator = options.OutcomeGenerator; } - public Func, ValueTask>? OnOutcomeInjected { get; } - - public Func?>> OutcomeGenerator { get; } - protected internal override async ValueTask> ExecuteCore(Func>> callback, ResilienceContext context, TState state) { try { - if (await ShouldInjectAsync(context).ConfigureAwait(context.ContinueOnCapturedContext)) + if (await ShouldInjectAsync(context).ConfigureAwait(context.ContinueOnCapturedContext) + && await _outcomeGenerator(new(context)).ConfigureAwait(context.ContinueOnCapturedContext) is Outcome outcome) { - var outcome = await OutcomeGenerator(new(context)).ConfigureAwait(context.ContinueOnCapturedContext); - var args = new OnOutcomeInjectedArguments(context, outcome.Value); + var args = new OnOutcomeInjectedArguments(context, outcome); _telemetry.Report(new(ResilienceEventSeverity.Information, ChaosOutcomeConstants.OnOutcomeInjectedEvent), context, args); - if (OnOutcomeInjected is not null) + if (_onOutcomeInjected is not null) + { + await _onOutcomeInjected(args).ConfigureAwait(context.ContinueOnCapturedContext); + } + + if (outcome.HasResult) { - await OnOutcomeInjected(args).ConfigureAwait(context.ContinueOnCapturedContext); + return new Outcome(outcome.Result); } - return new Outcome(outcome.Value.Result); + return new Outcome(outcome.Exception!); } return await StrategyHelper.ExecuteCallbackSafeAsync(callback, context, state).ConfigureAwait(context.ContinueOnCapturedContext); diff --git a/test/Polly.Core.Tests/Simmy/Outcomes/ChaosOutcomePipelineBuilderExtensionsTests.cs b/test/Polly.Core.Tests/Simmy/Outcomes/ChaosOutcomePipelineBuilderExtensionsTests.cs index 0bcc4f1120..192948c550 100644 --- a/test/Polly.Core.Tests/Simmy/Outcomes/ChaosOutcomePipelineBuilderExtensionsTests.cs +++ b/test/Polly.Core.Tests/Simmy/Outcomes/ChaosOutcomePipelineBuilderExtensionsTests.cs @@ -12,26 +12,27 @@ public class ChaosOutcomePipelineBuilderExtensionsTests { builder => { - builder.AddChaosOutcome(new ChaosOutcomeStrategyOptions + var options = new ChaosOutcomeStrategyOptions { InjectionRate = 0.6, Randomizer = () => 0.5, OutcomeGenerator = (_) => new ValueTask?>(Outcome.FromResult(100)) - }); + }; + builder.AddChaosOutcome(options); - AssertResultStrategy(builder, true, 0.6, new(100)); + AssertResultStrategy(builder, options, true, 0.6, new(100)); }, }; #pragma warning restore IDE0028 - private static void AssertResultStrategy(ResiliencePipelineBuilder builder, bool enabled, double injectionRate, Outcome outcome) + private static void AssertResultStrategy(ResiliencePipelineBuilder builder, ChaosOutcomeStrategyOptions options, bool enabled, double injectionRate, Outcome outcome) { var context = ResilienceContextPool.Shared.Get(); var strategy = builder.Build().GetPipelineDescriptor().FirstStrategy.StrategyInstance.Should().BeOfType>().Subject; strategy.EnabledGenerator.Invoke(new(context)).Preserve().GetAwaiter().GetResult().Should().Be(enabled); strategy.InjectionRateGenerator.Invoke(new(context)).Preserve().GetAwaiter().GetResult().Should().Be(injectionRate); - strategy.OutcomeGenerator!.Invoke(new(context)).Preserve().GetAwaiter().GetResult().Should().Be(outcome); + options.OutcomeGenerator!.Invoke(new(context)).Preserve().GetAwaiter().GetResult().Should().Be(outcome); } [MemberData(nameof(ResultStrategy))] @@ -46,11 +47,14 @@ internal void AddResult_Options_Ok(Action> config public void AddResult_Shortcut_Generator_Option_Ok() { var builder = new ResiliencePipelineBuilder(); - builder + var pipeline = builder .AddChaosOutcome(0.5, () => 120) .Build(); - AssertResultStrategy(builder, true, 0.5, new(120)); + var descriptor = pipeline.GetPipelineDescriptor(); + var options = Assert.IsType>(descriptor.Strategies[0].Options); + + AssertResultStrategy(builder, options, true, 0.5, new(120)); } [Fact] diff --git a/test/Polly.Core.Tests/Simmy/Outcomes/ChaosOutcomeStrategyTests.cs b/test/Polly.Core.Tests/Simmy/Outcomes/ChaosOutcomeStrategyTests.cs index b5359a4161..e3aa310171 100644 --- a/test/Polly.Core.Tests/Simmy/Outcomes/ChaosOutcomeStrategyTests.cs +++ b/test/Polly.Core.Tests/Simmy/Outcomes/ChaosOutcomeStrategyTests.cs @@ -61,7 +61,7 @@ public void Given_not_enabled_should_not_inject_result() InjectionRate = 0.6, Enabled = false, Randomizer = () => 0.5, - OutcomeGenerator = (_) => new ValueTask?>(Outcome.FromResult(fakeResult)) + OutcomeGenerator = _ => new ValueTask?>(Outcome.FromResult(fakeResult)) }; var sut = CreateSut(options); @@ -81,7 +81,7 @@ public async Task Given_enabled_and_randomly_within_threshold_should_inject_resu { InjectionRate = 0.6, Randomizer = () => 0.5, - OutcomeGenerator = (_) => new ValueTask?>(Outcome.FromResult(fakeResult)), + OutcomeGenerator = _ => new ValueTask?>(Outcome.FromResult(fakeResult)), OnOutcomeInjected = args => { args.Context.Should().NotBeNull(); @@ -92,10 +92,10 @@ public async Task Given_enabled_and_randomly_within_threshold_should_inject_resu }; var sut = CreateSut(options); - var response = await sut.ExecuteAsync(async _ => + var response = await sut.ExecuteAsync(_ => { _userDelegateExecuted = true; - return await Task.FromResult(HttpStatusCode.OK); + return new ValueTask(HttpStatusCode.OK); }); response.Should().Be(fakeResult); @@ -117,7 +117,7 @@ public void Given_enabled_and_randomly_not_within_threshold_should_not_inject_re InjectionRate = 0.3, Enabled = false, Randomizer = () => 0.5, - OutcomeGenerator = (_) => new ValueTask?>(Outcome.FromResult(fakeResult)) + OutcomeGenerator = _ => new ValueTask?>(Outcome.FromResult(fakeResult)) }; var sut = CreateSut(options); @@ -133,21 +133,21 @@ public void Given_enabled_and_randomly_not_within_threshold_should_not_inject_re } [Fact] - public async Task Given_enabled_and_randomly_within_threshold_should_inject_result_even_as_null() + public async Task Given_enabled_and_randomly_within_threshold_should_inject_result_if_it_is_null() { - Outcome? nullOutcome = Outcome.FromResult(null); + Outcome? outcomeWithNullResult = Outcome.FromResult(null); var options = new ChaosOutcomeStrategyOptions { InjectionRate = 0.6, Randomizer = () => 0.5, - OutcomeGenerator = (_) => new ValueTask?>(nullOutcome) + OutcomeGenerator = _ => new ValueTask?>(outcomeWithNullResult) }; var sut = CreateSut(options); - var response = await sut.ExecuteAsync(async _ => + var response = await sut.ExecuteAsync(_ => { _userDelegateExecuted = true; - return await Task.FromResult(HttpStatusCode.OK); + return new ValueTask(HttpStatusCode.OK); }); response.Should().Be(null); @@ -155,6 +155,66 @@ public async Task Given_enabled_and_randomly_within_threshold_should_inject_resu _onOutcomeInjectedExecuted.Should().BeFalse(); } + [Fact] + public async Task Given_enabled_and_randomly_within_threshold_should_not_inject_if_generator_returns_null() + { + Outcome? nullOutcome = null; + var options = new ChaosOutcomeStrategyOptions + { + InjectionRate = 0.6, + Randomizer = () => 0.5, + OutcomeGenerator = _ => new ValueTask?>(nullOutcome), + OnOutcomeInjected = args => + { + _onOutcomeInjectedExecuted = true; + return default; + } + }; + + var sut = CreateSut(options); + var response = await sut.ExecuteAsync(_ => + { + _userDelegateExecuted = true; + return new ValueTask(42); + }); + + response.Should().Be(42); + _userDelegateExecuted.Should().BeTrue(); + _onOutcomeInjectedExecuted.Should().BeFalse(); + } + + [Fact] + public async Task Given_enabled_and_randomly_within_threshold_should_inject_exception() + { + var exception = new InvalidOperationException(); + var options = new ChaosOutcomeStrategyOptions + { + InjectionRate = 0.6, + Randomizer = () => 0.5, + OutcomeGenerator = _ => new ValueTask?>(Outcome.FromException(exception)), + OnOutcomeInjected = args => + { + args.Outcome.Result.Should().Be(default); + args.Outcome.Exception.Should().Be(exception); + _onOutcomeInjectedExecuted = true; + return default; + } + }; + + var sut = CreateSut(options); + await sut.Invoking(s => s.ExecuteAsync(_ => + { + _userDelegateExecuted = true; + return new ValueTask(42); + }, CancellationToken.None) + .AsTask()) + .Should() + .ThrowAsync(); + + _userDelegateExecuted.Should().BeFalse(); + _onOutcomeInjectedExecuted.Should().BeTrue(); + } + [Fact] public async Task Should_not_execute_user_delegate_when_it_was_cancelled_running_the_strategy() { @@ -163,19 +223,19 @@ public async Task Should_not_execute_user_delegate_when_it_was_cancelled_running { InjectionRate = 0.6, Randomizer = () => 0.5, - EnabledGenerator = (_) => + EnabledGenerator = _ => { cts.Cancel(); return new ValueTask(true); }, - OutcomeGenerator = (_) => new ValueTask?>(Outcome.FromResult(HttpStatusCode.TooManyRequests)) + OutcomeGenerator = _ => new ValueTask?>(Outcome.FromResult(HttpStatusCode.TooManyRequests)) }; var sut = CreateSut(options); - await sut.Invoking(s => s.ExecuteAsync(async _ => + await sut.Invoking(s => s.ExecuteAsync(_ => { _userDelegateExecuted = true; - return await Task.FromResult(HttpStatusCode.OK); + return new ValueTask(HttpStatusCode.OK); }, cts.Token) .AsTask()) .Should()