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

Do not encourage re-throwing the same exception instance #1900

Merged
merged 1 commit into from
Jan 17, 2024
Merged
Show file tree
Hide file tree
Changes from all 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
2 changes: 0 additions & 2 deletions src/Polly.Core/PublicAPI.Unshipped.txt
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,6 @@ Polly.Simmy.Fault.FaultGeneratorArguments.Context.get -> Polly.ResilienceContext
Polly.Simmy.Fault.FaultGeneratorArguments.FaultGeneratorArguments() -> void
Polly.Simmy.Fault.FaultGeneratorArguments.FaultGeneratorArguments(Polly.ResilienceContext! context) -> void
Polly.Simmy.Fault.FaultStrategyOptions
Polly.Simmy.Fault.FaultStrategyOptions.Fault.get -> System.Exception?
Polly.Simmy.Fault.FaultStrategyOptions.Fault.set -> void
Polly.Simmy.Fault.FaultStrategyOptions.FaultGenerator.get -> System.Func<Polly.Simmy.Fault.FaultGeneratorArguments, System.Threading.Tasks.ValueTask<System.Exception?>>?
Polly.Simmy.Fault.FaultStrategyOptions.FaultGenerator.set -> void
Polly.Simmy.Fault.FaultStrategyOptions.FaultStrategyOptions() -> void
Expand Down
11 changes: 2 additions & 9 deletions src/Polly.Core/Simmy/Fault/FaultChaosStrategy.cs
Original file line number Diff line number Diff line change
Expand Up @@ -9,23 +9,16 @@ internal class FaultChaosStrategy : MonkeyStrategy
public FaultChaosStrategy(FaultStrategyOptions options, ResilienceStrategyTelemetry telemetry)
: base(options)
{
if (options.Fault is null && options.FaultGenerator is null)
{
throw new InvalidOperationException("Either Fault or FaultGenerator is required.");
}

_telemetry = telemetry;
Fault = options.Fault;

OnFaultInjected = options.OnFaultInjected;
FaultGenerator = options.FaultGenerator is not null ? options.FaultGenerator : (_) => new(options.Fault);
FaultGenerator = options.FaultGenerator!;
}

public Func<OnFaultInjectedArguments, ValueTask>? OnFaultInjected { get; }

public Func<FaultGeneratorArguments, ValueTask<Exception?>> FaultGenerator { get; }

public Exception? Fault { get; }

protected internal override async ValueTask<Outcome<TResult>> ExecuteCore<TResult, TState>(
Func<ResilienceContext, TState, ValueTask<Outcome<TResult>>> callback,
ResilienceContext context,
Expand Down
14 changes: 4 additions & 10 deletions src/Polly.Core/Simmy/Fault/FaultStrategyOptions.cs
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
namespace Polly.Simmy.Fault;
using System.ComponentModel.DataAnnotations;

namespace Polly.Simmy.Fault;

/// <summary>
/// Represents the options for the Fault chaos strategy.
Expand All @@ -20,14 +22,6 @@ public class FaultStrategyOptions : MonkeyStrategyOptions
/// Defaults to <see langword="null"/>. Either <see cref="Fault"/> or this property is required.
/// When this property is <see langword="null"/> the <see cref="Fault"/> is used.
/// </remarks>
[Required]
public Func<FaultGeneratorArguments, ValueTask<Exception?>>? FaultGenerator { get; set; } = default!;

/// <summary>
/// Gets or sets the outcome to be injected for a given execution.
/// </summary>
/// <remarks>
/// Defaults to <see langword="null"/>. Either <see cref="FaultGenerator"/> or this property is required.
/// When this property is <see langword="null"/> the <see cref="FaultGenerator"/> is used.
/// </remarks>
public Exception? Fault { get; set; }
}
Original file line number Diff line number Diff line change
Expand Up @@ -17,11 +17,10 @@ public class FaultChaosPipelineBuilderExtensionsTests
InjectionRate = 0.6,
Enabled = true,
Randomizer = () => 0.5,
Fault = new InvalidOperationException("Dummy exception.")
FaultGenerator = _=> new ValueTask<Exception?>( new InvalidOperationException("Dummy exception."))
});

