From 4971b60be04de2ce31846c23a3a9616d24c53b91 Mon Sep 17 00:00:00 2001 From: Martin Tomka Date: Thu, 2 Nov 2023 13:33:09 +0100 Subject: [PATCH 1/3] [Docs] Improve timeout docs --- docs/strategies/timeout.md | 53 +++++++++++++++++++ .../Retry/RetryStrategyOptions.TResult.cs | 2 +- src/Snippets/Docs/Timeout.cs | 34 ++++++++++++ 3 files changed, 88 insertions(+), 1 deletion(-) diff --git a/docs/strategies/timeout.md b/docs/strategies/timeout.md index d2c41009d02..d49a759e7ca 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 @@ -119,3 +124,51 @@ sequenceDiagram T->>P: Throws
TimeoutRejectedException P->>C: Propagates exception ``` + +## Anti-patterns + +### Ignoring Cancellation Token + +❌ DON'T + +Ignore the cancellation token provided by the resilience pipeline: + + +```cs +var outerToken = CancellationToken.None; + +var pipeline = new ResiliencePipelineBuilder() + .AddTimeout(TimeSpan.FromSeconds(1)) + .Build(); + +await pipeline.ExecuteAsync( + async innerToken => await Task.Delay(3000, outerToken), // The delay call should use innerToken + outerToken); +``` + + +**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: + + +```cs +var outerToken = CancellationToken.None; + +var pipeline = new ResiliencePipelineBuilder() + .AddTimeout(TimeSpan.FromSeconds(1)) + .Build(); + +await pipeline.ExecuteAsync( + async innerToken => await Task.Delay(3000, innerToken), + outerToken); +``` + + +**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. 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 : ResilienceStrategyOptions /// Represents the constant delay between retries. /// /// - /// This property is ignored when is set. + /// This property is ignored when is set and returns a valid value. /// /// /// The default value is 2 seconds. diff --git a/src/Snippets/Docs/Timeout.cs b/src/Snippets/Docs/Timeout.cs index fc436266f35..274973ecc76 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() + { + #region timeout-ignore-cancellation-token + + var outerToken = CancellationToken.None; + + var pipeline = new ResiliencePipelineBuilder() + .AddTimeout(TimeSpan.FromSeconds(1)) + .Build(); + + await pipeline.ExecuteAsync( + async innerToken => await Task.Delay(3000, outerToken), // The delay call should use innerToken + outerToken); + + #endregion + } + + public static async Task RespectCancellationToken() + { + #region timeout-respect-cancellation-token + + var outerToken = CancellationToken.None; + + var pipeline = new ResiliencePipelineBuilder() + .AddTimeout(TimeSpan.FromSeconds(1)) + .Build(); + + await pipeline.ExecuteAsync( + async innerToken => await Task.Delay(3000, innerToken), + outerToken); + + #endregion + } } From 033148d7280699ecaffe41068dcb5465cbdc3004 Mon Sep 17 00:00:00 2001 From: Martin Tomka Date: Thu, 2 Nov 2023 13:47:08 +0100 Subject: [PATCH 2/3] PR comments --- docs/strategies/timeout.md | 2 +- src/Snippets/Docs/Timeout.cs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/strategies/timeout.md b/docs/strategies/timeout.md index d49a759e7ca..72cd30230d0 100644 --- a/docs/strategies/timeout.md +++ b/docs/strategies/timeout.md @@ -164,7 +164,7 @@ var pipeline = new ResiliencePipelineBuilder() .Build(); await pipeline.ExecuteAsync( - async innerToken => await Task.Delay(3000, innerToken), + static async innerToken => await Task.Delay(3000, innerToken), outerToken); ``` diff --git a/src/Snippets/Docs/Timeout.cs b/src/Snippets/Docs/Timeout.cs index 274973ecc76..17de71f9931 100644 --- a/src/Snippets/Docs/Timeout.cs +++ b/src/Snippets/Docs/Timeout.cs @@ -95,7 +95,7 @@ public static async Task RespectCancellationToken() .Build(); await pipeline.ExecuteAsync( - async innerToken => await Task.Delay(3000, innerToken), + static async innerToken => await Task.Delay(3000, innerToken), outerToken); #endregion From 83c51bc1186aa796dac8add8c827ee03ff208a66 Mon Sep 17 00:00:00 2001 From: Martin Tomka Date: Thu, 2 Nov 2023 14:32:41 +0100 Subject: [PATCH 3/3] PR comments --- docs/strategies/timeout.md | 10 +++------- src/Snippets/Docs/Timeout.cs | 12 ++++++------ 2 files changed, 9 insertions(+), 13 deletions(-) diff --git a/docs/strategies/timeout.md b/docs/strategies/timeout.md index 72cd30230d0..c512cbf62b9 100644 --- a/docs/strategies/timeout.md +++ b/docs/strategies/timeout.md @@ -135,14 +135,12 @@ Ignore the cancellation token provided by the resilience pipeline: ```cs -var outerToken = CancellationToken.None; - var pipeline = new ResiliencePipelineBuilder() .AddTimeout(TimeSpan.FromSeconds(1)) .Build(); await pipeline.ExecuteAsync( - async innerToken => await Task.Delay(3000, outerToken), // The delay call should use innerToken + async innerToken => await Task.Delay(TimeSpan.FromSeconds(3), outerToken), // The delay call should use innerToken outerToken); ``` @@ -157,18 +155,16 @@ Respect the cancellation token provided by the pipeline: ```cs -var outerToken = CancellationToken.None; - var pipeline = new ResiliencePipelineBuilder() .AddTimeout(TimeSpan.FromSeconds(1)) .Build(); await pipeline.ExecuteAsync( - static async innerToken => await Task.Delay(3000, innerToken), + static async innerToken => await Task.Delay(TimeSpan.FromSeconds(3), innerToken), outerToken); ``` **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. +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/Snippets/Docs/Timeout.cs b/src/Snippets/Docs/Timeout.cs index 17de71f9931..9d05dae0a8e 100644 --- a/src/Snippets/Docs/Timeout.cs +++ b/src/Snippets/Docs/Timeout.cs @@ -69,16 +69,16 @@ public static async Task Usage() public static async Task IgnoreCancellationToken() { - #region timeout-ignore-cancellation-token - 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(3000, outerToken), // The delay call should use innerToken + async innerToken => await Task.Delay(TimeSpan.FromSeconds(3), outerToken), // The delay call should use innerToken outerToken); #endregion @@ -86,16 +86,16 @@ await pipeline.ExecuteAsync( public static async Task RespectCancellationToken() { - #region timeout-respect-cancellation-token - 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(3000, innerToken), + static async innerToken => await Task.Delay(TimeSpan.FromSeconds(3), innerToken), outerToken); #endregion