diff --git a/docs/strategies/fallback.md b/docs/strategies/fallback.md index b0a467eb0b7..19be48fd558 100644 --- a/docs/strategies/fallback.md +++ b/docs/strategies/fallback.md @@ -68,15 +68,15 @@ new ResiliencePipelineBuilder() | `FallbackAction` | `Null`, **Required** | Fallback action to be executed. | | `OnFallback` | `null` | Event that is raised when fallback happens. | +## Patterns and anti-patterns -## Patterns and Anti-patterns -Throughout the years many people have used Polly in so many different ways. Some reoccuring patterns are suboptimal. So, this section shows the donts and dos. +Over the years, many developers have used Polly in various ways. Some of these recurring patterns may not be ideal. This section highlights the recommended practices and those to avoid. ### 1 - Using fallback to replace thrown exception ❌ DON'T -Throw custom exception from the `OnFallback` +Throw custom exceptions from the `OnFallback`: ```cs @@ -92,11 +92,12 @@ var fallback = new ResiliencePipelineBuilder() **Reasoning**: -- Throwing an exception in an user-defined delegate is never a good idea because it is breaking the normal control flow. + +Throwing an exception from a user-defined delegate can disrupt the normal control flow. ✅ DO -Use `ExecuteOutcomeAsync` and then assess `Exception` +Use `ExecuteOutcomeAsync` and then evaluate the `Exception`: ```cs @@ -109,10 +110,8 @@ if (outcome.Exception is HttpRequestException hre) **Reasoning**: -- This approach executes the strategy/pipeline without "jumping out from the normal flow" -- If you find yourself in a situation that you write this Exception "remapping" logic again and again - - then mark the to-be-decorated method as `private` - - and expose the "remapping" logic as `public` + +This method lets you execute the strategy or pipeline smoothly, without unexpected interruptions. If you repeatedly find yourself writing this exception "remapping" logic, consider marking the method you wish to decorate as `private` and expose the "remapping" logic publicly. ```cs @@ -143,13 +142,13 @@ private static ValueTask ActionCore() ``` -### 2 - Using retry to perform fallback +### 2 - Using retry for fallback -Lets suppose you have a primary and a secondary endpoints. If primary fails then you want to call the secondary. +Suppose you have a primary and a secondary endpoint. If the primary fails, you want to call the secondary. ❌ DON'T -Use retry to perform fallback +Use retry for fallback: ```cs @@ -185,14 +184,12 @@ return result; **Reasoning**: -- Retry strategy by default executes the exact same operation at most `n` times - - where `n` equals to the initial attempt + `MaxRetryAttempts` - - So, in this particular case this means __2__ -- Here the fallback is produced as a side-effect rather than as a substitute + +A retry strategy by default executes the same operation up to `n` times, where `n` equals the initial attempt plus `MaxRetryAttempts`. In this case, that means **2** times. Here, the fallback is introduced as a side effect rather than a replacement. ✅ DO -Use fallback to call secondary +Use fallback to call the secondary: ```cs @@ -210,19 +207,20 @@ return await fallback.ExecuteAsync(CallPrimary, CancellationToken.None); **Reasoning**: -- The to-be-decorated code is executed only once -- The fallback value will be returned without any extra code (no need for `Context` or `ExecuteOutcomeAsync`) + +- The target code is executed only once. +- The fallback value is returned directly, eliminating the need for additional code like `Context` or `ExecuteOutcomeAsync`. ### 3 - Nesting `ExecuteAsync` calls -There are many ways to combine multiple strategies together. One of the least desired one is the `Execute` hell. +Combining multiple strategies can be achieved in various ways. However, deeply nesting `ExecuteAsync` calls can lead to what's commonly referred to as `Execute` hell. > [!NOTE] -> This is not strictly related to Fallback but we have seen it many times when Fallback was the most outer. +> While this isn't strictly tied to the Fallback mechanism, it's frequently observed when Fallback is the outermost layer. ❌ DON'T -Nest `ExecuteAsync` calls +Nest `ExecuteAsync` calls: ```cs @@ -239,11 +237,12 @@ return result; **Reasoning**: -- This is the same as javascript's callback hell or pyramid of doom -- It is pretty easy to refer to the wrong `CancellationToken` parameter + +This is akin to JavaScript's callback hell or the pyramid of doom. It's easy to mistakenly reference the wrong `CancellationToken` parameter. ✅ DO -Use `ResiliencePipelineBuilder` to chain them + +Use `ResiliencePipelineBuilder` to chain strategies: ```cs @@ -257,5 +256,5 @@ return await pipeline.ExecuteAsync(CallExternalSystem, CancellationToken.None); **Reasoning**: -- Here we are relying Polly provided escalation mechanism rather than building our own via nesting -- The `CancellationToken`s are propagated between the policies automatically on your behalf + +In this approach, we leverage the escalation mechanism provided by Polly rather than creating our own through nesting. `CancellationToken`s are automatically propagated between the strategies for you. diff --git a/docs/strategies/retry.md b/docs/strategies/retry.md index e7e50a535b8..9906e0509f6 100644 --- a/docs/strategies/retry.md +++ b/docs/strategies/retry.md @@ -106,14 +106,15 @@ new ResiliencePipelineBuilder().AddRetry(new RetryStrategyOptions | `DelayGenerator` | `null` | Used for generating custom delays for retries. | | `OnRetry` | `null` | Action executed when retry occurs. | -## Patterns and Anti-patterns -Throughout the years many people have used Polly in so many different ways. Some reoccuring patterns are suboptimal. So, this section shows the donts and dos. +## Patterns and anti-patterns + +Over the years, many developers have used Polly in various ways. Some of these recurring patterns may not be ideal. This section highlights the recommended practices and those to avoid. ### 1 - Overusing builder methods ❌ DON'T -Use more than one `Handle/HandleResult` +Overuse `Handle/HandleResult`: ```cs @@ -133,12 +134,12 @@ var retry = new ResiliencePipelineBuilder() **Reasoning**: -- Even though this builder method signature is quite concise you repeat the same thing over and over (_please trigger retry if the to-be-decorated code throws XYZ exception_). -- A better approach would be to tell _please trigger retry if the to-be-decorated code throws one of the retriable exceptions_. + +Using multiple `Handle/HandleResult` methods is redundant. Instead of specifying to retry if the decorated code throws a certain exception repeatedly, it's more efficient to state that retries should occur if any of the retryable exceptions are thrown. ✅ DO -Use collections and simple predicate functions +Use collections and simple predicate functions: ```cs @@ -148,14 +149,14 @@ ImmutableArray networkExceptions = new[] typeof(HttpRequestException), }.ToImmutableArray(); -ImmutableArray policyExceptions = new[] +ImmutableArray strategyExceptions = new[] { typeof(TimeoutRejectedException), typeof(BrokenCircuitException), typeof(RateLimitRejectedException), }.ToImmutableArray(); -ImmutableArray retryableExceptions = networkExceptions.Union(policyExceptions) +ImmutableArray retryableExceptions = networkExceptions.Union(strategyExceptions) .ToImmutableArray(); var retry = new ResiliencePipelineBuilder() @@ -169,14 +170,14 @@ var retry = new ResiliencePipelineBuilder() **Reasoning**: -- This approach embraces re-usability. - - For instance the `networkExceptions` can be reused across many the strategies (retry, circuit breaker, etc..). -### 2 - Using retry as a periodical executor +Grouping exceptions simplifies the configuration and improves reusability. For example, `networkExceptions` can be reused in various strategies such as retry, circuit breaker, and more. + +### 2 - Using retry for periodic execution ❌ DON'T -Define a retry strategy to run forever in a given frequency +Use a retry strategy to run indefinitely at a specified interval: ```cs @@ -191,22 +192,23 @@ var retry = new ResiliencePipelineBuilder() **Reasoning**: -- The sleep period can be blocking or non-blocking depending on how you define your strategy/pipeline. -- Even if it is used in a non-blocking manner it consumes (_unnecessarily_) memory which can't be garbage collected. + +The waiting period can be either blocking or non-blocking, based on the defined strategy/pipeline. Even when used non-blockingly, it unnecessarily consumes memory that can't be reclaimed by the garbage collector. ✅ DO -Use appropriate tool to schedule recurring jobs like *Quartz.Net*, *Hangfire* or similar. +Use a suitable tool to schedule recurring tasks, such as *Quartz.Net*, *Hangfire*, or others. **Reasoning**: -- Polly was never design to support this use case rather than its main aim is to help you overcome **short** transient failures. -- Dedicated job scheduler tools are more efficient (in terms of memory) and can be configured to withstand machine failure by utilizing persistence storage. + +- Polly was not designed to support this scenario; its primary purpose is to help manage **brief** transient failures. +- Specialized job scheduling tools are more memory-efficient and can be set up to withstand machine failures by using persistent storage. ### 3 - Combining multiple sleep duration strategies ❌ DON'T -Mix the ever increasing values with constant ones +Mix increasing values with constant ones: ```cs @@ -227,16 +229,17 @@ var retry = new ResiliencePipelineBuilder() ``` -Reasoning: -- By changing the behaviour based on state we basically created here a state machine -- Even though it is a really compact/concise way to express sleep durations there are three main drawbacks - - This approach does not embrace re-usability (you can't re-use only the quick retries) - - The sleep duration logic is tightly coupled to the `AttemptNumber` - - It is harder to unit test +**Reasoning:** + +Using this approach essentially turns the logic into a state machine. Although this offers a concise way to express sleep durations, it has several disadvantages: + +- It doesn't support reusability (for instance, you can't use only the quick retries). +- The sleep duration logic is closely tied to the `AttemptNumber`. +- Testing becomes more challenging. ✅ DO -Define two separate retry strategy options and chain them +Use two distinct retry strategy options and link them: ```cs @@ -262,20 +265,19 @@ var retry = new ResiliencePipelineBuilder() ``` -Reasoning: -- Even though this approach is a bit more verbose (compared to the previous one) it is more flexible -- You can compose the retry strategies in any order (slower is the outer and quicker is the inner or vice versa) -- You can define different triggers for the retry policies - - Which allows you to switch back and forth between the policies based on the thrown exception or on the result - - There is no strict order so, quick and slow retries can interleave +**Reasoning**: + +- While this method may appear more verbose than the first, it offers greater flexibility. +- Retry strategies can be arranged in any order (either slower first and then quicker, or the other way around). +- Different triggers can be defined for the retry strategies, allowing for switches between them based on exceptions or results. The order isn't fixed, so quick and slow retries can alternate. -### 4 - Branching retry logic based on request url +### 4 - Branching retry logic based on request URL -Lets suppose you have an `HttpClient` and you want to decorate it with a retry only if a request is against a certain endpoint +Suppose you have an `HttpClient` and you want to add a retry only for specific endpoints. ❌ DON'T -Use `ResiliencePipeline.Empty` and `?:` operator +Use `ResiliencePipeline.Empty` and the `?:` operator: ```cs @@ -287,12 +289,12 @@ var retry = **Reasoning**: -- In this case the triggering conditions/logic are scattered in multiple places -- From extensibility perspective it is also not desirable since it can easily become less and less legible as you add more conditions + +The triggering conditions and logic are spread across different sections. This design is not ideal for extensibility since adding more conditions can make the code less readable. ✅ DO -Use the `ShouldHandle` clause to define triggering logic +Use the `ShouldHandle` clause to define the triggering logic: ```cs @@ -306,14 +308,15 @@ var retry = new ResiliencePipelineBuilder() **Reasoning**: -- The triggering conditions are located in a single, well-known place -- There is no need to cover _"what to do when policy shouldn't trigger"_ -### 5 - Calling a given method before/after each retry attempt +- The conditions for triggering are consolidated in a familiar and easily accessible location. +- You don't need to specify actions for scenarios when the strategy shouldn't be triggered. + +### 5 - Calling a method before/after each retry attempt ❌ DON'T -Call explicitly a given method before `Execute`/`ExecuteAsync` +Call a specific method before `Execute`/`ExecuteAsync`: ```cs @@ -334,15 +337,14 @@ await retry.ExecuteAsync(DoSomething); **Reasoning**: -- The `OnRetry` is called before each **retry** attempt. - - So, it won't be called before the very first initial attempt (because that is not a retry) -- If this strategy is used in multiple places it is quite likely that you will forgot to call `BeforeEachAttempt` before every `Execute` calls -- Here the naming is very explicit but in real world scenario your method might not be prefixed with `Before` - - So, one might call it after the `Execute` call which is not the intended usage + +- The `OnRetry` function is triggered before each **retry** attempt, but it doesn't activate before the initial attempt since it's not considered a retry. +- Using this method across various parts can lead to accidentally omitting the `BeforeEachAttempt` call before every `Execute`. +- Even though the naming here is straightforward, in real-world scenarios, your method might not start with 'Before', leading to potential misuse by calling it after the `Execute`. ✅ DO -Decorate the two method calls together +Group the two method calls: ```cs @@ -359,16 +361,16 @@ await retry.ExecuteAsync(ct => **Reasoning**: -- If the `DoSomething` and `BeforeEachRetry` coupled together then decorate them together - - Or create a simple wrapper to call them in the desired order -### 6 - Having a single policy to cover multiple failures +If `DoSomething` and `BeforeEachAttempt` are interdependent, group them or craft a simple wrapper to invoke them in the correct sequence. + +### 6 - Having a single strategy for multiple failures -Lets suppose we have an `HttpClient` which issues a request and then we try to parse a large Json +Suppose we have an `HttpClient` that issues a request and then we try to parse a large JSON. ❌ DON'T -Have a single policy to cover everything +Use a single strategy for everything: ```cs @@ -391,13 +393,12 @@ await pipeline.ExecuteAsync(async (ct) => **Reasoning**: -- In the previous point it was suggested that _if the `X` and `Y` coupled together then decorate them together_ - - only if they are all part of the same failure domain - - in other words a pipeline should cover one failure domain + +Previously, it was suggested that you should combine `X` and `Y` only if they are part of the same failure domain. In simpler terms, a pipeline should address only one type of failure. ✅ DO -Define a strategy per failure domain +Define a strategy for each failure domain: ```cs @@ -420,16 +421,16 @@ var foo = await timeout.ExecuteAsync((ct) => JsonSerializer.DeserializeAsync **Reasoning**: -- Network call's failure domain is different than deserialization's failures - - Having dedicated strategies makes the application more robust against different transient failures -### 7 - Cancelling retry in case of given exception +The failure domain of a network call is different from that of deserialization. Using dedicated strategies makes the application more resilient to various transient failures. + +### 7 - Cancelling retry for specific exceptions -After you receive a `TimeoutException` you don't want to perform any more retries +If you encounter a `TimeoutException`, you may not want to retry the operation. ❌ DON'T -Add cancellation logic inside `OnRetry` +Embed cancellation logic within `OnRetry`: ```cs @@ -455,12 +456,12 @@ var retry = new ResiliencePipelineBuilder() **Reasoning**: -- The triggering logic/conditions should be placed inside `ShouldHandle` -- "Jumping out from a strategy" from a user-defined delegate either via an `Exception` or by a `CancellationToken` just complicates the control flow unnecessarily + +Conditions for triggering retries should be located in `ShouldHandle`. Bypassing the strategy from within a user-defined delegate—either through an `Exception` or a `CancellationToken`—unnecessarily complicates the control flow. ✅ DO -Define triggering logic inside `ShouldHandle` +Set the condition for retry within `ShouldHandle`: ```cs @@ -474,5 +475,5 @@ var retry = new ResiliencePipelineBuilder() **Reasoning**: -- As it was stated above please use the dedicated place to define triggering condition - - Try to rephrase your original exit condition in a way to express _when should a retry trigger_ + +As previously mentioned, always use the designated area to define retry conditions. Reframe your original exit conditions to specify when a retry should be initiated. diff --git a/src/Snippets/Docs/Retry.cs b/src/Snippets/Docs/Retry.cs index 1386d6a93ea..79675978fde 100644 --- a/src/Snippets/Docs/Retry.cs +++ b/src/Snippets/Docs/Retry.cs @@ -139,14 +139,14 @@ public static void Pattern_1() typeof(HttpRequestException), }.ToImmutableArray(); - ImmutableArray policyExceptions = new[] + ImmutableArray strategyExceptions = new[] { typeof(TimeoutRejectedException), typeof(BrokenCircuitException), typeof(RateLimitRejectedException), }.ToImmutableArray(); - ImmutableArray retryableExceptions = networkExceptions.Union(policyExceptions) + ImmutableArray retryableExceptions = networkExceptions.Union(strategyExceptions) .ToImmutableArray(); var retry = new ResiliencePipelineBuilder()