From 37241838e73d6f38fed7ce1780ee3379883e47af Mon Sep 17 00:00:00 2001
From: Martin Tomka <martintomka@microsoft.com>
Date: Fri, 8 Sep 2023 09:55:49 +0200
Subject: [PATCH] Improve the docs and behavior around infinite retries

---
 README_V8.md                                  |  2 +-
 docs/strategies/retry.md                      |  2 +-
 .../Retry/RetryResilienceStrategy.cs          | 19 ++++++++++++++++---
 .../Retry/RetryStrategyOptions.TResult.cs     |  3 +++
 src/Snippets/Docs/Retry.cs                    |  2 +-
 .../Retry/RetryResilienceStrategyTests.cs     | 10 ++++++++++
 6 files changed, 32 insertions(+), 6 deletions(-)

diff --git a/README_V8.md b/README_V8.md
index 858899bdb3..b0d8274875 100644
--- a/README_V8.md
+++ b/README_V8.md
@@ -193,7 +193,7 @@ new ResiliencePipelineBuilder().AddRetry(new RetryStrategyOptions
     }
 });
 
-// To keep retrying indefinitely until successful
+// To keep retrying indefinitely or until success use int.MaxValue.
 new ResiliencePipelineBuilder().AddRetry(new RetryStrategyOptions
 {
     MaxRetryAttempts = int.MaxValue,
diff --git a/docs/strategies/retry.md b/docs/strategies/retry.md
index 277146287b..6ed0ecd958 100644
--- a/docs/strategies/retry.md
+++ b/docs/strategies/retry.md
@@ -86,7 +86,7 @@ new ResiliencePipelineBuilder().AddRetry(new RetryStrategyOptions
     }
 });
 
-// To keep retrying indefinitely until successful
+// To keep retrying indefinitely or until success use int.MaxValue.
 new ResiliencePipelineBuilder().AddRetry(new RetryStrategyOptions
 {
     MaxRetryAttempts = int.MaxValue,
diff --git a/src/Polly.Core/Retry/RetryResilienceStrategy.cs b/src/Polly.Core/Retry/RetryResilienceStrategy.cs
index 6d61e7bdaf..9eb5f1068c 100644
--- a/src/Polly.Core/Retry/RetryResilienceStrategy.cs
+++ b/src/Polly.Core/Retry/RetryResilienceStrategy.cs
@@ -56,7 +56,7 @@ protected internal override async ValueTask<Outcome<T>> ExecuteCore<TState>(Func
 
             TelemetryUtil.ReportExecutionAttempt(_telemetry, context, outcome, attempt, executionTime, handle);
 
-            if (context.CancellationToken.IsCancellationRequested || IsLastAttempt(attempt) || !handle)
+            if (context.CancellationToken.IsCancellationRequested || IsLastAttempt(attempt, out bool incrementAttempts) || !handle)
             {
                 return outcome;
             }
@@ -98,9 +98,22 @@ protected internal override async ValueTask<Outcome<T>> ExecuteCore<TState>(Func
                 }
             }
 
-            attempt++;
+            if (incrementAttempts)
+            {
+                attempt++;
+            }
         }
     }
 
-    private bool IsLastAttempt(int attempt) => attempt >= RetryCount;
+    internal bool IsLastAttempt(int attempt, out bool incrementAttempts)
+    {
+        if (attempt == int.MaxValue)
+        {
+            incrementAttempts = false;
+            return false;
+        }
+
+        incrementAttempts = true;
+        return attempt >= RetryCount;
+    }
 }
diff --git a/src/Polly.Core/Retry/RetryStrategyOptions.TResult.cs b/src/Polly.Core/Retry/RetryStrategyOptions.TResult.cs
index 797d65c981..fa56848069 100644
--- a/src/Polly.Core/Retry/RetryStrategyOptions.TResult.cs
+++ b/src/Polly.Core/Retry/RetryStrategyOptions.TResult.cs
@@ -20,6 +20,9 @@ public class RetryStrategyOptions<TResult> : ResilienceStrategyOptions
     /// <value>
     /// The default value is 3 retries.
     /// </value>
+    /// <remarks>
+    /// To retry indefinitely use <see cref="int.MaxValue"/>. Note that the reported attempt number is capped at <see cref="int.MaxValue"/>.
+    /// </remarks>
     [Range(1, RetryConstants.MaxRetryCount)]
     public int MaxRetryAttempts { get; set; } = RetryConstants.DefaultRetryCount;
 
diff --git a/src/Snippets/Docs/Retry.cs b/src/Snippets/Docs/Retry.cs
index 3973f8f491..a67653d18b 100644
--- a/src/Snippets/Docs/Retry.cs
+++ b/src/Snippets/Docs/Retry.cs
@@ -81,7 +81,7 @@ public static void Usage()
             }
         });
 
-        // To keep retrying indefinitely until successful
+        // To keep retrying indefinitely or until success use int.MaxValue.
         new ResiliencePipelineBuilder().AddRetry(new RetryStrategyOptions
         {
             MaxRetryAttempts = int.MaxValue,
diff --git a/test/Polly.Core.Tests/Retry/RetryResilienceStrategyTests.cs b/test/Polly.Core.Tests/Retry/RetryResilienceStrategyTests.cs
index 3e209ffa5c..c41a69ca18 100644
--- a/test/Polly.Core.Tests/Retry/RetryResilienceStrategyTests.cs
+++ b/test/Polly.Core.Tests/Retry/RetryResilienceStrategyTests.cs
@@ -2,6 +2,7 @@
 using Microsoft.Extensions.Time.Testing;
 using Polly.Retry;
 using Polly.Telemetry;
+using Polly.Testing;
 
 namespace Polly.Core.Tests.Retry;
 
@@ -190,6 +191,15 @@ public async Task RetryDelayGenerator_ZeroDelay_NoTimeProviderCalls()
         generatedValues.Should().Be(3);
     }
 
+    [Fact]
+    public void IsLastAttempt_Ok()
+    {
+        var sut = (RetryResilienceStrategy<object>)CreateSut().GetPipelineDescriptor().FirstStrategy.StrategyInstance;
+
+        sut.IsLastAttempt(int.MaxValue, out var increment).Should().BeFalse();
+        increment.Should().BeFalse();
+    }
+
     private sealed class ThrowingFakeTimeProvider : FakeTimeProvider
     {
         public override DateTimeOffset GetUtcNow() => throw new AssertionFailedException("TimeProvider should not be used.");