Skip to content

Commit

Permalink
[Bug] Fix chaos outcome exception handling (#2101)
Browse files Browse the repository at this point in the history
  • Loading branch information
@peter-csala
peter-csala authored May 10, 2024
1 parent 98e9a44 commit 41d27ff
Show file tree
Hide file tree
Showing 3 changed files with 100 additions and 33 deletions.
27 changes: 15 additions & 12 deletions src/Polly.Core/Simmy/Outcomes/ChaosOutcomeStrategy.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,35 +5,38 @@ 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))
if (await ShouldInjectAsync(context).ConfigureAwait(context.ContinueOnCapturedContext)
&& await _outcomeGenerator(new(context)).ConfigureAwait(context.ContinueOnCapturedContext) is Outcome<T> outcome)
{
var outcome = await OutcomeGenerator(new(context)).ConfigureAwait(context.ContinueOnCapturedContext);
var args = new OnOutcomeInjectedArguments<T>(context, outcome.Value);
var args = new OnOutcomeInjectedArguments<T>(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<T>(outcome.Result);
}

return new Outcome<T>(outcome.Value.Result);
return new Outcome<T>(outcome.Exception!);
}

return await StrategyHelper.ExecuteCallbackSafeAsync(callback, context, state).ConfigureAwait(context.ContinueOnCapturedContext);
Expand Down
18 changes: 11 additions & 7 deletions test/Polly.Core.Tests/Simmy/Outcomes/ChaosOutcomePipelineBuilderExtensionsTests.cs
View file
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
88 changes: 74 additions & 14 deletions test/Polly.Core.Tests/Simmy/Outcomes/ChaosOutcomeStrategyTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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<HttpStatusCode>?>(Outcome.FromResult(fakeResult))
OutcomeGenerator = _ => new ValueTask<Outcome<HttpStatusCode>?>(Outcome.FromResult(fakeResult))
};

var sut = CreateSut(options);
Expand All @@ -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<HttpStatusCode>?>(Outcome.FromResult(fakeResult)),
OutcomeGenerator = _ => new ValueTask<Outcome<HttpStatusCode>?>(Outcome.FromResult(fakeResult)),
OnOutcomeInjected = args =>
{
args.Context.Should().NotBeNull();
Expand All @@ -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>(HttpStatusCode.OK);
});

response.Should().Be(fakeResult);
Expand All @@ -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<HttpStatusCode>?>(Outcome.FromResult(fakeResult))
OutcomeGenerator = _ => new ValueTask<Outcome<HttpStatusCode>?>(Outcome.FromResult(fakeResult))
};

var sut = CreateSut(options);
Expand All @@ -133,28 +133,88 @@ 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<HttpStatusCode?>? nullOutcome = Outcome.FromResult<HttpStatusCode?>(null);
Outcome<HttpStatusCode?>? outcomeWithNullResult = Outcome.FromResult<HttpStatusCode?>(null);
var options = new ChaosOutcomeStrategyOptions<HttpStatusCode?>
{
InjectionRate = 0.6,
Randomizer = () => 0.5,
OutcomeGenerator = (_) => new ValueTask<Outcome<HttpStatusCode?>?>(nullOutcome)
OutcomeGenerator = _ => new ValueTask<Outcome<HttpStatusCode?>?>(outcomeWithNullResult)
};

var sut = CreateSut(options);
var response = await sut.ExecuteAsync<HttpStatusCode?>(async _ =>
var response = await sut.ExecuteAsync<HttpStatusCode?>(_ =>
{
_userDelegateExecuted = true;
return await Task.FromResult(HttpStatusCode.OK);
return new ValueTask<HttpStatusCode?>(HttpStatusCode.OK);
});

response.Should().Be(null);
_userDelegateExecuted.Should().BeFalse();
_onOutcomeInjectedExecuted.Should().BeFalse();
}

[Fact]
public async Task Given_enabled_and_randomly_within_threshold_should_not_inject_if_generator_returns_null()
{
Outcome<int>? nullOutcome = null;
var options = new ChaosOutcomeStrategyOptions<int>
{
InjectionRate = 0.6,
Randomizer = () => 0.5,
OutcomeGenerator = _ => new ValueTask<Outcome<int>?>(nullOutcome),
OnOutcomeInjected = args =>
{
_onOutcomeInjectedExecuted = true;
return default;
}
};

var sut = CreateSut(options);
var response = await sut.ExecuteAsync(_ =>
{
_userDelegateExecuted = true;
return new ValueTask<int>(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<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(_ =>
{
_userDelegateExecuted = true;
return new ValueTask<int>(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 All @@ -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<bool>(true);
},
OutcomeGenerator = (_) => new ValueTask<Outcome<HttpStatusCode>?>(Outcome.FromResult(HttpStatusCode.TooManyRequests))
OutcomeGenerator = _ => new ValueTask<Outcome<HttpStatusCode>?>(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>(HttpStatusCode.OK);
}, cts.Token)
.AsTask())
.Should()
Expand Down

0 comments on commit 41d27ff

Please sign in to comment.