Make the primary properties of the policies verifyable

[peter-csala]

Please make it possible to verify policies' main properties.

Let's suppose I want to create a combined policy where I want to put together a Retry, Circuit Breaker and Timeout policies to surround my HttpClients.

The configuration class:

public class HttpResilienceSettings
{
    public int TimeoutInMilliseconds { get; set; }
    public int RetrySleepInMilliseconds { get; set; }
    public int RetryCount { get; set; }
    public int CircuitBreakerFailCount { get; set; }
    public int CircuitBreakerDelayInMilliseconds { get; set; }
}

The initialization of the policy registry:

public class HttpResiliencePolicies
{
    public const string RequestRetryPolicy = "RetryPolicy";
    public const string RequestTimeoutPolicy = "TimeoutPolicy";
    public const string RequestCircuitBreakerPolicy = "CircuitBreakerPolicy";
    public const string CombinedPolicy = "Retry-CircuitBreaker-Timeout";

    public PolicyRegistry InitializePolicies(HttpResilienceSettings settings)
        => new PolicyRegistry
        {
            { RequestTimeoutPolicy, TimeoutPolicy(settings) },
            { RequestCircuitBreakerPolicy, CircuitBreakerPolicy(settings) },
            { RequestRetryPolicy, RetryPolicy(settings) },
            { CombinedPolicy, Policy.WrapAsync(RetryPolicy(settings), CircuitBreakerPolicy(settings), TimeoutPolicy(settings))}
        };

    private IAsyncPolicy<HttpResponseMessage> TimeoutPolicy(HttpResilienceSettings settings)
        => Policy
            .TimeoutAsync<HttpResponseMessage>(
                timeout: TimeSpan.FromMilliseconds(settings.TimeoutInMilliseconds));

    private IAsyncPolicy<HttpResponseMessage> CircuitBreakerPolicy(HttpResilienceSettings settings)
        => HttpPolicyExtensions
            .HandleTransientHttpError() 
            .Or<TimeoutRejectedException>()
            .CircuitBreakerAsync( 
                handledEventsAllowedBeforeBreaking: settings.CircuitBreakerFailCount, 
                durationOfBreak: TimeSpan.FromMilliseconds(settings.CircuitBreakerDelayInMilliseconds)); 

    private IAsyncPolicy<HttpResponseMessage> RetryPolicy(HttpResilienceSettings settings)
        => Policy
            .Handle<BrokenCircuitException>() 
            .Or<TimeoutRejectedException>()
            .Or<HttpRequestException>()
            .WaitAndRetryAsync( 
                retryCount: settings.RetryCount,
                sleepDurationProvider: _ => TimeSpan.FromMilliseconds(settings.RetrySleepInMilliseconds))
            .AsAsyncPolicy<HttpResponseMessage>();
}

I want to be able to catch these sort of issues:

• sleepDurationProvider: _ => TimeSpan.FromSeconds(settings.RetrySleepInMilliseconds)
• sleepDurationProvider: _ => TimeSpan.FromMilliseconds(settings.RetrySleepInMilliseconds) + 2000
• sleepDurationProvider: _ => TimeSpan.FromSeconds(settings.TimeoutInMilliseconds)

Current solution:
If I want to test for example the Timeout policy's Timeout then I can do something like this:

[Fact]
public async Task HttpResiliencePolicies_TimeoutPolicy_Set_Timeout_Property()
{
    //Arrange
    const int ExpectedTimeoutMS = 1234;
    var httpPolicies = new HttpResiliencePolicies();
    PolicyRegistry registry = null;
    HttpResilienceSettings settings = GenerateSettings(timeoutMS: ExpectedTimeoutMS);

    //REF: https://github.com/App-vNext/Polly/blob/ffe99a34edd96d5c5c94075fe22aae8e1a7ed3ed/src/Polly/Timeout/AsyncTimeoutPolicy.cs#L53
    const string timeoutProviderFieldName = "_timeoutProvider";

    //Act
    registry = httpPolicies.InitializePolicies(settings);

    //Assert
    var timeoutPolicy = registry[HttpResiliencePolicies.RequestTimeoutPolicy] as AsyncTimeoutPolicy<HttpResponseMessage>;
    Func<Context, TimeSpan> timeoutProvider = GetPrivateFieldValue <AsyncTimeoutPolicy<HttpResponseMessage>, Func<Context, TimeSpan>>
        (timeoutProviderFieldName, timeoutPolicy);

    //Pretend that the operation succeeded under the given timeframe / constraint
    var policyExecutionResult = await timeoutPolicy.ExecuteAndCaptureAsync(
        () => Task.FromResult(new HttpResponseMessage()));

    var actualTimeSpan = timeoutProvider.Invoke(policyExecutionResult.Context);
    Assert.Equal(ExpectedTimeoutMS, actualTimeSpan.TotalMilliseconds);
}

