feat(middleware-retry) add RetryStrategyV2 #4248
Conversation
| const retryAfter = response.headers[retryAfterHeaderName]; | ||
|
|
||
| const retryAfterSeconds = Number(retryAfter); | ||
| const retryAfterDate = new Date(retryAfter); |
There was a problem hiding this comment.
the line above implies retryAfter is a seconds unit, but then you initialize a Date with it?
Is retryAfter seconds or milliseconds?
There was a problem hiding this comment.
below you multiply by 1000, so shouldn't this also be x1000?
There was a problem hiding this comment.
This was patterned off of this logic in the current StandardRetryStrategy. retyAfter should be a string of the number of seconds, but if it isn't a number, we let new Date() parse the raw retryAfter string.
| export const isServerError = (error: SdkError) => { | ||
| if (error.$metadata?.httpStatusCode !== undefined) { | ||
| const statusCode = error.$metadata.httpStatusCode; | ||
| if (statusCode > 499 && statusCode <= 599 && !isTransientError(error)) { |
There was a problem hiding this comment.
| if (statusCode > 499 && statusCode <= 599 && !isTransientError(error)) { | |
| if (500 <= statusCode && statusCode <= 599 && !isTransientError(error)) { |
|
How was this change tested? |
srchase
changed the title
srchase
force-pushed
the
middleware-retry-retrystrategyv2
branch
from
978aab3 to
a8d783d
Compare
Integration tests were added to |
srchase
force-pushed
the
middleware-retry-retrystrategyv2
branch
from
a8d783d to
0a328e8
Compare
|
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
This PR updates the middleware-retry to use the RetryStrategyV2 and its implementation added in #4224.
Clients are updated to pull in retry related configuration values that were moved from
middleware-retytoutil-retry.Description
The retry middleware will now use the new
RetryStrategyV2implementation that was added in #4224. For customers that have previously implemented and configured aRetryStrategy, the middleware will continue to use that strategy.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.