Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Bug] Fix chaos outcome exception handling #2101

Merged
merged 8 commits into from
May 10, 2024
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 12 additions & 9 deletions src/Polly.Core/Simmy/Outcomes/ChaosOutcomeStrategy.cs
Original file line number Diff line number Diff line change
Expand Up @@ -5,32 +5,35 @@ namespace Polly.Simmy.Outcomes;
internal class ChaosOutcomeStrategy<T> : ChaosStrategy<T>
{
private readonly ResilienceStrategyTelemetry _telemetry;
private readonly Func<OnOutcomeInjectedArguments<T>, ValueTask>? _onOutcomeInjected;
private readonly Func<OutcomeGeneratorArguments, ValueTask<Outcome<T>?>> _outcomeGenerator;

public ChaosOutcomeStrategy(ChaosOutcomeStrategyOptions<T> options, ResilienceStrategyTelemetry telemetry)
: base(options)
{
_telemetry = telemetry;
OnOutcomeInjected = options.OnOutcomeInjected;
OutcomeGenerator = options.OutcomeGenerator;
_onOutcomeInjected = options.OnOutcomeInjected;
_outcomeGenerator = options.OutcomeGenerator;
}

public Func<OnOutcomeInjectedArguments<T>, ValueTask>? OnOutcomeInjected { get; }

public Func<OutcomeGeneratorArguments, ValueTask<Outcome<T>?>> OutcomeGenerator { get; }

protected internal override async ValueTask<Outcome<T>> ExecuteCore<TState>(Func<ResilienceContext, TState, ValueTask<Outcome<T>>> callback, ResilienceContext context, TState state)
{
try
{
if (await ShouldInjectAsync(context).ConfigureAwait(context.ContinueOnCapturedContext))
peter-csala marked this conversation as resolved.
Show resolved Hide resolved
{
var outcome = await OutcomeGenerator(new(context)).ConfigureAwait(context.ContinueOnCapturedContext);
var outcome = await _outcomeGenerator(new(context)).ConfigureAwait(context.ContinueOnCapturedContext);
var args = new OnOutcomeInjectedArguments<T>(context, outcome.Value);
_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.Value.Exception is not null)
martincostello marked this conversation as resolved.
Show resolved Hide resolved
{
await OnOutcomeInjected(args).ConfigureAwait(context.ContinueOnCapturedContext);
return new Outcome<T>(outcome.Value.Exception);
}

return new Outcome<T>(outcome.Value.Result);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,26 +12,27 @@ public class ChaosOutcomePipelineBuilderExtensionsTests
{
builder =>
{
builder.AddChaosOutcome(new ChaosOutcomeStrategyOptions<int>
var options = new ChaosOutcomeStrategyOptions<int>
{
InjectionRate = 0.6,
Randomizer = () => 0.5,
OutcomeGenerator = (_) => new ValueTask<Outcome<int>?>(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<T>(ResiliencePipelineBuilder<T> builder, bool enabled, double injectionRate, Outcome<T> outcome)
private static void AssertResultStrategy<T>(ResiliencePipelineBuilder<T> builder, ChaosOutcomeStrategyOptions<T> options, bool enabled, double injectionRate, Outcome<T> outcome)
{
var context = ResilienceContextPool.Shared.Get();
var strategy = builder.Build().GetPipelineDescriptor().FirstStrategy.StrategyInstance.Should().BeOfType<ChaosOutcomeStrategy<T>>().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))]
Expand All @@ -46,11 +47,14 @@ internal void AddResult_Options_Ok(Action<ResiliencePipelineBuilder<int>> config
public void AddResult_Shortcut_Generator_Option_Ok()
{
var builder = new ResiliencePipelineBuilder<int>();
builder
var pipeline = builder
.AddChaosOutcome(0.5, () => 120)
.Build();

AssertResultStrategy(builder, true, 0.5, new(120));
var descriptor = pipeline.GetPipelineDescriptor();
var options = Assert.IsType<ChaosOutcomeStrategyOptions<int>>(descriptor.Strategies[0].Options);

AssertResultStrategy(builder, options, true, 0.5, new(120));
}

[Fact]
Expand Down
33 changes: 33 additions & 0 deletions test/Polly.Core.Tests/Simmy/Outcomes/ChaosOutcomeStrategyTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -155,6 +155,39 @@ 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_inject_exception()
{
var exception = new InvalidOperationException();
var options = new ChaosOutcomeStrategyOptions<int>
{
InjectionRate = 0.6,
Randomizer = () => 0.5,
OutcomeGenerator = (_) => new ValueTask<Outcome<int>?>(
Outcome.FromException<int>(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<int>(async _ =>
{
_userDelegateExecuted = true;
return await Task.FromResult(42);
}, CancellationToken.None)
.AsTask())
.Should()
.ThrowAsync<InvalidOperationException>();

_userDelegateExecuted.Should().BeFalse();
_onOutcomeInjectedExecuted.Should().BeTrue();
}

[Fact]
public async Task Should_not_execute_user_delegate_when_it_was_cancelled_running_the_strategy()
{
Expand Down
Loading