AssertFaultStrategy<InvalidOperationException>(builder, true, 0.6)
.Fault.Should().BeOfType(typeof(InvalidOperationException));
AssertFaultStrategy<InvalidOperationException>(builder, true, 0.6).FaultGenerator.Should().NotBeNull();
},
};
#pragma warning restore IDE0028
Expand Down
12 changes: 6 additions & 6 deletions test/Polly.Core.Tests/Simmy/Fault/FaultChaosStrategyTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ public void Given_not_enabled_should_not_inject_fault()
InjectionRate = 0.6,
Enabled = false,
Randomizer = () => 0.5,
Fault = fault
FaultGenerator = _ => new ValueTask<Exception?>(fault)
};

var sut = CreateSut(options);
Expand All @@ -79,7 +79,7 @@ public async Task Given_not_enabled_should_not_inject_fault_and_return_outcome()
InjectionRate = 0.6,
Enabled = false,
Randomizer = () => 0.5,
Fault = fault
FaultGenerator = _ => new ValueTask<Exception?>(fault)
};

var sut = new ResiliencePipelineBuilder<HttpStatusCode>().AddChaosFault(options).Build();
Expand All @@ -106,7 +106,7 @@ public async Task Given_enabled_and_randomly_within_threshold_should_inject_faul
InjectionRate = 0.6,
Enabled = true,
Randomizer = () => 0.5,
Fault = fault,
FaultGenerator = _ => new ValueTask<Exception?>(fault),
OnFaultInjected = args =>
{
args.Context.Should().NotBeNull();
Expand Down Expand Up @@ -143,7 +143,7 @@ public async Task Given_enabled_and_randomly_within_threshold_should_inject_faul
InjectionRate = 0.6,
Enabled = true,
Randomizer = () => 0.5,
Fault = fault,
FaultGenerator = _ => new ValueTask<Exception?>(fault),
OnFaultInjected = args =>
{
args.Context.Should().NotBeNull();
Expand Down Expand Up @@ -181,7 +181,7 @@ public void Given_enabled_and_randomly_not_within_threshold_should_not_inject_fa
InjectionRate = 0.3,
Enabled = true,
Randomizer = () => 0.5,
Fault = fault
FaultGenerator = _ => new ValueTask<Exception?>(fault)
};

var sut = CreateSut(options);
Expand Down Expand Up @@ -232,7 +232,7 @@ public async Task Should_not_execute_user_delegate_when_it_was_cancelled_running
return new ValueTask<bool>(true);
},
Randomizer = () => 0.5,
Fault = fault
FaultGenerator = _ => new ValueTask<Exception?>(fault)
};

var sut = CreateSut(options);
Expand Down
24 changes: 22 additions & 2 deletions test/Polly.Core.Tests/Simmy/Fault/FaultStrategyOptionsTests.cs
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
using Polly.Simmy;
using System.ComponentModel.DataAnnotations;
using Polly.Simmy;
using Polly.Simmy.Fault;
using Polly.Utils;

namespace Polly.Core.Tests.Simmy.Fault;

Expand All @@ -14,8 +16,26 @@ public void Ctor_Ok()
sut.EnabledGenerator.Should().BeNull();
sut.InjectionRate.Should().Be(MonkeyStrategyConstants.DefaultInjectionRate);
sut.InjectionRateGenerator.Should().BeNull();
sut.Fault.Should().BeNull();
sut.OnFaultInjected.Should().BeNull();
sut.FaultGenerator.Should().BeNull();
}

[Fact]
public void InvalidOptions()
{
var options = new FaultStrategyOptions
{
FaultGenerator = null!,
};

options.Invoking(o => ValidationHelper.ValidateObject(new(o, "Invalid Options")))
.Should()
.Throw<ValidationException>()
.WithMessage("""
Invalid Options

Validation Errors:
The FaultGenerator field is required.
""");
}
}
Loading