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
smithy-lang · Feb 27, 2024 · dd0d9b4 · dd0d9b4
1 parent 3b37740
commit dd0d9b4
Show file tree

Hide file tree

Showing 8 changed files with 55 additions and 4 deletions.
diff --git a/.changeset/giant-pianos-lie.md 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
@@ -48,14 +48,20 @@ export interface RetryInputConfig {
   retryStrategy?: RetryStrategy | RetryStrategyV2;
 }
 
-interface PreviouslyResolved {
+/**
+ * @internal
+ */
+export interface PreviouslyResolved {
   /**
    * Specifies provider for retry algorithm to use.
    * @internal
    */
   retryMode: string | Provider<string>;
 }
 
+/**
+ * @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
@@ -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
@@ -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
@@ -55,7 +55,7 @@ export const retryMiddleware = (options: RetryResolvedConfig) => <Output extends
         output.$metadata.attempts = attempts + 1;
         output.$metadata.totalRetryDelay = totalRetryDelay;
         return { response, output };
-      } catch (e) {
+      } catch (e: any) {
         const retryErrorInfo = getRetryErrorInfo(e);
         lastError = asSdkError(e);
 
@@ -97,6 +97,7 @@ const isRetryStrategyV2 = (retryStrategy: RetryStrategy | RetryStrategyV2) =>
 
 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
@@ -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
@@ -1,3 +1,5 @@
+import { SdkError } from "./shapes";
+
 /**
  * @public
  */
@@ -33,14 +35,19 @@ export type RetryErrorType =
  * @public
  */
 export interface RetryErrorInfo {
+  /**
+   * The error thrown during the initial request, if available.
+   */
+  error?: SdkError;
+
   errorType: RetryErrorType;
 
   /**
    * Protocol hint. This could come from Http's 'retry-after' header or
    * 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
@@ -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<SmithyException> & Partial<MetadataBearer>;
+export type SdkError = Error &
+  Partial<SmithyException> &
+  Partial<MetadataBearer> & {
+    $metadata?: Partial<MetadataBearer>["$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;
+    };
+  };