Helpers:

private HttpResilienceSettings GenerateSettings(
    int timeoutMS = 1,
    int sleepDurationMS = 1, int retryCount = 1,  //retry
    int failureCount = 1, int delayDurationMS = 1) //circuit breaker
    => new HttpResilienceSettings
    {
        TimeoutInMilliseconds = timeoutMS,
        RetrySleepInMilliseconds = sleepDurationMS,
        RetryCount = retryCount,
        CircuitBreakerFailCount = failureCount,
        CircuitBreakerDelayInMilliseconds = delayDurationMS,
    };

private TFieldValue GetPrivateFieldValue<TPolicy, TFieldValue>(string fieldName, TPolicy actualInstance)
    where TPolicy : IAsyncPolicy<HttpResponseMessage>
    where TFieldValue : class
    => actualInstance.GetType()
        .GetField(fieldName, BindingFlags.NonPublic | BindingFlags.Instance)
        .GetValue(actualInstance) as TFieldValue;

As you can see I highly rely on the actual implementation in order to be able to verify it.
To make thing worst, I can do this sort of sorcery only for the following properties:

• Timeout's Timeout: _timeoutProvider
• Retry's RetryCount: _permittedRetryCount
• Retry's SleepDuration: _sleepDurationsEnumerable

So I can't make sure that the circuit break policy was set up correctly.

Proposals:

1. Expose these fields as readonly properties to able to query them.
2. Extend the interfaces (ITimeoutPolicy, IRetryPolicy, etc.) on which these properties are accessible
3. Create a separate lib which is exposing only Verification helpers

[peter-csala]

Can you please provide some feedback?

[reisenberger]

I appreciate the patience you have shown. COVID-19 has meant some refocus of my personal priorities away from OSS at this time.

The underlying point is a good one. The configuration process for Polly is expected to change for the next major version (which would affect the specifics you proposed). We will consider whether there are any ways to address the goal in that version.

[peter-csala]

Health is the number one priority nowadays, so no worries. :)

I could not found anything about v8's new configuration process under the v8 pull request. Where can I find any information about that?

[reisenberger]

This in the roadmap is an early sketch for the possible v8 syntax. This is likely to lead to a configuration class encapsulating all the configuration properties for a given policy type (eg RetryConfiguration, or RetryConfiguration<TResult>). Assuming that is the final direction, it should be possible to expose that configuration object via the interface type for the policy (eg IRetryPolicy) - similar to points 1 and 2 in your proposal.

[RichieRunner]

I'd also like to see this; for the purpose of unit testing that my initialization of Polly was done correctly and the right configurations for my policy was set.

[peter-csala]

Hi @reisenberger, is there any progress regarding this feature suggestion?

[PeterCsalaHbo]

@reisenberger is there any news regarding issue?

[martincostello]

There is no progress regarding this issue.

[PeterCsalaHbo]

@martincostello Shall I close it? Or does it make any sense to wait even further?

[martincostello]

Looking at the comments from Dylan above, it sounds like this will be addressed in v8. However, v8 development is currently on hiatus so I can't say when it would specifically be addressed.

[udlose]

Any word on v8 progress?

[martintmk]

Any word on v8 progress?

V8 alpha is released where the validation is addressed.

Check the new package:
https://www.nuget.org/packages/Polly.Core/

And samples:
https://github.com/App-vNext/Polly/tree/main/samples

cc @martincostello