diff --git a/packages/middleware-retry/src/types.ts b/packages/middleware-retry/src/types.ts index e6d76b951b7f..445d64db18c8 100644 --- a/packages/middleware-retry/src/types.ts +++ b/packages/middleware-retry/src/types.ts @@ -4,7 +4,7 @@ import { SdkError } from "@aws-sdk/types"; * Determines whether an error is retryable based on the number of retries * already attempted, the HTTP status code, and the error received (if any). * - * @param error The error encountered. + * @param error - The error encountered. */ export interface RetryDecider { (error: SdkError): boolean; @@ -13,8 +13,8 @@ export interface RetryDecider { /** * Determines the number of milliseconds to wait before retrying an action. * - * @param delayBase The base delay (in milliseconds). - * @param attempts The number of times the action has already been tried. + * @param delayBase - The base delay (in milliseconds). + * @param attempts - The number of times the action has already been tried. */ export interface DelayDecider { (delayBase: number, attempts: number): number; diff --git a/packages/types/src/retry.ts b/packages/types/src/retry.ts index 7c856c4afdae..ee3ba0b72708 100644 --- a/packages/types/src/retry.ts +++ b/packages/types/src/retry.ts @@ -95,26 +95,9 @@ export interface RetryToken { */ export interface StandardRetryToken extends RetryToken { /** - * @returns wheather token has remaining tokens. + * @returns the cost of the last retry attempt. */ - hasRetryTokens(errorType: RetryErrorType): boolean; - - /** - * @returns the number of available tokens. - */ - getRetryTokenCount(errorInfo: RetryErrorInfo): number; - - /** - * @returns the cost of the last retry attemp. - */ - getLastRetryCost(): number | undefined; - - /** - * Releases a number of tokens. - * - * @param amount - of tokens to release. - */ - releaseRetryTokens(amount?: number): void; + getRetryCost(): number | undefined; } /** diff --git a/packages/util-retry/src/AdaptiveRetryStrategy.spec.ts b/packages/util-retry/src/AdaptiveRetryStrategy.spec.ts index b28ad91bdbdd..27220d4ca084 100644 --- a/packages/util-retry/src/AdaptiveRetryStrategy.spec.ts +++ b/packages/util-retry/src/AdaptiveRetryStrategy.spec.ts @@ -1,4 +1,4 @@ -import { RetryErrorInfo, RetryErrorType, StandardRetryToken } from "@aws-sdk/types"; +import { RetryErrorInfo, StandardRetryToken } from "@aws-sdk/types"; import { AdaptiveRetryStrategy } from "./AdaptiveRetryStrategy"; import { RETRY_MODES } from "./config"; @@ -17,10 +17,7 @@ describe(AdaptiveRetryStrategy.name, () => { updateClientSendingRate: jest.fn(), }; const mockRetryToken: StandardRetryToken = { - hasRetryTokens: (errorType: RetryErrorType) => true, - getLastRetryCost: () => 1, - getRetryTokenCount: (errorInfo: RetryErrorInfo) => 1, - releaseRetryTokens: (amount: number) => {}, + getRetryCost: () => 1, getRetryCount: () => 1, getRetryDelay: () => 1, }; diff --git a/packages/util-retry/src/ConfiguredRetryStrategy.spec.ts b/packages/util-retry/src/ConfiguredRetryStrategy.spec.ts index f080d7c7dc31..4d1638e35a77 100644 --- a/packages/util-retry/src/ConfiguredRetryStrategy.spec.ts +++ b/packages/util-retry/src/ConfiguredRetryStrategy.spec.ts @@ -11,7 +11,7 @@ describe(ConfiguredRetryStrategy.name, () => { errorType: "TRANSIENT", }); - expect(retryToken.getRetryCount()).toBe(4); - expect(retryToken.getRetryDelay()).toBe(4000); + expect(retryToken.getRetryCount()).toBe(5); + expect(retryToken.getRetryDelay()).toBe(5000); }); }); diff --git a/packages/util-retry/src/StandardRetryStrategy.spec.ts b/packages/util-retry/src/StandardRetryStrategy.spec.ts index 409ed71c6ea6..cd6bd3e153d4 100644 --- a/packages/util-retry/src/StandardRetryStrategy.spec.ts +++ b/packages/util-retry/src/StandardRetryStrategy.spec.ts @@ -2,7 +2,7 @@ import { RetryErrorInfo, RetryErrorType } from "@aws-sdk/types"; import { RETRY_MODES } from "./config"; import { DEFAULT_RETRY_DELAY_BASE, INITIAL_RETRY_TOKENS } from "./constants"; -import { getDefaultRetryToken } from "./defaultRetryToken"; +import { createDefaultRetryToken } from "./defaultRetryToken"; import { StandardRetryStrategy } from "./StandardRetryStrategy"; jest.mock("./defaultRetryToken"); @@ -18,7 +18,7 @@ describe(StandardRetryStrategy.name, () => { const errorInfo = { errorType: "TRANSIENT" } as RetryErrorInfo; beforeEach(() => { - (getDefaultRetryToken as jest.Mock).mockReturnValue(mockRetryToken); + (createDefaultRetryToken as jest.Mock).mockReturnValue(mockRetryToken); }); afterEach(() => { @@ -37,40 +37,32 @@ describe(StandardRetryStrategy.name, () => { expect(retryStrategy.mode).toStrictEqual(RETRY_MODES.STANDARD); }); - describe("retryToken init", () => { - it("sets retryToken", () => { - const retryStrategy = new StandardRetryStrategy(() => Promise.resolve(maxAttempts)); - expect(retryStrategy["retryToken"]).toBe(getDefaultRetryToken(INITIAL_RETRY_TOKENS, DEFAULT_RETRY_DELAY_BASE)); - }); - }); - describe("acquireInitialRetryToken", () => { it("returns default retryToken", async () => { const retryStrategy = new StandardRetryStrategy(() => Promise.resolve(maxAttempts)); const retryToken = await retryStrategy.acquireInitialRetryToken(retryTokenScope); - expect(retryToken).toEqual(getDefaultRetryToken(INITIAL_RETRY_TOKENS, DEFAULT_RETRY_DELAY_BASE)); + expect(retryToken).toEqual( + createDefaultRetryToken({ + retryDelay: DEFAULT_RETRY_DELAY_BASE, + retryCount: 0, + }) + ); }); }); describe("refreshRetryTokenForRetry", () => { it("refreshes the token", async () => { - const getRetryTokenCount = jest.fn().mockReturnValue(1); const getRetryCount = jest.fn().mockReturnValue(0); const hasRetryTokens = jest.fn().mockReturnValue(true); const mockRetryToken = { getRetryCount, - getRetryTokenCount, hasRetryTokens, }; - (getDefaultRetryToken as jest.Mock).mockReturnValue(mockRetryToken); + (createDefaultRetryToken as jest.Mock).mockReturnValue(mockRetryToken); const retryStrategy = new StandardRetryStrategy(() => Promise.resolve(maxAttempts)); const token = await retryStrategy.acquireInitialRetryToken(retryTokenScope); - const refreshedToken = await retryStrategy.refreshRetryTokenForRetry(token, errorInfo); - expect(getRetryTokenCount).toHaveBeenCalledTimes(1); - expect(getRetryTokenCount).toHaveBeenCalledWith(errorInfo); - expect(getRetryCount).toHaveBeenCalledTimes(1); - expect(hasRetryTokens).toHaveBeenCalledTimes(1); - expect(hasRetryTokens).toHaveBeenCalledWith(errorInfo.errorType); + await retryStrategy.refreshRetryTokenForRetry(token, errorInfo); + expect(getRetryCount).toHaveBeenCalledTimes(3); }); it("throws when attempts exceeds maxAttempts", async () => { @@ -78,7 +70,7 @@ describe(StandardRetryStrategy.name, () => { getRetryCount: () => 2, getRetryTokenCount: (errorInfo: any) => 1, }; - (getDefaultRetryToken as jest.Mock).mockReturnValue(mockRetryToken); + (createDefaultRetryToken as jest.Mock).mockReturnValue(mockRetryToken); const retryStrategy = new StandardRetryStrategy(() => Promise.resolve(1)); const token = await retryStrategy.acquireInitialRetryToken(retryTokenScope); try { @@ -93,7 +85,7 @@ describe(StandardRetryStrategy.name, () => { getRetryCount: () => 5, getRetryTokenCount: (errorInfo: any) => 1, }; - (getDefaultRetryToken as jest.Mock).mockReturnValue(mockRetryToken); + (createDefaultRetryToken as jest.Mock).mockReturnValue(mockRetryToken); const retryStrategy = new StandardRetryStrategy(() => Promise.resolve(5)); const token = await retryStrategy.acquireInitialRetryToken(retryTokenScope); try { @@ -109,7 +101,7 @@ describe(StandardRetryStrategy.name, () => { getRetryTokenCount: (errorInfo: any) => 1, hasRetryTokens: (errorType: RetryErrorType) => false, }; - (getDefaultRetryToken as jest.Mock).mockReturnValue(mockRetryToken); + (createDefaultRetryToken as jest.Mock).mockReturnValue(mockRetryToken); const retryStrategy = new StandardRetryStrategy(() => Promise.resolve(maxAttempts)); const token = await retryStrategy.acquireInitialRetryToken(retryTokenScope); try { @@ -125,7 +117,7 @@ describe(StandardRetryStrategy.name, () => { getRetryTokenCount: (errorInfo: any) => 1, hasRetryTokens: (errorType: RetryErrorType) => true, }; - (getDefaultRetryToken as jest.Mock).mockReturnValue(mockRetryToken); + (createDefaultRetryToken as jest.Mock).mockReturnValue(mockRetryToken); const retryStrategy = new StandardRetryStrategy(() => Promise.resolve(maxAttempts)); const token = await retryStrategy.acquireInitialRetryToken(retryTokenScope); const errorInfo = { @@ -137,24 +129,5 @@ describe(StandardRetryStrategy.name, () => { expect(error).toStrictEqual(noRetryTokenAvailableError); } }); - - describe("recordSuccess", () => { - it("releases tokens", async () => { - const retryCost = 1; - const releaseRetryTokens = jest.fn(); - const getLastRetryCost = jest.fn().mockReturnValue(retryCost); - const mockRetryToken = { - releaseRetryTokens, - getLastRetryCost, - }; - (getDefaultRetryToken as jest.Mock).mockReturnValue(mockRetryToken); - const retryStrategy = new StandardRetryStrategy(() => Promise.resolve(maxAttempts)); - const token = await retryStrategy.acquireInitialRetryToken(retryTokenScope); - retryStrategy.recordSuccess(token); - expect(releaseRetryTokens).toHaveBeenCalledTimes(1); - expect(releaseRetryTokens).toHaveBeenCalledWith(retryCost); - expect(getLastRetryCost).toHaveBeenCalledTimes(1); - }); - }); }); }); diff --git a/packages/util-retry/src/StandardRetryStrategy.ts b/packages/util-retry/src/StandardRetryStrategy.ts index 1e15cacf87ca..05140f3462f7 100644 --- a/packages/util-retry/src/StandardRetryStrategy.ts +++ b/packages/util-retry/src/StandardRetryStrategy.ts @@ -1,47 +1,82 @@ import { Provider, RetryErrorInfo, RetryErrorType, RetryStrategyV2, StandardRetryToken } from "@aws-sdk/types"; import { DEFAULT_MAX_ATTEMPTS, RETRY_MODES } from "./config"; -import { DEFAULT_RETRY_DELAY_BASE, INITIAL_RETRY_TOKENS } from "./constants"; -import { getDefaultRetryToken } from "./defaultRetryToken"; +import { + DEFAULT_RETRY_DELAY_BASE, + INITIAL_RETRY_TOKENS, + NO_RETRY_INCREMENT, + RETRY_COST, + THROTTLING_RETRY_DELAY_BASE, + TIMEOUT_RETRY_COST, +} from "./constants"; +import { getDefaultRetryBackoffStrategy } from "./defaultRetryBackoffStrategy"; +import { createDefaultRetryToken } from "./defaultRetryToken"; /** * @public */ export class StandardRetryStrategy implements RetryStrategyV2 { public readonly mode: string = RETRY_MODES.STANDARD; - private retryToken: StandardRetryToken; + private capacity: number = INITIAL_RETRY_TOKENS; + private readonly retryBackoffStrategy = getDefaultRetryBackoffStrategy(); private readonly maxAttemptsProvider: Provider; constructor(maxAttempts: number); constructor(maxAttemptsProvider: Provider); constructor(private readonly maxAttempts: number | Provider) { - this.retryToken = getDefaultRetryToken(INITIAL_RETRY_TOKENS, DEFAULT_RETRY_DELAY_BASE); this.maxAttemptsProvider = typeof maxAttempts === "function" ? maxAttempts : async () => maxAttempts; } public async acquireInitialRetryToken(retryTokenScope: string): Promise { - return this.retryToken; + return createDefaultRetryToken({ + retryDelay: DEFAULT_RETRY_DELAY_BASE, + retryCount: 0, + }); } public async refreshRetryTokenForRetry( - tokenToRenew: StandardRetryToken, + token: StandardRetryToken, errorInfo: RetryErrorInfo ): Promise { const maxAttempts = await this.getMaxAttempts(); - if (this.shouldRetry(tokenToRenew, errorInfo, maxAttempts)) { - tokenToRenew.getRetryTokenCount(errorInfo); - return tokenToRenew; + if (this.shouldRetry(token, errorInfo, maxAttempts)) { + const errorType = errorInfo.errorType; + this.retryBackoffStrategy.setDelayBase( + errorType === "THROTTLING" ? THROTTLING_RETRY_DELAY_BASE : DEFAULT_RETRY_DELAY_BASE + ); + + const delayFromErrorType = this.retryBackoffStrategy.computeNextBackoffDelay(token.getRetryCount()); + const retryDelay = errorInfo.retryAfterHint + ? Math.max(errorInfo.retryAfterHint.getTime() - Date.now() || 0, delayFromErrorType) + : delayFromErrorType; + + const capacityCost = this.getCapacityCost(errorType); + this.capacity -= capacityCost; + return createDefaultRetryToken({ + retryDelay, + retryCount: token.getRetryCount() + 1, + retryCost: capacityCost, + }); } + throw new Error("No retry token available"); } public recordSuccess(token: StandardRetryToken): void { - this.retryToken.releaseRetryTokens(token.getLastRetryCost()); + this.capacity = Math.max(INITIAL_RETRY_TOKENS, this.capacity + (token.getRetryCost() ?? NO_RETRY_INCREMENT)); + } + + /** + * @returns the current available retry capacity. + * + * This number decreases when retries are executed and refills when requests or retries succeed. + */ + public getCapacity(): number { + return this.capacity; } private async getMaxAttempts() { - let maxAttempts: number; try { return await this.maxAttemptsProvider(); } catch (error) { @@ -52,13 +87,18 @@ export class StandardRetryStrategy implements RetryStrategyV2 { private shouldRetry(tokenToRenew: StandardRetryToken, errorInfo: RetryErrorInfo, maxAttempts: number): boolean { const attempts = tokenToRenew.getRetryCount(); + return ( attempts < maxAttempts && - tokenToRenew.hasRetryTokens(errorInfo.errorType) && + this.capacity >= this.getCapacityCost(errorInfo.errorType) && this.isRetryableError(errorInfo.errorType) ); } + private getCapacityCost(errorType: RetryErrorType) { + return errorType === "TRANSIENT" ? TIMEOUT_RETRY_COST : RETRY_COST; + } + private isRetryableError(errorType: RetryErrorType): boolean { return errorType === "THROTTLING" || errorType === "TRANSIENT"; } diff --git a/packages/util-retry/src/defaultRetryToken.spec.ts b/packages/util-retry/src/defaultRetryToken.spec.ts index b0c43c8a8ef2..e0604d4a79c1 100644 --- a/packages/util-retry/src/defaultRetryToken.spec.ts +++ b/packages/util-retry/src/defaultRetryToken.spec.ts @@ -1,303 +1,56 @@ -import { RetryErrorInfo, RetryErrorType, SdkError } from "@aws-sdk/types"; - -import { - DEFAULT_RETRY_DELAY_BASE, - INITIAL_RETRY_TOKENS, - MAXIMUM_RETRY_DELAY, - NO_RETRY_INCREMENT, - RETRY_COST, - TIMEOUT_RETRY_COST, -} from "./constants"; -import { getDefaultRetryBackoffStrategy } from "./defaultRetryBackoffStrategy"; -import { getDefaultRetryToken } from "./defaultRetryToken"; +import { DEFAULT_RETRY_DELAY_BASE, MAXIMUM_RETRY_DELAY } from "./constants"; +import { createDefaultRetryToken } from "./defaultRetryToken"; jest.mock("./defaultRetryBackoffStrategy"); describe("defaultRetryToken", () => { - const transientErrorType = "TRANSIENT" as RetryErrorType; - const nonTransientErrorType = "THROTTLING" as RetryErrorType; - - const getDrainedRetryToken = ( - targetCapacity: number, - error: RetryErrorInfo, - initialRetryTokens: number = INITIAL_RETRY_TOKENS - ) => { - const retryToken = getDefaultRetryToken(initialRetryTokens, DEFAULT_RETRY_DELAY_BASE); - let availableCapacity = initialRetryTokens; - while (availableCapacity >= targetCapacity) { - retryToken.getRetryTokenCount(error); - availableCapacity -= targetCapacity; - } - return retryToken; - }; - const mathDotRandom = Math.random; - const setDelayBase = jest.fn(); - const mockRetryBackoffStrategy = { - computeNextBackoffDelay: (attempts: number) => 100, - setDelayBase, - }; - - beforeEach(() => { - Math.random = jest.fn().mockReturnValue(1); - (getDefaultRetryBackoffStrategy as jest.Mock).mockReturnValue(mockRetryBackoffStrategy); - }); - - afterEach(() => { - Math.random = mathDotRandom; - }); - - describe("custom initial retry tokens", () => { - it("hasRetryTokens returns false if capacity is not available", () => { - const customRetryTokens = 5; - const retryToken = getDefaultRetryToken(customRetryTokens, DEFAULT_RETRY_DELAY_BASE); - expect(retryToken.hasRetryTokens(transientErrorType)).toBe(false); - }); - - it("retrieveRetryToken throws error if retry tokens not available", () => { - const customRetryTokens = 5; - const retryToken = getDefaultRetryToken(customRetryTokens, DEFAULT_RETRY_DELAY_BASE); - expect(() => { - retryToken.getRetryTokenCount({ errorType: transientErrorType }); - }).toThrowError(new Error("No retry token available")); - }); - }); - - describe("hasRetryTokens", () => { - describe("returns true if capacity is available", () => { - it("when it's transient error", () => { - const retryToken = getDefaultRetryToken(INITIAL_RETRY_TOKENS, DEFAULT_RETRY_DELAY_BASE); - expect(retryToken.hasRetryTokens(transientErrorType)).toBe(true); - }); - - it("when it's not transient error", () => { - const retryToken = getDefaultRetryToken(INITIAL_RETRY_TOKENS, DEFAULT_RETRY_DELAY_BASE); - expect(retryToken.hasRetryTokens(nonTransientErrorType)).toBe(true); - }); - }); - - describe("returns false if capacity is not available", () => { - it("when it's transient error", () => { - const retryToken = getDrainedRetryToken(TIMEOUT_RETRY_COST, { errorType: transientErrorType }); - expect(retryToken.hasRetryTokens(transientErrorType)).toBe(false); - }); - - it("when it's not transient error", () => { - const retryToken = getDrainedRetryToken(RETRY_COST, { errorType: nonTransientErrorType }); - expect(retryToken.hasRetryTokens(nonTransientErrorType)).toBe(false); - }); - }); - }); - - describe("retrieveRetryToken", () => { - describe("returns retry tokens amount if available", () => { - it("when it's transient error", () => { - const retryToken = getDefaultRetryToken(INITIAL_RETRY_TOKENS, DEFAULT_RETRY_DELAY_BASE); - expect(retryToken.getRetryTokenCount({ errorType: transientErrorType })).toBe(TIMEOUT_RETRY_COST); - expect(retryToken.getLastRetryCost()).toBe(TIMEOUT_RETRY_COST); - }); - - it("when it's not transient error", () => { - const retryToken = getDefaultRetryToken(INITIAL_RETRY_TOKENS, DEFAULT_RETRY_DELAY_BASE); - expect(retryToken.getRetryTokenCount({ errorType: nonTransientErrorType })).toBe(RETRY_COST); - expect(retryToken.getLastRetryCost()).toBe(RETRY_COST); + describe("getRetryCost", () => { + it("is undefined before an error is encountered", () => { + const retryToken = createDefaultRetryToken({ + retryDelay: DEFAULT_RETRY_DELAY_BASE, + retryCount: 0, }); + expect(retryToken.getRetryCost()).toBeUndefined(); }); - describe("throws error if retry tokens not available", () => { - it("when it's transient error", () => { - const retryToken = getDrainedRetryToken(TIMEOUT_RETRY_COST, { errorType: transientErrorType }); - expect(() => { - retryToken.getRetryTokenCount({ errorType: transientErrorType }); - }).toThrowError(new Error("No retry token available")); + it("returns set value", () => { + const retryToken = createDefaultRetryToken({ + retryDelay: DEFAULT_RETRY_DELAY_BASE, + retryCount: 0, + retryCost: 25, }); - - it("when it's not transient error", () => { - const retryToken = getDrainedRetryToken(RETRY_COST, { errorType: nonTransientErrorType }); - expect(() => { - retryToken.getRetryTokenCount({ errorType: nonTransientErrorType }); - }).toThrowError(new Error("No retry token available")); - }); - }); - }); - - describe("getLastRetryCost", () => { - it("is undefined before an error is encountered", () => { - const retryToken = getDefaultRetryToken(INITIAL_RETRY_TOKENS, DEFAULT_RETRY_DELAY_BASE); - expect(retryToken.getLastRetryCost()).toBeUndefined(); - }); - - it("is updated with successive errors", () => { - const retryToken = getDefaultRetryToken(INITIAL_RETRY_TOKENS, DEFAULT_RETRY_DELAY_BASE); - retryToken.getRetryTokenCount({ errorType: transientErrorType }); - expect(retryToken.getLastRetryCost()).toBe(TIMEOUT_RETRY_COST); - retryToken.getRetryTokenCount({ errorType: nonTransientErrorType }); - expect(retryToken.getLastRetryCost()).toBe(RETRY_COST); + expect(retryToken.getRetryCost()).toBe(25); }); }); describe("getRetryCount", () => { - it("returns 0 when count is not set", () => { - const retryToken = getDefaultRetryToken(INITIAL_RETRY_TOKENS, DEFAULT_RETRY_DELAY_BASE); - expect(retryToken.getRetryCount()).toBe(0); - }); - it("returns amount set when token is created", () => { const retryCount = 3; - const retryToken = getDefaultRetryToken(INITIAL_RETRY_TOKENS, DEFAULT_RETRY_DELAY_BASE, retryCount); + const retryToken = createDefaultRetryToken({ + retryDelay: DEFAULT_RETRY_DELAY_BASE, + retryCount, + }); expect(retryToken.getRetryCount()).toBe(retryCount); }); - - it("increments when retries occur", () => { - const retryToken = getDefaultRetryToken(INITIAL_RETRY_TOKENS, DEFAULT_RETRY_DELAY_BASE, 1); - expect(retryToken.getRetryCount()).toBe(1); - retryToken.getRetryTokenCount({ errorType: transientErrorType }); - expect(retryToken.getRetryCount()).toBe(2); - retryToken.getRetryTokenCount({ errorType: nonTransientErrorType }); - expect(retryToken.getRetryCount()).toBe(3); - }); }); describe("getRetryDelay", () => { it("returns initial delay", () => { - const retryToken = getDefaultRetryToken(INITIAL_RETRY_TOKENS, DEFAULT_RETRY_DELAY_BASE); - expect(retryToken.getRetryDelay()).toBe(DEFAULT_RETRY_DELAY_BASE); - }); - - describe("retry delay increases exponentially with attempt number for non-throttling error", () => { - const computeNextBackoffDelay = jest - .fn() - .mockReturnValueOnce(100) - .mockReturnValueOnce(200) - .mockReturnValueOnce(400) - .mockReturnValueOnce(800); - const mockRetryBackoffStrategy = { - computeNextBackoffDelay, - setDelayBase, - }; - (getDefaultRetryBackoffStrategy as jest.Mock).mockReturnValue(mockRetryBackoffStrategy); - const retryToken = getDefaultRetryToken(INITIAL_RETRY_TOKENS, DEFAULT_RETRY_DELAY_BASE); - [0, 1, 2, 3].forEach((attempts) => { - const mockDelayBase = 100; - const expectedDelay = Math.floor(2 ** attempts * mockDelayBase); - it(`(${mockDelayBase}, ${attempts}) returns ${expectedDelay}`, () => { - retryToken.getRetryTokenCount({ errorType: transientErrorType }); - expect(retryToken.getRetryDelay()).toBe(expectedDelay); - expect(computeNextBackoffDelay).toHaveBeenCalledTimes(attempts + 1); - }); - }); - }); - - describe("retry delay increases exponentially with attempt number for throttling error", () => { - const computeNextBackoffDelay = jest - .fn() - .mockReturnValueOnce(500) - .mockReturnValueOnce(1000) - .mockReturnValueOnce(2000) - .mockReturnValueOnce(4000); - const mockRetryBackoffStrategy = { - computeNextBackoffDelay, - setDelayBase, - }; - (getDefaultRetryBackoffStrategy as jest.Mock).mockReturnValue(mockRetryBackoffStrategy); - const retryToken = getDefaultRetryToken(INITIAL_RETRY_TOKENS, DEFAULT_RETRY_DELAY_BASE); - [0, 1, 2, 3].forEach((attempts) => { - const mockDelayBase = 500; - const expectedDelay = Math.floor(2 ** attempts * mockDelayBase); - it(`(${mockDelayBase}, ${attempts}) returns ${expectedDelay}`, () => { - retryToken.getRetryTokenCount({ errorType: nonTransientErrorType }); - expect(retryToken.getRetryDelay()).toBe(expectedDelay); - expect(computeNextBackoffDelay).toHaveBeenCalledTimes(attempts + 1); - }); + const retryToken = createDefaultRetryToken({ + retryDelay: DEFAULT_RETRY_DELAY_BASE, + retryCount: 0, }); + expect(retryToken.getRetryDelay()).toBe(DEFAULT_RETRY_DELAY_BASE); }); describe(`caps retry delay at ${MAXIMUM_RETRY_DELAY / 1000} seconds`, () => { it("when value exceeded because of high delayBase", () => { - const retryToken = getDefaultRetryToken(INITIAL_RETRY_TOKENS, DEFAULT_RETRY_DELAY_BASE * 1000); - expect(retryToken.getRetryDelay()).toBe(MAXIMUM_RETRY_DELAY); - }); - - it("when value exceeded because of high attempts number", () => { - const computeNextBackoffDelay = jest.fn().mockReturnValue(MAXIMUM_RETRY_DELAY); - const mockRetryBackoffStrategy = { - computeNextBackoffDelay, - setDelayBase, - }; - (getDefaultRetryBackoffStrategy as jest.Mock).mockReturnValue(mockRetryBackoffStrategy); - const largeAttemptsNumber = Math.ceil(Math.log2(MAXIMUM_RETRY_DELAY)); - const retryToken = getDefaultRetryToken( - INITIAL_RETRY_TOKENS * largeAttemptsNumber, - DEFAULT_RETRY_DELAY_BASE, - largeAttemptsNumber - ); - retryToken.getRetryTokenCount({ errorType: transientErrorType }); + const retryToken = createDefaultRetryToken({ + retryDelay: DEFAULT_RETRY_DELAY_BASE * 1000, + retryCount: 0, + }); expect(retryToken.getRetryDelay()).toBe(MAXIMUM_RETRY_DELAY); }); }); - - it("uses retry-after hint", () => { - const retryToken = getDefaultRetryToken(INITIAL_RETRY_TOKENS, DEFAULT_RETRY_DELAY_BASE); - // 5 minutes, greater than maximum allowed for normal retry. - const expectedDelay = 5 * 60 * 1000; - const retryAfterHint = new Date(Date.now() + expectedDelay); - const errorInfo: RetryErrorInfo = { - errorType: "TRANSIENT", - retryAfterHint, - }; - retryToken.getRetryTokenCount(errorInfo); - // Subtract small offset on expectedDelay to account for delta to when - // Date.now() is invoked. - expect(retryToken.getRetryDelay()).toBeGreaterThan(expectedDelay - 50); - }); - }); - - describe("releaseRetryToken", () => { - it("adds capacityReleaseAmount if passed", () => { - const { errorType } = { errorType: nonTransientErrorType }; - const retryToken = getDrainedRetryToken(RETRY_COST, { errorType: nonTransientErrorType }); - - // Ensure that retry tokens are not available. - expect(retryToken.hasRetryTokens(errorType)).toBe(false); - - // Release RETRY_COST tokens. - retryToken.releaseRetryTokens(RETRY_COST); - expect(retryToken.hasRetryTokens(errorType)).toBe(true); - expect(retryToken.getRetryTokenCount({ errorType: nonTransientErrorType })).toBe(RETRY_COST); - expect(retryToken.hasRetryTokens(errorType)).toBe(false); - }); - - it("adds NO_RETRY_INCREMENT if capacityReleaseAmount not passed", () => { - const { errorType } = { errorType: nonTransientErrorType }; - const retryToken = getDrainedRetryToken(RETRY_COST, { errorType: nonTransientErrorType }); - - // retry tokens will not be available till NO_RETRY_INCREMENT is added - // till it's equal to RETRY_COST - (INITIAL_RETRY_TOKENS % RETRY_COST) - let tokensReleased = 0; - const tokensToBeReleased = RETRY_COST - (INITIAL_RETRY_TOKENS % RETRY_COST); - while (tokensReleased < tokensToBeReleased) { - expect(retryToken.hasRetryTokens(errorType)).toBe(false); - retryToken.releaseRetryTokens(); - tokensReleased += NO_RETRY_INCREMENT; - } - expect(retryToken.hasRetryTokens(errorType)).toBe(true); - }); - - it("ensures availableCapacity is maxed at INITIAL_RETRY_TOKENS", () => { - const retryToken = getDefaultRetryToken(INITIAL_RETRY_TOKENS, DEFAULT_RETRY_DELAY_BASE); - const { errorType } = { errorType: nonTransientErrorType }; - - // release 100 tokens. - [...Array(100).keys()].forEach(() => { - retryToken.releaseRetryTokens(); - }); - - // availableCapacity is still maxed at INITIAL_RETRY_TOKENS - // hasRetryTokens would be true only till INITIAL_RETRY_TOKENS/RETRY_COST times - [...Array(Math.floor(INITIAL_RETRY_TOKENS / RETRY_COST)).keys()].forEach(() => { - expect(retryToken.hasRetryTokens(errorType)).toBe(true); - retryToken.getRetryTokenCount({ errorType: nonTransientErrorType }); - }); - expect(retryToken.hasRetryTokens(errorType)).toBe(false); - }); }); }); diff --git a/packages/util-retry/src/defaultRetryToken.ts b/packages/util-retry/src/defaultRetryToken.ts index 479fafaa0935..e5ab316ae2d4 100644 --- a/packages/util-retry/src/defaultRetryToken.ts +++ b/packages/util-retry/src/defaultRetryToken.ts @@ -1,97 +1,26 @@ -import { RetryErrorInfo, RetryErrorType, StandardRetryBackoffStrategy, StandardRetryToken } from "@aws-sdk/types"; +import { StandardRetryToken } from "@aws-sdk/types"; -import { - DEFAULT_RETRY_DELAY_BASE, - MAXIMUM_RETRY_DELAY, - NO_RETRY_INCREMENT, - RETRY_COST, - THROTTLING_RETRY_DELAY_BASE, - TIMEOUT_RETRY_COST, -} from "./constants"; -import { getDefaultRetryBackoffStrategy } from "./defaultRetryBackoffStrategy"; - -/** - * @public - */ -export interface DefaultRetryTokenOptions { - /** - * The total amount of retry tokens to be decremented from retry token balance. - */ - retryCost?: number; - - /** - * The total amount of retry tokens to be decremented from retry token balance - * when a throttling error is encountered. - */ - timeoutRetryCost?: number; - - /** - * - */ - retryBackoffStrategy?: StandardRetryBackoffStrategy; -} +import { MAXIMUM_RETRY_DELAY } from "./constants"; /** * @internal */ -export const getDefaultRetryToken = ( - initialRetryTokens: number, - initialRetryDelay: number, - initialRetryCount?: number, - options?: DefaultRetryTokenOptions -): StandardRetryToken => { - const MAX_CAPACITY = initialRetryTokens; - const retryCost = options?.retryCost ?? RETRY_COST; - const timeoutRetryCost = options?.timeoutRetryCost ?? TIMEOUT_RETRY_COST; - const retryBackoffStrategy = options?.retryBackoffStrategy ?? getDefaultRetryBackoffStrategy(); - - let availableCapacity = initialRetryTokens; - let retryDelay = Math.min(MAXIMUM_RETRY_DELAY, initialRetryDelay); - let lastRetryCost: number | undefined = undefined; - let retryCount = initialRetryCount ?? 0; - - const getCapacityAmount = (errorType: RetryErrorType) => (errorType === "TRANSIENT" ? timeoutRetryCost : retryCost); - +export const createDefaultRetryToken = ({ + retryDelay, + retryCount, + retryCost, +}: { + retryDelay: number; + retryCount: number; + retryCost?: number; +}): StandardRetryToken => { const getRetryCount = (): number => retryCount; - - const getRetryDelay = (): number => retryDelay; - - const getLastRetryCost = (): number | undefined => lastRetryCost; - - const hasRetryTokens = (errorType: RetryErrorType): boolean => getCapacityAmount(errorType) <= availableCapacity; - - const getRetryTokenCount = (errorInfo: RetryErrorInfo) => { - const errorType = errorInfo.errorType; - if (!hasRetryTokens(errorType)) { - throw new Error("No retry token available"); - } - const capacityAmount = getCapacityAmount(errorType); - const delayBase = errorType === "THROTTLING" ? THROTTLING_RETRY_DELAY_BASE : DEFAULT_RETRY_DELAY_BASE; - retryBackoffStrategy.setDelayBase(delayBase); - const delayFromErrorType = retryBackoffStrategy.computeNextBackoffDelay(retryCount); - if (errorInfo.retryAfterHint) { - const delayFromRetryAfterHint = errorInfo.retryAfterHint.getTime() - Date.now(); - retryDelay = Math.max(delayFromRetryAfterHint || 0, delayFromErrorType); - } else { - retryDelay = delayFromErrorType; - } - retryCount++; - lastRetryCost = capacityAmount; - availableCapacity -= capacityAmount; - return capacityAmount; - }; - - const releaseRetryTokens = (releaseAmount?: number) => { - availableCapacity += releaseAmount ?? NO_RETRY_INCREMENT; - availableCapacity = Math.min(availableCapacity, MAX_CAPACITY); - }; + const getRetryDelay = (): number => Math.min(MAXIMUM_RETRY_DELAY, retryDelay); + const getRetryCost = (): number | undefined => retryCost; return { getRetryCount, getRetryDelay, - getLastRetryCost, - hasRetryTokens, - getRetryTokenCount, - releaseRetryTokens, + getRetryCost, }; }; diff --git a/private/aws-client-retry-test/src/ClientRetryTest.spec.ts b/private/aws-client-retry-test/src/ClientRetryTest.spec.ts index 1aa486b2a61e..a4820d4db6a3 100644 --- a/private/aws-client-retry-test/src/ClientRetryTest.spec.ts +++ b/private/aws-client-retry-test/src/ClientRetryTest.spec.ts @@ -1,10 +1,21 @@ -import { HeadObjectCommand, S3Client, S3ServiceException } from "@aws-sdk/client-s3"; +import { HeadObjectCommand, S3, S3Client, S3ServiceException } from "@aws-sdk/client-s3"; import { HttpResponse } from "@aws-sdk/protocol-http"; import { RequestHandlerOutput } from "@aws-sdk/types"; -import { StandardRetryStrategy } from "@aws-sdk/util-retry"; +import { ConfiguredRetryStrategy, StandardRetryStrategy } from "@aws-sdk/util-retry"; import { Readable } from "stream"; -describe("Middleware-retry integration tests", () => { +class MockRequestHandler { + async handle() { + return { + response: new HttpResponse({ + statusCode: 429, + body: Buffer.from(""), + }), + }; + } +} + +describe("util-retry integration tests", () => { const mockThrottled: RequestHandlerOutput = { response: new HttpResponse({ statusCode: 429, @@ -22,6 +33,7 @@ describe("Middleware-retry integration tests", () => { Bucket: "TEST_BUCKET", Key: "TEST_KEY", }); + it("should not retry on 200", async () => { const client = new S3Client({ requestHandler: { @@ -34,6 +46,7 @@ describe("Middleware-retry integration tests", () => { expect(response.$metadata.attempts).toBe(1); expect(response.$metadata.totalRetryDelay).toBe(0); }); + it("should retry until success", async () => { const mockHandle = jest .fn() @@ -52,7 +65,8 @@ describe("Middleware-retry integration tests", () => { expect(response.$metadata.attempts).toBe(3); expect(response.$metadata.totalRetryDelay).toBeGreaterThan(0); }); - it("should retry until attemps are exhausted", async () => { + + it("should retry until attempts are exhausted", async () => { const expectedException = new S3ServiceException({ $metadata: { httpStatusCode: 429, @@ -79,4 +93,34 @@ describe("Middleware-retry integration tests", () => { expect(error.$metadata.totalRetryDelay).toBeGreaterThan(0); } }); + + it("should use a shared capacity for retries", async () => { + const expectedInitialCapacity = 500; + const expectedDrainPerAttempt = 5; + const expectedRetryAttemptsPerRequest = 7; + const delayPerRetry = 1; + const expectedRequests = 4; + const expectedRemainingCapacity = + expectedInitialCapacity - expectedDrainPerAttempt * expectedRetryAttemptsPerRequest * expectedRequests; + + const retryStrategy = new ConfiguredRetryStrategy(expectedRetryAttemptsPerRequest, delayPerRetry); + const s3 = new S3({ + requestHandler: new MockRequestHandler(), + retryStrategy, + }); + + expect(retryStrategy.getCapacity()).toEqual(expectedInitialCapacity); + + await Promise.all([ + s3.headBucket({ Bucket: "undefined" }), + s3.headBucket({ Bucket: "undefined" }), + s3.headBucket({ Bucket: "undefined" }), + s3.headBucket({ Bucket: "undefined" }), + ]).catch((e) => { + expect(e.$metadata.attempts).toBe(1 + expectedRetryAttemptsPerRequest); + expect(e.$metadata.totalRetryDelay).toBe(expectedRetryAttemptsPerRequest * delayPerRetry); + }); + + expect(retryStrategy.getCapacity()).toEqual(expectedRemainingCapacity); + }); });