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.

Sign up for GitHub

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

[Bug] Fix chaos outcome exception handling #2101

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
6693095
Replace public properties to private fields
peter-csala May 9, 2024
e34a9c2
Throw exception if needed
peter-csala May 9, 2024
221da2b
Fix build
peter-csala May 9, 2024
7d11148
Fix build
peter-csala May 9, 2024
8832da8
Handle each outcome case explicitly
peter-csala May 10, 2024
a17a644
Fix sonar rule violation
peter-csala May 10, 2024
1f0a4d1
Apply suggestion
peter-csala May 10, 2024
d689f14
Fix .net481 support
peter-csala May 10, 2024
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
27 changes: 15 additions & 12 deletions src/Polly.Core/Simmy/Outcomes/ChaosOutcomeStrategy.cs
View file
Open in desktop
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
Open in desktop
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
Open in desktop
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
Loading