From f44f34eeb105dc81c6f45108295393e96553ddbf Mon Sep 17 00:00:00 2001 From: George Fu Date: Thu, 25 May 2023 19:01:01 +0000 Subject: [PATCH] fix(util-retry): remove deprecated parts of StandardRetryToken --- packages/types/src/retry.ts | 24 +---- .../src/AdaptiveRetryStrategy.spec.ts | 7 +- .../util-retry/src/StandardRetryStrategy.ts | 2 +- .../util-retry/src/defaultRetryToken.spec.ts | 93 +------------------ packages/util-retry/src/defaultRetryToken.ts | 34 +------ 5 files changed, 12 insertions(+), 148 deletions(-) diff --git a/packages/types/src/retry.ts b/packages/types/src/retry.ts index 6034cd8abb3be..ee3ba0b727082 100644 --- a/packages/types/src/retry.ts +++ b/packages/types/src/retry.ts @@ -97,29 +97,7 @@ export interface StandardRetryToken extends RetryToken { /** * @returns the cost of the last retry attempt. */ - getLastRetryCost(): number | undefined; - - /** - * @deprecated StandardRetryStrategy will track retry capacity. - * - * @param errorType - derives the cost of the retry to check whether capacity remains. - * @returns wheather there is enough remaining capacity to retry the given error. - */ - hasRetryTokens(errorType: RetryErrorType): boolean; - - /** - * @deprecated StandardRetryStrategy will track retry capacity. - * @returns the number of available tokens. - */ - getRetryTokenCount(errorInfo: RetryErrorInfo): number; - - /** - * @deprecated use \@aws-sdk/util-retry's StandardRetryStrategy#recordSuccess. - * - * Warning: this method is no-op on StandardRetryTokens provided by - * StandardRetryStrategy in \@aws-sdk/util-retry. - */ - 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 b1a3d0b7891a9..27220d4ca084a 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/StandardRetryStrategy.ts b/packages/util-retry/src/StandardRetryStrategy.ts index 681722406d9fb..bf98131c789d2 100644 --- a/packages/util-retry/src/StandardRetryStrategy.ts +++ b/packages/util-retry/src/StandardRetryStrategy.ts @@ -71,7 +71,7 @@ export class StandardRetryStrategy implements RetryStrategyV2 { } public recordSuccess(token: StandardRetryToken): void { - this.capacity = Math.max(INITIAL_RETRY_TOKENS, this.capacity + (token.getLastRetryCost() ?? NO_RETRY_INCREMENT)); + this.capacity = Math.max(INITIAL_RETRY_TOKENS, this.capacity + (token.getRetryCost() ?? NO_RETRY_INCREMENT)); } /** diff --git a/packages/util-retry/src/defaultRetryToken.spec.ts b/packages/util-retry/src/defaultRetryToken.spec.ts index 0627a99998402..e4706dddfd865 100644 --- a/packages/util-retry/src/defaultRetryToken.spec.ts +++ b/packages/util-retry/src/defaultRetryToken.spec.ts @@ -1,88 +1,17 @@ -import { RetryErrorType } from "@aws-sdk/types"; - -import { - DEFAULT_RETRY_DELAY_BASE, - INITIAL_RETRY_TOKENS, - MAXIMUM_RETRY_DELAY, - NO_RETRY_INCREMENT, - RETRY_COST, -} from "./constants"; +import { DEFAULT_RETRY_DELAY_BASE, INITIAL_RETRY_TOKENS, 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) => { - const retryToken = createDefaultRetryToken({ - availableCapacity: targetCapacity, - retryDelay: DEFAULT_RETRY_DELAY_BASE, - retryCount: 0, - }); - return retryToken; - }; - - describe("custom initial retry tokens", () => { - it("hasRetryTokens returns false if capacity is not available", () => { - const customRetryTokens = 5; - const retryToken = createDefaultRetryToken({ - availableCapacity: customRetryTokens, - retryDelay: DEFAULT_RETRY_DELAY_BASE, - retryCount: 0, - }); - - expect(retryToken.hasRetryTokens(transientErrorType)).toBe(false); - }); - }); - - describe("hasRetryTokens", () => { - describe("returns true if capacity is available", () => { - it("when it's transient error", () => { - const retryToken = createDefaultRetryToken({ - availableCapacity: INITIAL_RETRY_TOKENS, - retryDelay: DEFAULT_RETRY_DELAY_BASE, - retryCount: 0, - }); - expect(retryToken.hasRetryTokens(transientErrorType)).toBe(true); - }); - - it("when it's not transient error", () => { - const retryToken = createDefaultRetryToken({ - availableCapacity: INITIAL_RETRY_TOKENS, - retryDelay: DEFAULT_RETRY_DELAY_BASE, - retryCount: 0, - }); - expect(retryToken.hasRetryTokens(nonTransientErrorType)).toBe(true); - }); - }); - - it("returns false if capacity is not available", () => { - const retryToken = getDrainedRetryToken(0); - expect(retryToken.hasRetryTokens(nonTransientErrorType)).toBe(false); - }); - }); - - describe("getRetryTokenCount", () => { - it("returns retry tokens amount", () => { - const retryToken = createDefaultRetryToken({ - availableCapacity: 123, - retryDelay: DEFAULT_RETRY_DELAY_BASE, - retryCount: 0, - }); - expect(retryToken.getRetryTokenCount({ errorType: "TRANSIENT" })).toBe(123); - }); - }); - - describe("getLastRetryCost", () => { + describe("getRetryCost", () => { it("is undefined before an error is encountered", () => { const retryToken = createDefaultRetryToken({ availableCapacity: INITIAL_RETRY_TOKENS, retryDelay: DEFAULT_RETRY_DELAY_BASE, retryCount: 0, }); - expect(retryToken.getLastRetryCost()).toBeUndefined(); + expect(retryToken.getRetryCost()).toBeUndefined(); }); it("returns set value", () => { @@ -92,7 +21,7 @@ describe("defaultRetryToken", () => { retryCount: 0, lastRetryCost: 25, }); - expect(retryToken.getLastRetryCost()).toBe(25); + expect(retryToken.getRetryCost()).toBe(25); }); }); @@ -129,18 +58,4 @@ describe("defaultRetryToken", () => { }); }); }); - - describe("releaseRetryToken", () => { - it("ensures availableCapacity is maxed at INITIAL_RETRY_TOKENS", () => { - const retryToken = createDefaultRetryToken({ - availableCapacity: INITIAL_RETRY_TOKENS, - retryDelay: DEFAULT_RETRY_DELAY_BASE, - retryCount: 0, - }); - - retryToken.releaseRetryTokens(); - - expect(retryToken.getRetryTokenCount({ errorType: "TRANSIENT" })).toBe(INITIAL_RETRY_TOKENS); - }); - }); }); diff --git a/packages/util-retry/src/defaultRetryToken.ts b/packages/util-retry/src/defaultRetryToken.ts index 8f06e55f691b0..367f4350f0ffa 100644 --- a/packages/util-retry/src/defaultRetryToken.ts +++ b/packages/util-retry/src/defaultRetryToken.ts @@ -1,12 +1,11 @@ -import { RetryErrorInfo, RetryErrorType, StandardRetryToken } from "@aws-sdk/types"; +import { StandardRetryToken } from "@aws-sdk/types"; -import { MAXIMUM_RETRY_DELAY, RETRY_COST, TIMEOUT_RETRY_COST } from "./constants"; +import { MAXIMUM_RETRY_DELAY } from "./constants"; /** * @internal */ export const createDefaultRetryToken = ({ - availableCapacity, retryDelay, retryCount, lastRetryCost, @@ -16,38 +15,13 @@ export const createDefaultRetryToken = ({ retryCount: number; lastRetryCost?: number; }): StandardRetryToken => { - const getCapacityAmount = (errorType: RetryErrorType) => - errorType === "TRANSIENT" ? TIMEOUT_RETRY_COST : RETRY_COST; - const getRetryCount = (): number => retryCount; const getRetryDelay = (): number => Math.min(MAXIMUM_RETRY_DELAY, retryDelay); - const getLastRetryCost = (): number | undefined => lastRetryCost; - - /** - * @deprecated managed by StandardRetryStrategy. - */ - const hasRetryTokens = (errorType: RetryErrorType): boolean => { - return getCapacityAmount(errorType) <= availableCapacity; - }; - - /** - * @deprecated managed by StandardRetryStrategy. - */ - const getRetryTokenCount = (errorInfo?: RetryErrorInfo) => { - return availableCapacity; - }; - - /** - * @deprecated this is done in StandardRetryStrategy.recordSuccess. - */ - const releaseRetryTokens = (releaseAmount?: number) => {}; + const getRetryCost = (): number | undefined => lastRetryCost; return { getRetryCount, getRetryDelay, - getLastRetryCost, - hasRetryTokens, - getRetryTokenCount, - releaseRetryTokens, + getRetryCost, }; };