diff --git a/packages/middleware-retry/src/configurations.spec.ts b/packages/middleware-retry/src/configurations.spec.ts index 2559251b6483..78205690a12a 100644 --- a/packages/middleware-retry/src/configurations.spec.ts +++ b/packages/middleware-retry/src/configurations.spec.ts @@ -1,46 +1,83 @@ import { resolveRetryConfig } from "./configurations"; import { StandardRetryStrategy } from "./defaultStrategy"; +jest.mock("./defaultStrategy", () => ({ + StandardRetryStrategy: jest.fn().mockReturnValue({}) +})); + describe("resolveRetryConfig", () => { + const maxAttemptsDefaultProvider = jest.fn(); + + afterEach(() => { + jest.clearAllMocks(); + }); + describe("maxAttempts", () => { - it("uses passed maxAttempts value if present", () => { - [1, 2, 3].forEach(maxAttempts => { - expect(resolveRetryConfig({ maxAttempts }).maxAttempts).toEqual( - maxAttempts - ); - }); + it("assigns maxAttempts value if present", async () => { + for (const maxAttempts of [1, 2, 3]) { + const output = await resolveRetryConfig({ + maxAttempts, + maxAttemptsDefaultProvider + }).maxAttempts(); + expect(output).toStrictEqual(maxAttempts.toString()); + expect(maxAttemptsDefaultProvider).not.toHaveBeenCalled(); + } }); - it("assigns default value of 3 if maxAttempts not passed", () => { - expect(resolveRetryConfig({}).maxAttempts).toEqual(3); + it("assigns maxAttemptsDefaultProvider if maxAttempts not present", () => { + const mockMaxAttempts = jest.fn(); + maxAttemptsDefaultProvider.mockReturnValueOnce(mockMaxAttempts); + + const input = { maxAttemptsDefaultProvider }; + expect(resolveRetryConfig(input).maxAttempts).toStrictEqual( + mockMaxAttempts + ); + + expect(maxAttemptsDefaultProvider).toHaveBeenCalledTimes(1); + expect(maxAttemptsDefaultProvider).toHaveBeenCalledWith(input); }); }); describe("retryStrategy", () => { - it("uses passed retryStrategy if present", () => { + it("passes retryStrategy if present", () => { const mockRetryStrategy = { - maxAttempts: 2, retry: jest.fn() }; const { retryStrategy } = resolveRetryConfig({ - retryStrategy: mockRetryStrategy + retryStrategy: mockRetryStrategy, + maxAttemptsDefaultProvider }); expect(retryStrategy).toEqual(mockRetryStrategy); }); describe("creates StandardRetryStrategy if retryStrategy not present", () => { - describe("uses maxAttempts if present", () => { - [1, 2, 3].forEach(maxAttempts => { - const { retryStrategy } = resolveRetryConfig({ maxAttempts }); - expect(retryStrategy).toBeInstanceOf(StandardRetryStrategy); - expect(retryStrategy.maxAttempts).toBe(maxAttempts); - }); + describe("passes maxAttempts if present", () => { + for (const maxAttempts of [1, 2, 3]) { + it(`when maxAttempts=${maxAttempts}`, async () => { + const { retryStrategy } = resolveRetryConfig({ + maxAttempts, + maxAttemptsDefaultProvider + }); + expect(retryStrategy).toBeInstanceOf(StandardRetryStrategy); + expect(StandardRetryStrategy as jest.Mock).toHaveBeenCalledTimes(1); + const output = await (StandardRetryStrategy as jest.Mock).mock.calls[0][0](); + expect(output).toStrictEqual(maxAttempts.toString()); + }); + } }); - it("uses default 3 if maxAttempts is not present", () => { - const { retryStrategy } = resolveRetryConfig({}); + it("passes maxAttemptsDefaultProvider if maxAttempts is not present", () => { + const mockMaxAttempts = jest.fn(); + maxAttemptsDefaultProvider.mockReturnValueOnce(mockMaxAttempts); + + const { retryStrategy } = resolveRetryConfig({ + maxAttemptsDefaultProvider + }); expect(retryStrategy).toBeInstanceOf(StandardRetryStrategy); - expect(retryStrategy.maxAttempts).toBe(3); + expect(StandardRetryStrategy as jest.Mock).toHaveBeenCalledTimes(1); + expect((StandardRetryStrategy as jest.Mock).mock.calls[0][0]).toEqual( + mockMaxAttempts + ); }); }); }); diff --git a/packages/middleware-retry/src/configurations.ts b/packages/middleware-retry/src/configurations.ts index a52a18b83420..1da76376b697 100644 --- a/packages/middleware-retry/src/configurations.ts +++ b/packages/middleware-retry/src/configurations.ts @@ -1,4 +1,4 @@ -import { RetryStrategy } from "@aws-sdk/types"; +import { RetryStrategy, Provider } from "@aws-sdk/types"; import { StandardRetryStrategy } from "./defaultStrategy"; export interface RetryInputConfig { @@ -12,18 +12,32 @@ export interface RetryInputConfig { retryStrategy?: RetryStrategy; } +interface PreviouslyResolved { + maxAttemptsDefaultProvider: (input: any) => Provider; +} export interface RetryResolvedConfig { - maxAttempts: number; + maxAttempts: Provider; retryStrategy: RetryStrategy; } export const resolveRetryConfig = ( - input: T & RetryInputConfig + input: T & PreviouslyResolved & RetryInputConfig ): T & RetryResolvedConfig => { - const maxAttempts = input.maxAttempts ?? 3; + const maxAttempts = + normalizeMaxAttempts(input.maxAttempts) ?? + input.maxAttemptsDefaultProvider(input as any); return { ...input, maxAttempts, retryStrategy: input.retryStrategy || new StandardRetryStrategy(maxAttempts) }; }; + +const normalizeMaxAttempts = ( + maxAttempts?: number +): Provider | undefined => { + if (maxAttempts) { + const promisified = Promise.resolve(maxAttempts.toString()); + return () => promisified; + } +}; diff --git a/packages/middleware-retry/src/defaultStrategy.spec.ts b/packages/middleware-retry/src/defaultStrategy.spec.ts index b7bd60575562..7c2207a6237a 100644 --- a/packages/middleware-retry/src/defaultStrategy.spec.ts +++ b/packages/middleware-retry/src/defaultStrategy.spec.ts @@ -55,7 +55,9 @@ describe("defaultStrategy", () => { output: { $metadata: {} } }); - const retryStrategy = new StandardRetryStrategy(maxAttempts); + const retryStrategy = new StandardRetryStrategy(() => + Promise.resolve(maxAttempts.toString()) + ); return retryStrategy.retry(next, { request: { headers: {} } } as any); }; @@ -66,7 +68,9 @@ describe("defaultStrategy", () => { const mockError = options?.mockError ?? new Error("mockError"); next = jest.fn().mockRejectedValue(mockError); - const retryStrategy = new StandardRetryStrategy(maxAttempts); + const retryStrategy = new StandardRetryStrategy(() => + Promise.resolve(maxAttempts.toString()) + ); try { await retryStrategy.retry(next, { request: { headers: {} } } as any); } catch (error) { @@ -90,7 +94,9 @@ describe("defaultStrategy", () => { .mockRejectedValueOnce(mockError) .mockResolvedValueOnce(mockResponse); - const retryStrategy = new StandardRetryStrategy(maxAttempts); + const retryStrategy = new StandardRetryStrategy(() => + Promise.resolve(maxAttempts.toString()) + ); return retryStrategy.retry(next, { request: { headers: {} } } as any); }; @@ -110,7 +116,9 @@ describe("defaultStrategy", () => { .mockRejectedValueOnce(mockError) .mockResolvedValueOnce(mockResponse); - const retryStrategy = new StandardRetryStrategy(maxAttempts); + const retryStrategy = new StandardRetryStrategy(() => + Promise.resolve(maxAttempts.toString()) + ); return retryStrategy.retry(next, { request: { headers: {} } } as any); }; @@ -118,63 +126,86 @@ describe("defaultStrategy", () => { jest.clearAllMocks(); }); - it("sets maxAttempts as class member variable", () => { - [1, 2, 3].forEach(maxAttempts => { - const retryStrategy = new StandardRetryStrategy(maxAttempts); - expect(retryStrategy.maxAttempts).toBe(maxAttempts); + it("sets maxAttemptsProvider as class member variable", () => { + ["1", "2", "3"].forEach(maxAttempts => { + const retryStrategy = new StandardRetryStrategy(() => + Promise.resolve(maxAttempts) + ); + expect(retryStrategy["maxAttemptsProvider"]()).resolves.toBe(maxAttempts); }); }); describe("retryDecider init", () => { it("sets defaultRetryDecider if options is undefined", () => { - const retryStrategy = new StandardRetryStrategy(maxAttempts); + const retryStrategy = new StandardRetryStrategy(() => + Promise.resolve(maxAttempts.toString()) + ); expect(retryStrategy["retryDecider"]).toBe(defaultRetryDecider); }); it("sets defaultRetryDecider if options.retryDecider is undefined", () => { - const retryStrategy = new StandardRetryStrategy(maxAttempts, {}); + const retryStrategy = new StandardRetryStrategy( + () => Promise.resolve(maxAttempts.toString()), + {} + ); expect(retryStrategy["retryDecider"]).toBe(defaultRetryDecider); }); it("sets options.retryDecider if defined", () => { const retryDecider = jest.fn(); - const retryStrategy = new StandardRetryStrategy(maxAttempts, { - retryDecider - }); + const retryStrategy = new StandardRetryStrategy( + () => Promise.resolve(maxAttempts.toString()), + { + retryDecider + } + ); expect(retryStrategy["retryDecider"]).toBe(retryDecider); }); }); describe("delayDecider init", () => { it("sets defaultDelayDecider if options is undefined", () => { - const retryStrategy = new StandardRetryStrategy(maxAttempts); + const retryStrategy = new StandardRetryStrategy(() => + Promise.resolve(maxAttempts.toString()) + ); expect(retryStrategy["delayDecider"]).toBe(defaultDelayDecider); }); it("sets defaultDelayDecider if options.delayDecider undefined", () => { - const retryStrategy = new StandardRetryStrategy(maxAttempts, {}); + const retryStrategy = new StandardRetryStrategy( + () => Promise.resolve(maxAttempts.toString()), + {} + ); expect(retryStrategy["delayDecider"]).toBe(defaultDelayDecider); }); it("sets options.delayDecider if defined", () => { const delayDecider = jest.fn(); - const retryStrategy = new StandardRetryStrategy(maxAttempts, { - delayDecider - }); + const retryStrategy = new StandardRetryStrategy( + () => Promise.resolve(maxAttempts.toString()), + { + delayDecider + } + ); expect(retryStrategy["delayDecider"]).toBe(delayDecider); }); }); describe("retryQuota init", () => { it("sets getDefaultRetryQuota if options is undefined", () => { - const retryStrategy = new StandardRetryStrategy(maxAttempts); + const retryStrategy = new StandardRetryStrategy(() => + Promise.resolve(maxAttempts.toString()) + ); expect(retryStrategy["retryQuota"]).toBe( getDefaultRetryQuota(INITIAL_RETRY_TOKENS) ); }); it("sets getDefaultRetryQuota if options.delayDecider undefined", () => { - const retryStrategy = new StandardRetryStrategy(maxAttempts, {}); + const retryStrategy = new StandardRetryStrategy( + () => Promise.resolve(maxAttempts.toString()), + {} + ); expect(retryStrategy["retryQuota"]).toBe( getDefaultRetryQuota(INITIAL_RETRY_TOKENS) ); @@ -182,9 +213,12 @@ describe("defaultStrategy", () => { it("sets options.retryQuota if defined", () => { const retryQuota = {} as RetryQuota; - const retryStrategy = new StandardRetryStrategy(maxAttempts, { - retryQuota - }); + const retryStrategy = new StandardRetryStrategy( + () => Promise.resolve(maxAttempts.toString()), + { + retryQuota + } + ); expect(retryStrategy["retryQuota"]).toBe(retryQuota); }); }); @@ -483,7 +517,9 @@ describe("defaultStrategy", () => { output: { $metadata: {} } }); - const retryStrategy = new StandardRetryStrategy(maxAttempts); + const retryStrategy = new StandardRetryStrategy(() => + Promise.resolve(maxAttempts.toString()) + ); await retryStrategy.retry(next, { request: { headers: {} } } as any); await retryStrategy.retry(next, { request: { headers: {} } } as any); @@ -565,7 +601,9 @@ describe("defaultStrategy", () => { throw mockError; }); - const retryStrategy = new StandardRetryStrategy(maxAttempts); + const retryStrategy = new StandardRetryStrategy(() => + Promise.resolve(maxAttempts.toString()) + ); try { await retryStrategy.retry(next, { request: { headers: {} } } as any); } catch (error) { @@ -577,4 +615,54 @@ describe("defaultStrategy", () => { ((isInstance as unknown) as jest.Mock).mockReturnValue(false); }); }); + + describe("defaults maxAttempts to 3", () => { + 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}` + ); + return Promise.resolve({ + response: "mockResponse", + output: { $metadata: {} } + }); + }); + + const retryStrategy = new StandardRetryStrategy(() => + Promise.reject("ERROR") + ); + await retryStrategy.retry(next, { request: { headers: {} } } as any); + + expect(next).toHaveBeenCalledTimes(1); + ((isInstance as unknown) as jest.Mock).mockReturnValue(false); + }); + + 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}` + ); + return Promise.resolve({ + response: "mockResponse", + output: { $metadata: {} } + }); + }); + + const retryStrategy = new StandardRetryStrategy(() => + Promise.resolve("not-a-number") + ); + await retryStrategy.retry(next, { request: { headers: {} } } as any); + + expect(next).toHaveBeenCalledTimes(1); + ((isInstance as unknown) as jest.Mock).mockReturnValue(false); + }); + }); }); diff --git a/packages/middleware-retry/src/defaultStrategy.ts b/packages/middleware-retry/src/defaultStrategy.ts index 3271e6f31183..e2ace45a4dc7 100644 --- a/packages/middleware-retry/src/defaultStrategy.ts +++ b/packages/middleware-retry/src/defaultStrategy.ts @@ -11,7 +11,8 @@ import { FinalizeHandler, MetadataBearer, FinalizeHandlerArguments, - RetryStrategy + RetryStrategy, + Provider } from "@aws-sdk/types"; import { getDefaultRetryQuota } from "./defaultRetryQuota"; import { HttpRequest } from "@aws-sdk/protocol-http"; @@ -73,7 +74,7 @@ export class StandardRetryStrategy implements RetryStrategy { private retryQuota: RetryQuota; constructor( - public readonly maxAttempts: number, + private readonly maxAttemptsProvider: Provider, options?: StandardRetryStrategyOptions ) { this.retryDecider = options?.retryDecider ?? defaultRetryDecider; @@ -82,14 +83,25 @@ export class StandardRetryStrategy implements RetryStrategy { options?.retryQuota ?? getDefaultRetryQuota(INITIAL_RETRY_TOKENS); } - private shouldRetry(error: SdkError, attempts: number) { + private shouldRetry(error: SdkError, attempts: number, maxAttempts: number) { return ( - attempts < this.maxAttempts && + attempts < maxAttempts && this.retryDecider(error) && this.retryQuota.hasRetryTokens(error) ); } + private async getMaxAttempts() { + let maxAttemptsStr; + try { + maxAttemptsStr = await this.maxAttemptsProvider(); + } catch (error) { + maxAttemptsStr = "3"; + } + const maxAttempts = parseInt(maxAttemptsStr); + return Number.isNaN(maxAttempts) ? 3 : maxAttempts; + } + async retry( next: FinalizeHandler, args: FinalizeHandlerArguments @@ -98,6 +110,8 @@ export class StandardRetryStrategy implements RetryStrategy { let attempts = 0; let totalDelay = 0; + const maxAttempts = await this.getMaxAttempts(); + const { request } = args; if (HttpRequest.isInstance(request)) { request.headers["amz-sdk-invocation-id"] = v4(); @@ -106,9 +120,9 @@ export class StandardRetryStrategy implements RetryStrategy { while (true) { try { if (HttpRequest.isInstance(request)) { - request.headers["amz-sdk-request"] = `attempt=${attempts + 1}; max=${ - this.maxAttempts - }`; + request.headers["amz-sdk-request"] = `attempt=${ + attempts + 1 + }; max=${maxAttempts}`; } const { response, output } = await next(args); @@ -119,7 +133,7 @@ export class StandardRetryStrategy implements RetryStrategy { return { response, output }; } catch (err) { attempts++; - if (this.shouldRetry(err as SdkError, attempts)) { + if (this.shouldRetry(err as SdkError, attempts, maxAttempts)) { retryTokenAmount = this.retryQuota.retrieveRetryTokens(err); const delay = this.delayDecider( isThrottlingError(err) diff --git a/packages/middleware-retry/src/retryMiddleware.spec.ts b/packages/middleware-retry/src/retryMiddleware.spec.ts index 08a49b3708e5..ac583b279e07 100644 --- a/packages/middleware-retry/src/retryMiddleware.spec.ts +++ b/packages/middleware-retry/src/retryMiddleware.spec.ts @@ -18,11 +18,11 @@ describe("getRetryPlugin", () => { jest.clearAllMocks(); }); - describe("adds retryMiddleware if maxAttempts > 1", () => { - [2, 3, 4].forEach(maxAttempts => { + describe("adds retryMiddleware", () => { + [1, 2, 3].forEach(maxAttempts => { it(`when maxAttempts=${maxAttempts}`, () => { getRetryPlugin({ - maxAttempts, + maxAttempts: () => Promise.resolve(maxAttempts.toString()), retryStrategy: {} as RetryStrategy }).applyToStack( (mockClientStack as unknown) as MiddlewareStack @@ -34,20 +34,6 @@ describe("getRetryPlugin", () => { }); }); }); - - describe("skips adding retryMiddleware if maxAttempts <= 1", () => { - [0, 1].forEach(maxAttempts => { - it(`when maxAttempts=${maxAttempts}`, () => { - getRetryPlugin({ - maxAttempts, - retryStrategy: {} as RetryStrategy - }).applyToStack( - (mockClientStack as unknown) as MiddlewareStack - ); - expect(mockClientStack.add).toHaveBeenCalledTimes(0); - }); - }); - }); }); describe("retryMiddleware", () => { @@ -67,7 +53,7 @@ describe("retryMiddleware", () => { }; await retryMiddleware({ - maxAttempts, + maxAttempts: () => Promise.resolve(maxAttempts.toString()), retryStrategy: mockRetryStrategy })(next)(args as FinalizeHandlerArguments); expect(mockRetryStrategy.retry).toHaveBeenCalledTimes(1); diff --git a/packages/middleware-retry/src/retryMiddleware.ts b/packages/middleware-retry/src/retryMiddleware.ts index bbd72ec957f4..fcd829463078 100644 --- a/packages/middleware-retry/src/retryMiddleware.ts +++ b/packages/middleware-retry/src/retryMiddleware.ts @@ -30,8 +30,6 @@ export const getRetryPlugin = ( options: RetryResolvedConfig ): Pluggable => ({ applyToStack: clientStack => { - if (options.maxAttempts > 1) { - clientStack.add(retryMiddleware(options), retryMiddlewareOptions); - } + clientStack.add(retryMiddleware(options), retryMiddlewareOptions); } }); diff --git a/packages/types/src/util.ts b/packages/types/src/util.ts index 83512cdea706..9d0136f5ff59 100644 --- a/packages/types/src/util.ts +++ b/packages/types/src/util.ts @@ -57,11 +57,6 @@ export interface BodyLengthCalculator { * Interface that specifies the retry behavior */ export interface RetryStrategy { - /** - * the maximum number of times requests that encounter potentially - * transient failures should be retried - */ - maxAttempts: number; /** * the retry behavior the will invoke the next handler and handle the retry accordingly. * This function should also update the $metadata from the response accordingly.