From dd0d9b4bb0854fa6d0147afe8b605475950f44e9 Mon Sep 17 00:00:00 2001 From: George Fu Date: Tue, 27 Feb 2024 12:57:14 -0500 Subject: [PATCH] fix(core): retry after clock skew correction (#1170) * fix(core): retry after clock skew correction * fix: variable rename * fix: make clock skew corrected errors transient * fix types and formatting * unit test update * move system clock offset check to AWS SDK * formatting --- .changeset/giant-pianos-lie.md | 7 +++++++ packages/middleware-retry/src/configurations.ts | 8 +++++++- packages/middleware-retry/src/retryDecider.ts | 3 +++ .../middleware-retry/src/retryMiddleware.spec.ts | 7 +++++++ packages/middleware-retry/src/retryMiddleware.ts | 3 ++- packages/service-error-classification/src/index.ts | 9 +++++++++ packages/types/src/retry.ts | 9 ++++++++- packages/types/src/shapes.ts | 13 ++++++++++++- 8 files changed, 55 insertions(+), 4 deletions(-) create mode 100644 .changeset/giant-pianos-lie.md diff --git a/.changeset/giant-pianos-lie.md b/.changeset/giant-pianos-lie.md new file mode 100644 index 00000000000..4fcbff06270 --- /dev/null +++ b/.changeset/giant-pianos-lie.md @@ -0,0 +1,7 @@ +--- +"@smithy/service-error-classification": patch +"@smithy/middleware-retry": patch +"@smithy/types": patch +--- + +make clock skew correcting errors transient diff --git a/packages/middleware-retry/src/configurations.ts b/packages/middleware-retry/src/configurations.ts index a0c70811bef..1b6fb7f01cd 100644 --- a/packages/middleware-retry/src/configurations.ts +++ b/packages/middleware-retry/src/configurations.ts @@ -48,7 +48,10 @@ export interface RetryInputConfig { retryStrategy?: RetryStrategy | RetryStrategyV2; } -interface PreviouslyResolved { +/** + * @internal + */ +export interface PreviouslyResolved { /** * Specifies provider for retry algorithm to use. * @internal @@ -56,6 +59,9 @@ interface PreviouslyResolved { retryMode: string | Provider; } +/** + * @internal + */ export interface RetryResolvedConfig { /** * Resolved value for input config {@link RetryInputConfig.maxAttempts} diff --git a/packages/middleware-retry/src/retryDecider.ts b/packages/middleware-retry/src/retryDecider.ts index 5a43f9f34af..e2913d065bc 100644 --- a/packages/middleware-retry/src/retryDecider.ts +++ b/packages/middleware-retry/src/retryDecider.ts @@ -6,6 +6,9 @@ import { } from "@smithy/service-error-classification"; import { SdkError } from "@smithy/types"; +/** + * @deprecated this is only used in the deprecated StandardRetryStrategy. Do not use in new code. + */ export const defaultRetryDecider = (error: SdkError) => { if (!error) { return false; diff --git a/packages/middleware-retry/src/retryMiddleware.spec.ts b/packages/middleware-retry/src/retryMiddleware.spec.ts index c06cca32c90..c39ac5bb1c4 100644 --- a/packages/middleware-retry/src/retryMiddleware.spec.ts +++ b/packages/middleware-retry/src/retryMiddleware.spec.ts @@ -138,6 +138,7 @@ describe(retryMiddleware.name, () => { (isThrottlingError as jest.Mock).mockReturnValue(true); const next = jest.fn().mockRejectedValue(requestError); const errorInfo = { + error: requestError, errorType: "THROTTLING", }; const mockRetryStrategy = { @@ -172,6 +173,7 @@ describe(retryMiddleware.name, () => { (isThrottlingError as jest.Mock).mockReturnValue(true); const next = jest.fn().mockRejectedValueOnce(mockError).mockResolvedValueOnce(mockSuccess); const errorInfo = { + error: mockError, errorType: "THROTTLING", }; const { response, output } = await retryMiddleware({ @@ -193,6 +195,7 @@ describe(retryMiddleware.name, () => { (isThrottlingError as jest.Mock).mockReturnValue(false); const next = jest.fn().mockRejectedValueOnce(mockError).mockResolvedValueOnce(mockSuccess); const errorInfo = { + error: mockError, errorType: "TRANSIENT", }; const { response, output } = await retryMiddleware({ @@ -215,6 +218,7 @@ describe(retryMiddleware.name, () => { (isThrottlingError as jest.Mock).mockReturnValue(false); const next = jest.fn().mockRejectedValueOnce(mockError).mockResolvedValueOnce(mockSuccess); const errorInfo = { + error: mockError, errorType: "SERVER_ERROR", }; const { response, output } = await retryMiddleware({ @@ -237,6 +241,7 @@ describe(retryMiddleware.name, () => { (isThrottlingError as jest.Mock).mockReturnValue(false); const next = jest.fn().mockRejectedValueOnce(mockError).mockResolvedValueOnce(mockSuccess); const errorInfo = { + error: mockError, errorType: "CLIENT_ERROR", }; const { response, output } = await retryMiddleware({ @@ -265,6 +270,7 @@ describe(retryMiddleware.name, () => { }); const next = jest.fn().mockRejectedValueOnce(mockError).mockResolvedValueOnce(mockSuccess); const errorInfo = { + error: mockError, errorType: "CLIENT_ERROR", }; const { response, output } = await retryMiddleware({ @@ -289,6 +295,7 @@ describe(retryMiddleware.name, () => { const { isInstance } = HttpResponse; ((isInstance as unknown) as jest.Mock).mockReturnValue(true); const errorInfo = { + error: mockError, errorType: "CLIENT_ERROR", retryAfterHint: retryAfterDate, }; diff --git a/packages/middleware-retry/src/retryMiddleware.ts b/packages/middleware-retry/src/retryMiddleware.ts index 0f398870f1c..50afe9f23da 100644 --- a/packages/middleware-retry/src/retryMiddleware.ts +++ b/packages/middleware-retry/src/retryMiddleware.ts @@ -55,7 +55,7 @@ export const retryMiddleware = (options: RetryResolvedConfig) => const getRetryErrorInfo = (error: SdkError): RetryErrorInfo => { const errorInfo: RetryErrorInfo = { + error, errorType: getRetryErrorType(error), }; const retryAfterHint = getRetryAfterHint(error.$response); diff --git a/packages/service-error-classification/src/index.ts b/packages/service-error-classification/src/index.ts index a2c348daf66..77941ba8a5d 100644 --- a/packages/service-error-classification/src/index.ts +++ b/packages/service-error-classification/src/index.ts @@ -10,8 +10,16 @@ import { export const isRetryableByTrait = (error: SdkError) => error.$retryable !== undefined; +/** + * @deprecated use isClockSkewCorrectedError. This is only used in deprecated code. + */ export const isClockSkewError = (error: SdkError) => CLOCK_SKEW_ERROR_CODES.includes(error.name); +/** + * @returns whether the error resulted in a systemClockOffset aka clock skew correction. + */ +export const isClockSkewCorrectedError = (error: SdkError) => error.$metadata?.clockSkewCorrected; + export const isThrottlingError = (error: SdkError) => error.$metadata?.httpStatusCode === 429 || THROTTLING_ERROR_CODES.includes(error.name) || @@ -24,6 +32,7 @@ export const isThrottlingError = (error: SdkError) => * the name "TimeoutError" to be checked by the TRANSIENT_ERROR_CODES condition. */ export const isTransientError = (error: SdkError) => + isClockSkewCorrectedError(error) || TRANSIENT_ERROR_CODES.includes(error.name) || NODEJS_TIMEOUT_ERROR_CODES.includes((error as { code?: string })?.code || "") || TRANSIENT_ERROR_STATUS_CODES.includes(error.$metadata?.httpStatusCode || 0); diff --git a/packages/types/src/retry.ts b/packages/types/src/retry.ts index ee3ba0b7270..312342f8d28 100644 --- a/packages/types/src/retry.ts +++ b/packages/types/src/retry.ts @@ -1,3 +1,5 @@ +import { SdkError } from "./shapes"; + /** * @public */ @@ -33,6 +35,11 @@ export type RetryErrorType = * @public */ export interface RetryErrorInfo { + /** + * The error thrown during the initial request, if available. + */ + error?: SdkError; + errorType: RetryErrorType; /** @@ -40,7 +47,7 @@ export interface RetryErrorInfo { * something from MQTT or any other protocol that has the ability to convey * retry info from a peer. * - * @returns the Date after which a retry should be attempted. + * The Date after which a retry should be attempted. */ retryAfterHint?: Date; } diff --git a/packages/types/src/shapes.ts b/packages/types/src/shapes.ts index 1762083e8fa..129444877c1 100644 --- a/packages/types/src/shapes.ts +++ b/packages/types/src/shapes.ts @@ -79,4 +79,15 @@ export interface SmithyException { * the base exception for the service should be used. Each client exports * a base ServiceException prefixed with the service name. */ -export type SdkError = Error & Partial & Partial; +export type SdkError = Error & + Partial & + Partial & { + $metadata?: Partial["$metadata"] & { + /** + * If present, will have value of true and indicates that the error resulted in a + * correction of the clock skew, a.k.a. config.systemClockOffset. + * This is specific to AWS SDK and sigv4. + */ + readonly clockSkewCorrected?: true; + }; + };