Enhanced retry strategies for Leaky Bucket and Retry policies #998
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…ree separate methods
…etriable Also adds several additional retriable status codes based on Microsoft's Azure.Core Retry Policy. Each non-rate-limit response will only be retried a maximum of 3 times per request before throwing an exception.
|
@clement911 I'm merging this now so I can get a beta build published to unblock some work I'm doing with a client, but would you mind reviewing these changes and letting me know your thoughts whenever you have spare time? No rush, it doesn't have to be soon! |
|
I think this is fine. I had a custom policy already doing something similar. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This pull request adds small enhancements to the Leaky Bucket execution policy and the Retry execution policy to make them retry certain common request http request failures. I've based this code off of the retry policy found in Microsoft's Azure.Core SDK (MIT licensed), though it's not quite a one-to-one match since we don't have the concept of an HTTP pipeline in ShopifySharp.
These are the status codes that will be retried if a request fails in the Leaky Bucket and Retry execution policies:
The policy will retry these requests up to three times before throwing a
ShopifyHttpException.Importantly, the default behavior of the Leaky Bucket and Retry policies should remain intact -- they won't start retrying additional responses unless you opt-in to the behavior by using the new constructor overloads on each policy. I wanted to make this change available for testing and feedback right now, so I can gather some feedback on it.
Personally I think it makes sense to have this new behavior be the default, as having a 504 Gateway Timeout or 503 Service Unavailable break something instead of being retried is somewhat unexpected.