fix(middleware-retry): defaultStrategy handles any error (#2349)

* fix(middleware-retry): defaultStrategy handles any error

The lower level stacks than throw anything to retry strategy in
practice, so casting them to SdkError type is unsafe. With this
change the retryStrategy will do best effort to convert the
thrown any object to SdkError and then supply to retryDecider
and so on.

Another change is making SdkError type more general. It previously
requires $fault, $metadata, which is obviously incorrect. Error
thrown by SDK itself never contains these keys. So I mark them
optional. Granted this is a breaking change to function interface
like RetryDecider, but anyone depending it being required should
already fail anyway.

* fix(protocol-test): fake handler should throw error

Author: AllanZhengYP

packages/middleware-retry/src/defaultStrategy.spec.ts

@@ -100,6 +100,16 @@ describe("defaultStrategy", () => {
     });
   });
 
+  it("handles non-standard errors", () => {
+    const nonStandardErrors = [undefined, "foo", { foo: "bar" }, 123, false, null];
+    const maxAttempts = 1;
+    const retryStrategy = new StandardRetryStrategy(() => Promise.resolve(maxAttempts));
+    for (const error of nonStandardErrors) {
+      next = jest.fn().mockRejectedValue(error);
+      expect(retryStrategy.retry(next, { request: { headers: {} } } as any)).rejects.toBeInstanceOf(Error);
+    }
+  });
+
   describe("retryDecider init", () => {
     it("sets defaultRetryDecider if options is undefined", () => {
       const retryStrategy = new StandardRetryStrategy(() => Promise.resolve(maxAttempts));

packages/middleware-retry/src/defaultStrategy.ts

@@ -129,7 +129,8 @@ export class StandardRetryStrategy implements RetryStrategy {
         output.$metadata.totalRetryDelay = totalDelay;
 
         return { response, output };
-      } catch (err) {
+      } catch (e) {
+        const err = asSdkError(e);
         attempts++;
         if (this.shouldRetry(err as SdkError, attempts, maxAttempts)) {
           retryTokenAmount = this.retryQuota.retrieveRetryTokens(err);
@@ -154,3 +155,10 @@ export class StandardRetryStrategy implements RetryStrategy {
     }
   }
 }
+
+const asSdkError = (error: unknown): SdkError => {
+  if (error instanceof Error) return error;
+  if (error instanceof Object) return Object.assign(new Error(), error);
+  if (typeof error === "string") return new Error(error);
+  return new Error(`AWS SDK error wrapper for ${error}`);
+};

packages/smithy-client/src/sdk-error.ts

@@ -2,4 +2,4 @@ import { MetadataBearer } from "@aws-sdk/types";
 
 import { SmithyException } from "./exception";
 
-export type SdkError = Error & SmithyException & MetadataBearer;
+export type SdkError = Error & Partial<SmithyException> & Partial<MetadataBearer>;

protocol_tests/aws-ec2/tests/functional/ec2query.spec.ts

@@ -27,8 +27,10 @@ import { Readable } from "stream";
 /**
  * Throws an expected exception that contains the serialized request.
  */
-class EXPECTED_REQUEST_SERIALIZATION_ERROR {
-  constructor(readonly request: HttpRequest) {}
+class EXPECTED_REQUEST_SERIALIZATION_ERROR extends Error {
+  constructor(readonly request: HttpRequest) {
+    super();
+  }
 }
 
 /**

protocol_tests/aws-json/tests/functional/awsjson1_1.spec.ts

@@ -18,8 +18,10 @@ import { Readable } from "stream";
 /**
  * Throws an expected exception that contains the serialized request.
  */
-class EXPECTED_REQUEST_SERIALIZATION_ERROR {
-  constructor(readonly request: HttpRequest) {}
+class EXPECTED_REQUEST_SERIALIZATION_ERROR extends Error {
+  constructor(readonly request: HttpRequest) {
+    super();
+  }
 }
 
 /**

protocol_tests/aws-query/tests/functional/awsquery.spec.ts

@@ -35,8 +35,10 @@ import { Readable } from "stream";
 /**
  * Throws an expected exception that contains the serialized request.
  */
-class EXPECTED_REQUEST_SERIALIZATION_ERROR {
-  constructor(readonly request: HttpRequest) {}
+class EXPECTED_REQUEST_SERIALIZATION_ERROR extends Error {
+  constructor(readonly request: HttpRequest) {
+    super();
+  }
 }
 
 /**

protocol_tests/aws-restjson/tests/functional/restjson1.spec.ts

@@ -51,8 +51,10 @@ import { Readable } from "stream";
 /**
  * Throws an expected exception that contains the serialized request.
  */
-class EXPECTED_REQUEST_SERIALIZATION_ERROR {
-  constructor(readonly request: HttpRequest) {}
+class EXPECTED_REQUEST_SERIALIZATION_ERROR extends Error {
+  constructor(readonly request: HttpRequest) {
+    super();
+  }
 }
 
 /**

protocol_tests/aws-restxml/tests/functional/restxml.spec.ts

@@ -60,8 +60,10 @@ import { Readable } from "stream";
 /**
  * Throws an expected exception that contains the serialized request.
  */
-class EXPECTED_REQUEST_SERIALIZATION_ERROR {
-  constructor(readonly request: HttpRequest) {}
+class EXPECTED_REQUEST_SERIALIZATION_ERROR extends Error {
+  constructor(readonly request: HttpRequest) {
+    super();
+  }
 }
 
 /**