From c5d268df5fe096a3db9d49686e9e5dab1ca42931 Mon Sep 17 00:00:00 2001
From: martintmk <103487740+martintmk@users.noreply.github.com>
Date: Thu, 2 Nov 2023 15:23:45 +0100
Subject: [PATCH] [Docs] Improve timeout docs (#1767)

---
 docs/strategies/timeout.md                    | 49 +++++++++++++++++++
 .../Retry/RetryStrategyOptions.TResult.cs     |  2 +-
 src/Snippets/Docs/Timeout.cs                  | 34 +++++++++++++
 3 files changed, 84 insertions(+), 1 deletion(-)

diff --git a/docs/strategies/timeout.md b/docs/strategies/timeout.md
index d2c41009d02..c512cbf62b9 100644
--- a/docs/strategies/timeout.md
+++ b/docs/strategies/timeout.md
@@ -10,6 +10,11 @@
 
 ---
 
+The timeout resilience strategy cancels the execution if it does not complete within the specified timeout period. If the execution is canceled by the timeout strategy, it throws a `TimeoutRejectedException`. The timeout strategy operates by wrapping the incoming cancellation token with a new one. Should the original token be canceled, the timeout strategy will transparently honor the original cancellation token without throwing a `TimeoutRejectedException`.
+
+> ![IMPORTANT]
+> It is crucial that the user's callback respects the cancellation token. If it does not, the callback will continue executing even after a cancellation request, thereby ignoring the cancellation.
+
 ## Usage
 
 <!-- snippet: timeout -->
@@ -119,3 +124,47 @@ sequenceDiagram
     T->>P: Throws <br/>TimeoutRejectedException
     P->>C: Propagates exception
 ```
+
+## Anti-patterns
+
+### Ignoring Cancellation Token
+
+❌ DON'T
+
+Ignore the cancellation token provided by the resilience pipeline:
+
+<!-- snippet: timeout-ignore-cancellation-token -->
+```cs
+var pipeline = new ResiliencePipelineBuilder()
+    .AddTimeout(TimeSpan.FromSeconds(1))
+    .Build();
+
+await pipeline.ExecuteAsync(
+    async innerToken => await Task.Delay(TimeSpan.FromSeconds(3), outerToken), // The delay call should use innerToken
+    outerToken);
+```
+<!-- endSnippet -->
+
+**Reasoning**:
+
+The provided callback ignores the `innerToken` passed from the pipeline and instead uses the `outerToken`. For this reason, the cancelled `innerToken` is ignored, and the callback is not cancelled within 1 second.
+
+✅ DO
+
+Respect the cancellation token provided by the pipeline:
+
+<!-- snippet: timeout-respect-cancellation-token -->
+```cs
+var pipeline = new ResiliencePipelineBuilder()
+    .AddTimeout(TimeSpan.FromSeconds(1))
+    .Build();
+
+await pipeline.ExecuteAsync(
+    static async innerToken => await Task.Delay(TimeSpan.FromSeconds(3), innerToken),
+    outerToken);
+```
+<!-- endSnippet -->
+
+**Reasoning**:
+
+The provided callback respects the `innerToken` provided by the pipeline, and as a result, the callback is correctly cancelled by the timeout strategy after 1 second plus `TimeoutRejectedException` is thrown.
diff --git a/src/Polly.Core/Retry/RetryStrategyOptions.TResult.cs b/src/Polly.Core/Retry/RetryStrategyOptions.TResult.cs
index 4dec4806fa4..68c816ba2f3 100644
--- a/src/Polly.Core/Retry/RetryStrategyOptions.TResult.cs
+++ b/src/Polly.Core/Retry/RetryStrategyOptions.TResult.cs
@@ -67,7 +67,7 @@ public class RetryStrategyOptions<TResult> : ResilienceStrategyOptions
     /// <see cref="DelayBackoffType.Constant"/> Represents the constant delay between retries.
     /// </item>
     /// </list>
-    /// This property is ignored when <see cref="DelayGenerator"/> is set.
+    /// This property is ignored when <see cref="DelayGenerator"/> is set and returns a valid <see cref="TimeSpan"/> value.
     /// </remarks>
     /// <value>
     /// The default value is 2 seconds.
diff --git a/src/Snippets/Docs/Timeout.cs b/src/Snippets/Docs/Timeout.cs
index fc436266f35..9d05dae0a8e 100644
--- a/src/Snippets/Docs/Timeout.cs
+++ b/src/Snippets/Docs/Timeout.cs
@@ -66,4 +66,38 @@ public static async Task Usage()
 
         #endregion
     }
+
+    public static async Task IgnoreCancellationToken()
+    {
+        var outerToken = CancellationToken.None;
+
+        #region timeout-ignore-cancellation-token
+
+        var pipeline = new ResiliencePipelineBuilder()
+            .AddTimeout(TimeSpan.FromSeconds(1))
+            .Build();
+
+        await pipeline.ExecuteAsync(
+            async innerToken => await Task.Delay(TimeSpan.FromSeconds(3), outerToken), // The delay call should use innerToken
+            outerToken);
+
+        #endregion
+    }
+
+    public static async Task RespectCancellationToken()
+    {
+        var outerToken = CancellationToken.None;
+
+        #region timeout-respect-cancellation-token
+
+        var pipeline = new ResiliencePipelineBuilder()
+            .AddTimeout(TimeSpan.FromSeconds(1))
+            .Build();
+
+        await pipeline.ExecuteAsync(
+            static async innerToken => await Task.Delay(TimeSpan.FromSeconds(3), innerToken),
+            outerToken);
+
+        #endregion
+    }
 }