From 180e32e473ce822d86d653bb27f6f03a4dbf7466 Mon Sep 17 00:00:00 2001 From: Trivikram Kamat <16024985+trivikr@users.noreply.github.com> Date: Wed, 8 Jul 2020 02:39:12 +0000 Subject: [PATCH 1/4] fix: use fromStatic to return default config values --- .../src/defaultProvider.ts | 27 +++++++++++++++---- 1 file changed, 22 insertions(+), 5 deletions(-) diff --git a/packages/retry-config-provider/src/defaultProvider.ts b/packages/retry-config-provider/src/defaultProvider.ts index 8bb6370374fd..4b39b13228ad 100644 --- a/packages/retry-config-provider/src/defaultProvider.ts +++ b/packages/retry-config-provider/src/defaultProvider.ts @@ -3,30 +3,47 @@ import { SharedConfigInit, fromSharedConfigFiles } from "./fromSharedConfigFiles"; -import { chain, memoize } from "@aws-sdk/property-provider"; +import { chain, memoize, fromStatic } from "@aws-sdk/property-provider"; import { Provider } from "@aws-sdk/types"; export const ENV_MAX_ATTEMPTS = "AWS_MAX_ATTEMPTS"; export const CONFIG_MAX_ATTEMPTS = "max_attempts"; +export const DEFAULT_MAX_ATTEMPTS = "3"; export const ENV_RETRY_MODE = "AWS_RETRY_MODE"; export const CONFIG_RETRY_MODE = "retry_mode"; +export const DEFAULT_RETRY_MODE = "standard"; const defaultProvider = ( configuration: SharedConfigInit = {}, envVarName: string, - configKey: string + configKey: string, + defaultValue: string ): Provider => memoize( - chain(fromEnv(envVarName), fromSharedConfigFiles(configuration, configKey)) + chain( + fromEnv(envVarName), + fromSharedConfigFiles(configuration, configKey), + fromStatic(defaultValue) + ) ); export const maxAttemptsProvider = ( configuration: SharedConfigInit = {} ): Provider => - defaultProvider(configuration, ENV_MAX_ATTEMPTS, CONFIG_MAX_ATTEMPTS); + defaultProvider( + configuration, + ENV_MAX_ATTEMPTS, + CONFIG_MAX_ATTEMPTS, + DEFAULT_MAX_ATTEMPTS + ); export const retryModeProvider = ( configuration: SharedConfigInit = {} ): Provider => - defaultProvider(configuration, ENV_RETRY_MODE, CONFIG_RETRY_MODE); + defaultProvider( + configuration, + ENV_RETRY_MODE, + CONFIG_RETRY_MODE, + DEFAULT_RETRY_MODE + ); From f5acc8bfd288044b337e8025f338251c404a6641 Mon Sep 17 00:00:00 2001 From: Trivikram Kamat <16024985+trivikr@users.noreply.github.com> Date: Wed, 8 Jul 2020 02:46:01 +0000 Subject: [PATCH 2/4] test: update defaultProvider.spec.ts --- .../src/defaultProvider.spec.ts | 48 +++++++++++-------- 1 file changed, 29 insertions(+), 19 deletions(-) diff --git a/packages/retry-config-provider/src/defaultProvider.spec.ts b/packages/retry-config-provider/src/defaultProvider.spec.ts index e882ed462fa9..41a96c9283b2 100644 --- a/packages/retry-config-provider/src/defaultProvider.spec.ts +++ b/packages/retry-config-provider/src/defaultProvider.spec.ts @@ -3,28 +3,21 @@ import { fromSharedConfigFiles, SharedConfigInit } from "./fromSharedConfigFiles"; -import { chain, memoize } from "@aws-sdk/property-provider"; +import { chain, memoize, fromStatic } from "@aws-sdk/property-provider"; import { maxAttemptsProvider, ENV_MAX_ATTEMPTS, CONFIG_MAX_ATTEMPTS, + DEFAULT_MAX_ATTEMPTS, retryModeProvider, ENV_RETRY_MODE, - CONFIG_RETRY_MODE + CONFIG_RETRY_MODE, + DEFAULT_RETRY_MODE } from "./defaultProvider"; -jest.mock("./fromEnv", () => ({ - fromEnv: jest.fn() -})); - -jest.mock("./fromSharedConfigFiles", () => ({ - fromSharedConfigFiles: jest.fn() -})); - -jest.mock("@aws-sdk/property-provider", () => ({ - chain: jest.fn(), - memoize: jest.fn() -})); +jest.mock("./fromEnv"); +jest.mock("./fromSharedConfigFiles"); +jest.mock("@aws-sdk/property-provider"); describe("defaultProvider", () => { const configuration: SharedConfigInit = { @@ -38,9 +31,10 @@ describe("defaultProvider", () => { const testProvider = ( providerFunc: Function, envVarName: string, - configKey: string + configKey: string, + defaultValue: string ) => { - it("passes fromEnv() and fromSharedConfigFiles() to chain", () => { + it("passes fromEnv(), fromSharedConfigFiles() and fromStatic() to chain", () => { const mockFromEnvReturn = "mockFromEnvReturn"; (fromEnv as jest.Mock).mockReturnValueOnce(mockFromEnvReturn); @@ -49,6 +43,9 @@ describe("defaultProvider", () => { mockFromSharedConfigFilesReturn ); + const mockFromStatic = "mockFromStatic"; + (fromStatic as jest.Mock).mockReturnValueOnce(mockFromStatic); + providerFunc(configuration); expect(fromEnv).toHaveBeenCalledTimes(1); @@ -58,11 +55,14 @@ describe("defaultProvider", () => { configuration, configKey ); + expect(fromStatic).toHaveBeenCalledTimes(1); + expect(fromStatic).toHaveBeenCalledWith(defaultValue); expect(chain).toHaveBeenCalledTimes(1); expect(chain).toHaveBeenCalledWith( mockFromEnvReturn, - mockFromSharedConfigFilesReturn + mockFromSharedConfigFilesReturn, + mockFromStatic ); }); @@ -86,10 +86,20 @@ describe("defaultProvider", () => { }; describe("maxAttemptsProvider", () => { - testProvider(maxAttemptsProvider, ENV_MAX_ATTEMPTS, CONFIG_MAX_ATTEMPTS); + testProvider( + maxAttemptsProvider, + ENV_MAX_ATTEMPTS, + CONFIG_MAX_ATTEMPTS, + DEFAULT_MAX_ATTEMPTS + ); }); describe("retryModeProvider", () => { - testProvider(retryModeProvider, ENV_RETRY_MODE, CONFIG_RETRY_MODE); + testProvider( + retryModeProvider, + ENV_RETRY_MODE, + CONFIG_RETRY_MODE, + DEFAULT_RETRY_MODE + ); }); }); From 6bcd8d998ff9787ba69759436c189b06383b42c9 Mon Sep 17 00:00:00 2001 From: Trivikram Kamat <16024985+trivikr@users.noreply.github.com> Date: Wed, 8 Jul 2020 02:58:39 +0000 Subject: [PATCH 3/4] chore: use DEFAULT_MAX_ATTEMPTS in defaultStrategy --- packages/middleware-retry/package.json | 1 + packages/middleware-retry/src/defaultStrategy.spec.ts | 9 ++++----- packages/middleware-retry/src/defaultStrategy.ts | 9 ++++++--- 3 files changed, 11 insertions(+), 8 deletions(-) diff --git a/packages/middleware-retry/package.json b/packages/middleware-retry/package.json index ff66434d1d8a..55c46070a944 100644 --- a/packages/middleware-retry/package.json +++ b/packages/middleware-retry/package.json @@ -18,6 +18,7 @@ "license": "Apache-2.0", "dependencies": { "@aws-sdk/protocol-http": "1.0.0-gamma.1", + "@aws-sdk/retry-config-provider": "1.0.0-gamma.0", "@aws-sdk/service-error-classification": "1.0.0-gamma.1", "@aws-sdk/types": "1.0.0-gamma.1", "react-native-get-random-values": "^1.4.0", diff --git a/packages/middleware-retry/src/defaultStrategy.spec.ts b/packages/middleware-retry/src/defaultStrategy.spec.ts index 7c2207a6237a..7c880b1c4fb4 100644 --- a/packages/middleware-retry/src/defaultStrategy.spec.ts +++ b/packages/middleware-retry/src/defaultStrategy.spec.ts @@ -10,6 +10,7 @@ import { StandardRetryStrategy, RetryQuota } from "./defaultStrategy"; import { getDefaultRetryQuota } from "./defaultRetryQuota"; import { HttpRequest } from "@aws-sdk/protocol-http"; import { v4 } from "uuid"; +import { DEFAULT_MAX_ATTEMPTS } from "@aws-sdk/retry-config-provider"; jest.mock("@aws-sdk/service-error-classification", () => ({ isThrottlingError: jest.fn().mockReturnValue(true) @@ -616,15 +617,14 @@ describe("defaultStrategy", () => { }); }); - describe("defaults maxAttempts to 3", () => { + describe("defaults maxAttempts to DEFAULT_MAX_ATTEMPTS", () => { it("when maxAttemptsProvider throws error", async () => { - const maxAttempts = 3; const { isInstance } = HttpRequest; ((isInstance as unknown) as jest.Mock).mockReturnValue(true); next = jest.fn(args => { expect(args.request.headers["amz-sdk-request"]).toBe( - `attempt=1; max=${maxAttempts}` + `attempt=1; max=${DEFAULT_MAX_ATTEMPTS}` ); return Promise.resolve({ response: "mockResponse", @@ -642,13 +642,12 @@ describe("defaultStrategy", () => { }); it("when parseInt fails on maxAttemptsProvider", async () => { - const maxAttempts = 3; const { isInstance } = HttpRequest; ((isInstance as unknown) as jest.Mock).mockReturnValue(true); next = jest.fn(args => { expect(args.request.headers["amz-sdk-request"]).toBe( - `attempt=1; max=${maxAttempts}` + `attempt=1; max=${DEFAULT_MAX_ATTEMPTS}` ); return Promise.resolve({ response: "mockResponse", diff --git a/packages/middleware-retry/src/defaultStrategy.ts b/packages/middleware-retry/src/defaultStrategy.ts index e2ace45a4dc7..7ed7d276147e 100644 --- a/packages/middleware-retry/src/defaultStrategy.ts +++ b/packages/middleware-retry/src/defaultStrategy.ts @@ -17,6 +17,7 @@ import { import { getDefaultRetryQuota } from "./defaultRetryQuota"; import { HttpRequest } from "@aws-sdk/protocol-http"; import { v4 } from "uuid"; +import { DEFAULT_MAX_ATTEMPTS } from "@aws-sdk/retry-config-provider"; /** * Determines whether an error is retryable based on the number of retries @@ -92,14 +93,16 @@ export class StandardRetryStrategy implements RetryStrategy { } private async getMaxAttempts() { - let maxAttemptsStr; + let maxAttemptsStr: string; try { maxAttemptsStr = await this.maxAttemptsProvider(); } catch (error) { - maxAttemptsStr = "3"; + maxAttemptsStr = DEFAULT_MAX_ATTEMPTS; } const maxAttempts = parseInt(maxAttemptsStr); - return Number.isNaN(maxAttempts) ? 3 : maxAttempts; + return Number.isNaN(maxAttempts) + ? parseInt(DEFAULT_MAX_ATTEMPTS) + : maxAttempts; } async retry( From 00a293b54077ec6e3dfd855145f391d407e62b78 Mon Sep 17 00:00:00 2001 From: Trivikram Kamat <16024985+trivikr@users.noreply.github.com> Date: Wed, 8 Jul 2020 03:06:32 +0000 Subject: [PATCH 4/4] chore: convert second param of defaultProvider to options --- .../src/defaultProvider.ts | 34 +++++++++++-------- 1 file changed, 19 insertions(+), 15 deletions(-) diff --git a/packages/retry-config-provider/src/defaultProvider.ts b/packages/retry-config-provider/src/defaultProvider.ts index 4b39b13228ad..6597494db6b8 100644 --- a/packages/retry-config-provider/src/defaultProvider.ts +++ b/packages/retry-config-provider/src/defaultProvider.ts @@ -16,9 +16,15 @@ export const DEFAULT_RETRY_MODE = "standard"; const defaultProvider = ( configuration: SharedConfigInit = {}, - envVarName: string, - configKey: string, - defaultValue: string + { + envVarName, + configKey, + defaultValue + }: { + envVarName: string; + configKey: string; + defaultValue: string; + } ): Provider => memoize( chain( @@ -31,19 +37,17 @@ const defaultProvider = ( export const maxAttemptsProvider = ( configuration: SharedConfigInit = {} ): Provider => - defaultProvider( - configuration, - ENV_MAX_ATTEMPTS, - CONFIG_MAX_ATTEMPTS, - DEFAULT_MAX_ATTEMPTS - ); + defaultProvider(configuration, { + envVarName: ENV_MAX_ATTEMPTS, + configKey: CONFIG_MAX_ATTEMPTS, + defaultValue: DEFAULT_MAX_ATTEMPTS + }); export const retryModeProvider = ( configuration: SharedConfigInit = {} ): Provider => - defaultProvider( - configuration, - ENV_RETRY_MODE, - CONFIG_RETRY_MODE, - DEFAULT_RETRY_MODE - ); + defaultProvider(configuration, { + envVarName: ENV_RETRY_MODE, + configKey: CONFIG_RETRY_MODE, + defaultValue: DEFAULT_RETRY_MODE + });