fix: make clock skew corrected errors transient

Author: kuhe

.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

.changeset/red-carrots-clap.md

@@ -35,18 +35,11 @@ const defaultSuccessHandler: SuccessHandler = (
   signingProperties: Record<string, unknown>
 ): void => {};
 
-/**
- * @internal
- */
-interface HttpSigningMiddlewarePreviouslyResolved {
-  systemClockOffset?: number;
-}
-
 /**
  * @internal
  */
 export const httpSigningMiddleware = <Input extends object, Output extends object>(
-  config: HttpSigningMiddlewarePreviouslyResolved
+  config: object
 ): FinalizeRequestMiddleware<Input, Output> => (
   next: FinalizeHandler<Input, Output>,
   context: HttpSigningMiddlewareHandlerExecutionContext
@@ -67,35 +60,10 @@ export const httpSigningMiddleware = <Input extends object, Output extends objec
     identity,
     signer,
   } = scheme;
-  const initialSystemClockOffset = config.systemClockOffset | 0;
-
-  const makeSignedRequest = async () =>
-    next({
-      ...args,
-      request: await signer.sign(args.request as HttpRequest, identity, signingProperties),
-    });
-
-  const onError = (signer.errorHandler || defaultErrorHandler)(signingProperties);
-  const onSuccess = signer.successHandler || defaultSuccessHandler;
-
-  const output = await makeSignedRequest().catch(async (error: unknown) => {
-    let thrownError: unknown;
-    try {
-      onError(error as Error);
-    } catch (e) {
-      thrownError = e;
-    }
-    const latestSystemClockOffset = config.systemClockOffset | 0;
-    const systemClockOffsetWasModified = initialSystemClockOffset !== latestSystemClockOffset;
-
-    if (systemClockOffsetWasModified) {
-      return makeSignedRequest().catch(onError);
-    }
-
-    if (thrownError) {
-      throw thrownError;
-    }
-  });
-  onSuccess(output.response, signingProperties);
+  const output = await next({
+    ...args,
+    request: await signer.sign(args.request, identity, signingProperties),
+  }).catch((signer.errorHandler || defaultErrorHandler)(signingProperties));
+  (signer.successHandler || defaultSuccessHandler)(output.response, signingProperties);
   return output;
 };

.changeset/rude-apricots-remember.md

@@ -48,14 +48,22 @@ export interface RetryInputConfig {
   retryStrategy?: RetryStrategy | RetryStrategyV2;
 }
 
