From 57c91f11e6b3de9b3a1a936df2bb5cee66712b42 Mon Sep 17 00:00:00 2001 From: Joshua Harms Date: Thu, 16 May 2024 13:56:28 -0500 Subject: [PATCH] feature: Add support for retrying first failure immediately in ExponentialRetryPolicy with `ExponentialRetryPolicyOptions.FirstRetryIsImmediate` --- .../ExponentialRetryPolicyTests.cs | 38 +++++++++++++++++-- .../ExponentialRetryPolicy.cs | 31 +++++++-------- .../ExponentialRetryPolicyOptions.cs | 25 +++++++++++- 3 files changed, 74 insertions(+), 20 deletions(-) diff --git a/ShopifySharp.Tests/Infrastructure/Policies/ExponentialRetry/ExponentialRetryPolicyTests.cs b/ShopifySharp.Tests/Infrastructure/Policies/ExponentialRetry/ExponentialRetryPolicyTests.cs index e89045d5..e06fbdd9 100644 --- a/ShopifySharp.Tests/Infrastructure/Policies/ExponentialRetry/ExponentialRetryPolicyTests.cs +++ b/ShopifySharp.Tests/Infrastructure/Policies/ExponentialRetry/ExponentialRetryPolicyTests.cs @@ -161,6 +161,36 @@ public async Task Run_ShouldRetryWhenRequestIsRetriableAsync() }); } + [Fact] + public async Task Run_ShouldRetryImmediatelyWhenConfigured() + { + const bool firstRetryIsImmediate = true; + var ex = new TestShopifyException(); + var iteration = 0; + + _executeRequest.When(x => x.Invoke(_cloneableRequestMessage)) + .Do(_ => + { + if (iteration >= 3) throw new TestException(); + iteration++; + throw ex; + }); + _responseClassifier.IsRetriableException(ex, Arg.Any()) + .Returns(true); + + var policy = SetupPolicy(x => x.FirstRetryIsImmediate = firstRetryIsImmediate); + var act = () => policy.Run(_cloneableRequestMessage, _executeRequest, CancellationToken.None); + + await act.Should().ThrowAsync(); + iteration.Should().Be(3); + Received.InOrder(() => + { + _taskScheduler.DelayAsync(TimeSpan.Zero, Arg.Any()); + _taskScheduler.DelayAsync(TimeSpan.FromMilliseconds(100), Arg.Any()); + _taskScheduler.DelayAsync(TimeSpan.FromMilliseconds(200), Arg.Any()); + }); + } + [Fact(Timeout = 1000)] public async Task Run_ShouldHandleNullMaxRetries() { @@ -172,9 +202,9 @@ public async Task Run_ShouldHandleNullMaxRetries() .Do(_ => { iteration++; - // Cancel after 20 loops - if (iteration == expectedIterations) - throw new TestException(); + // Cancel after 20 loops + if (iteration == expectedIterations) + throw new TestException(); throw ex; }); @@ -297,7 +327,6 @@ public async Task Run_ShouldRetryUntilMaximumDelayIsReachedThenThrow() _executeRequest.When(x => x.Invoke(_cloneableRequestMessage)) .Throw(ex); - _responseClassifier.IsRetriableException(ex, Arg.Any()) .Returns(true); @@ -312,6 +341,7 @@ public async Task Run_ShouldRetryUntilMaximumDelayIsReachedThenThrow() // For now, we expect this test to execute the request, check the exception and wait. This // is because the cancellation token source puts the cancellation on a different thread when // cancellation is requested. + // TODO: This seems to be a little bit flaky due to the comment above, sometimes the test receives a second _executeRequest before cancelling Received.InOrder(() => { _executeRequest.Invoke(_cloneableRequestMessage); diff --git a/ShopifySharp/Infrastructure/Policies/ExponentialRetry/ExponentialRetryPolicy.cs b/ShopifySharp/Infrastructure/Policies/ExponentialRetry/ExponentialRetryPolicy.cs index 2a9cad4a..d25ae270 100644 --- a/ShopifySharp/Infrastructure/Policies/ExponentialRetry/ExponentialRetryPolicy.cs +++ b/ShopifySharp/Infrastructure/Policies/ExponentialRetry/ExponentialRetryPolicy.cs @@ -72,22 +72,10 @@ public async Task> Run( // We can quickly hit an overflow by using exponential math to calculate a delay and pass it to the timespan constructor. // To avoid that, we check to see if one of the previous loops' delays passed the maximum delay between retries. If so, // we use the maximum delay rather than calculating another one and potentially hitting that overflow. - TimeSpan nextDelay; + var nextDelay = useMaximumDelayBetweenRetries ? _options.MaximumDelayBetweenRetries : CalculateNextDelay(currentTry); - if (useMaximumDelayBetweenRetries) - { - nextDelay = _options.MaximumDelayBetweenRetries; - } - else - { - nextDelay = TimeSpan.FromMilliseconds(Math.Pow(2, currentTry - 1) * _options.InitialBackoffInMilliseconds); - - if (nextDelay > _options.MaximumDelayBetweenRetries) - { - useMaximumDelayBetweenRetries = true; - nextDelay = _options.MaximumDelayBetweenRetries; - } - } + if (nextDelay >= _options.MaximumDelayBetweenRetries) + useMaximumDelayBetweenRetries = true; currentTry++; @@ -95,4 +83,17 @@ public async Task> Run( await _taskScheduler.DelayAsync(nextDelay, combinedCancellationToken.Token); } } + + private TimeSpan CalculateNextDelay(int currentTry) + { + if (currentTry == 1 && _options.FirstRetryIsImmediate) + return TimeSpan.Zero; + + var exponent = currentTry - (_options.FirstRetryIsImmediate ? 2 : 1); + var delay = Math.Pow(2, Math.Max(0, exponent)) * _options.InitialBackoffInMilliseconds; + var calculatedDelay = TimeSpan.FromMilliseconds(delay); + + return calculatedDelay > _options.MaximumDelayBetweenRetries ? _options.MaximumDelayBetweenRetries : calculatedDelay; + } + } diff --git a/ShopifySharp/Infrastructure/Policies/ExponentialRetry/ExponentialRetryPolicyOptions.cs b/ShopifySharp/Infrastructure/Policies/ExponentialRetry/ExponentialRetryPolicyOptions.cs index 9e71d419..798ea6cf 100644 --- a/ShopifySharp/Infrastructure/Policies/ExponentialRetry/ExponentialRetryPolicyOptions.cs +++ b/ShopifySharp/Infrastructure/Policies/ExponentialRetry/ExponentialRetryPolicyOptions.cs @@ -10,23 +10,45 @@ namespace ShopifySharp.Infrastructure.Policies.ExponentialRetry; /// public record ExponentialRetryPolicyOptions { + /// + /// Indicates whether the policy should immediately retry the first failure per request before applying the + /// exponential backoff strategy. + /// + /// + /// true if the first retry should be immediate; otherwise, false. + /// + /// + /// Setting this property to true can be useful in scenarios where transient failures are likely and + /// immediate retry might resolve the failure. If set to false, all retries including the first one + /// will follow the exponential backoff intervals. + /// + public bool FirstRetryIsImmediate { get; set; } + #if NET8_0_OR_GREATER public required int InitialBackoffInMilliseconds { get; set; } + + /// /// The maximum amount of time that can be spent waiting before retrying a request. This is an effective cap on the /// exponential growth of the policy's retry delay, which could eventually lead to an overflow without it. + /// public required TimeSpan MaximumDelayBetweenRetries { get; set; } #else public int InitialBackoffInMilliseconds { get; set; } + /// /// The maximum amount of time that can be spent waiting before retrying a request. This is an effective cap on the /// exponential growth of the policy's retry delay, which could eventually lead to an overflow without it. + /// public TimeSpan MaximumDelayBetweenRetries { get; set; } #endif public int? MaximumRetriesBeforeRequestCancellation { get; set; } public TimeSpan? MaximumDelayBeforeRequestCancellation { get; set; } /// - /// Validates this instance and throws an if misconfigured. + /// Validates this instance and throws if misconfigured. /// + /// + /// when the options are misconfigured. + /// public void Validate() { if (InitialBackoffInMilliseconds <= 0) @@ -42,6 +64,7 @@ public void Validate() public static ExponentialRetryPolicyOptions Default() => new() { + FirstRetryIsImmediate = false, MaximumRetriesBeforeRequestCancellation = 10, MaximumDelayBetweenRetries = TimeSpan.FromSeconds(1), MaximumDelayBeforeRequestCancellation = TimeSpan.FromSeconds(5),