feat(retries): SRA retries #4187
Conversation
| "tslib": "^2.3.1", | ||
| "uuid": "^8.3.2" | ||
| }, | ||
| "devDependencies": { | ||
| "@aws-sdk/node-config-provider": "*", | ||
| "@aws-sdk/util-retry": "*", |
There was a problem hiding this comment.
shouldn't need this in devDeps, should be deps only
| export const isServerError = (error: SdkError) => { | ||
| if (error.$metadata?.httpStatusCode !== undefined) { | ||
| const statusCode = error.$metadata.httpStatusCode; | ||
| if (statusCode > 500 && statusCode <= 599 && !isTransientError(error)) { |
There was a problem hiding this comment.
shouldn't this include 500 itself?
| DEFAULT = "Default", | ||
| NONE = "None", | ||
| FULL = "Full", | ||
| DECORRELATED = "Decorrelated", |
There was a problem hiding this comment.
how about setting the enum string values equal their variable names?
| export class AdaptiveRetryStrategy implements RetryStrategyV2 { | ||
| private rateLimiter: RateLimiter; | ||
| private standardRetryStrategy: StandardRetryStrategy; | ||
| public mode: string = RETRY_MODES.ADAPTIVE; |
There was a problem hiding this comment.
this sounds like it should be readonly
| return this.standardRetryStrategy.refreshRetryTokenForRetry(tokenToRenew, errorInfo); | ||
| } | ||
|
|
||
| recordSuccess(token: StandardRetryToken): void { |
There was a problem hiding this comment.
please use explicit access modifier public
|
|
||
| export class StandardRetryStrategy implements RetryStrategyV2 { | ||
| private retryToken: StandardRetryToken; | ||
| public mode: string = RETRY_MODES.STANDARD; |
| const attempts = tokenToRenew.getRetryCount(); | ||
| return ( | ||
| attempts < maxAttempts && | ||
| attempts < DEFAULT_MAX_ATTEMPTS && |
There was a problem hiding this comment.
this should not include DEFAULT_MAX_ATTEMPTS unless it also means ABSOLUTE_MAX_ATTEMPTS a.k.a.
MAX_MAX_ATTEMPTS
|
This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs and link to relevant comments in this thread. |
Issue
Draft PR that adds a new package,
util-retry, which addsStandardRetryStrategyandAdaptiveRetryStrategy.middleware-retryis updated with new configuration resolution, and the middleware itself prefers the new strategies, while the existing strategies are deprecated.This is a draft only as the clients have not been updated to use the new retry strategy. Once this has been reviewed, the PR will be updated to add the client codegen changes.
Description
What does this implement/fix? Explain your changes.
Testing
How was this change tested?
Additional context
Add any other context about the PR here.
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.