-interface PreviouslyResolved {
+/**
+ * @internal
+ */
+export interface PreviouslyResolved {
   /**
    * Specifies provider for retry algorithm to use.
    * @internal
    */
   retryMode: string | Provider<string>;
+
+  systemClockOffset?: number;
 }
 
+/**
+ * @internal
+ */
 export interface RetryResolvedConfig {
   /**
    * Resolved value for input config {@link RetryInputConfig.maxAttempts}

packages/core/src/middleware-http-signing/httpSigningMiddleware.ts

@@ -16,15 +16,18 @@ import {
   RetryStrategyV2,
   RetryToken,
   SdkError,
+  SkdErrorWithClockSkewMetadata,
 } from "@smithy/types";
 import { INVOCATION_ID_HEADER, REQUEST_HEADER } from "@smithy/util-retry";
 import { v4 } from "uuid";
 
-import { RetryResolvedConfig } from "./configurations";
+import { PreviouslyResolved, RetryResolvedConfig } from "./configurations";
 import { isStreamingPayload } from "./isStreamingPayload/isStreamingPayload";
 import { asSdkError } from "./util";
 
-export const retryMiddleware = (options: RetryResolvedConfig) => <Output extends MetadataBearer = MetadataBearer>(
+export const retryMiddleware = (options: RetryResolvedConfig & Partial<PreviouslyResolved>) => <
+  Output extends MetadataBearer = MetadataBearer
+>(
   next: FinalizeHandler<any, Output>,
   context: HandlerExecutionContext
 ): FinalizeHandler<any, Output> => async (
@@ -45,19 +48,31 @@ export const retryMiddleware = (options: RetryResolvedConfig) => <Output extends
     if (isRequest) {
       request.headers[INVOCATION_ID_HEADER] = v4();
     }
+
+    let initialSystemClockOffset = 0;
+
     while (true) {
       try {
         if (isRequest) {
           request.headers[REQUEST_HEADER] = `attempt=${attempts + 1}; max=${maxAttempts}`;
         }
+        initialSystemClockOffset = options.systemClockOffset | 0;
         const { response, output } = await next(args);
         retryStrategy.recordSuccess(retryToken);
         output.$metadata.attempts = attempts + 1;
         output.$metadata.totalRetryDelay = totalRetryDelay;
         return { response, output };
-      } catch (e) {
-        const retryErrorInfo = getRetryErrorInfo(e);
-        lastError = asSdkError(e);
+      } catch (e: unknown) {
+        const latestSystemClockOffset = options.systemClockOffset | 0;
+        const clockSkewCorrected = initialSystemClockOffset !== latestSystemClockOffset;
+
+        const sdkError = e as SkdErrorWithClockSkewMetadata;
+        if (clockSkewCorrected && sdkError.$metadata && typeof sdkError.$metadata === "object") {
+          sdkError.$metadata.clockSkewCorrected = true;
+        }
+
+        const retryErrorInfo = getRetryErrorInfo(sdkError);
+        lastError = asSdkError(sdkError);
 
         if (isRequest && isStreamingPayload(request)) {
           (context.logger instanceof NoOpLogger ? console : context.logger)?.warn(
@@ -95,8 +110,9 @@ const isRetryStrategyV2 = (retryStrategy: RetryStrategy | RetryStrategyV2) =>
   typeof (retryStrategy as RetryStrategyV2).refreshRetryTokenForRetry !== "undefined" &&
   typeof (retryStrategy as RetryStrategyV2).recordSuccess !== "undefined";
 
-const getRetryErrorInfo = (error: SdkError): RetryErrorInfo => {
+const getRetryErrorInfo = (error: SkdErrorWithClockSkewMetadata): RetryErrorInfo => {
   const errorInfo: RetryErrorInfo = {
+    error,
     errorType: getRetryErrorType(error),
   };
   const retryAfterHint = getRetryAfterHint(error.$response);
@@ -106,7 +122,7 @@ const getRetryErrorInfo = (error: SdkError): RetryErrorInfo => {
   return errorInfo;
 };
 
-const getRetryErrorType = (error: SdkError): RetryErrorType => {
+const getRetryErrorType = (error: SkdErrorWithClockSkewMetadata): RetryErrorType => {
   if (isThrottlingError(error)) return "THROTTLING";
   if (isTransientError(error)) return "TRANSIENT";
   if (isServerError(error)) return "SERVER_ERROR";

packages/middleware-retry/src/configurations.ts

@@ -1,4 +1,4 @@
-import { SdkError } from "@smithy/types";
+import { SdkError, SkdErrorWithClockSkewMetadata } from "@smithy/types";
 
 import {
   CLOCK_SKEW_ERROR_CODES,
@@ -23,12 +23,13 @@ export const isThrottlingError = (error: SdkError) =>
  * cause where the NodeHttpHandler does not decorate the Error with
  * the name "TimeoutError" to be checked by the TRANSIENT_ERROR_CODES condition.
  */
-export const isTransientError = (error: SdkError) =>
+export const isTransientError = (error: SkdErrorWithClockSkewMetadata) =>
+  error.$metadata?.clockSkewCorrected ||
   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);
 
-export const isServerError = (error: SdkError) => {
+export const isServerError = (error: SkdErrorWithClockSkewMetadata) => {
   if (error.$metadata?.httpStatusCode !== undefined) {
     const statusCode = error.$metadata.httpStatusCode;
     if (500 <= statusCode && statusCode <= 599 && !isTransientError(error)) {

packages/middleware-retry/src/retryMiddleware.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;
 }

packages/service-error-classification/src/index.ts

@@ -80,3 +80,14 @@ export interface SmithyException {
  * a base ServiceException prefixed with the service name.
  */
 export type SdkError = Error & Partial<SmithyException> & Partial<MetadataBearer>;
+
+/**
+ * @internal
+ *
+ * @deprecated for same reason as SdkError. Use public client modeled exceptions in application code.
+ */
+export type SkdErrorWithClockSkewMetadata = SdkError & {
+  $metadata: SdkError["$metadata"] & {
+    clockSkewCorrected?: boolean;
+  };
+};