diff --git a/packages/util-retry/src/ConfiguredRetryStrategy.spec.ts b/packages/util-retry/src/ConfiguredRetryStrategy.spec.ts index 4d1638e35a77..9a1dd3ea3a88 100644 --- a/packages/util-retry/src/ConfiguredRetryStrategy.spec.ts +++ b/packages/util-retry/src/ConfiguredRetryStrategy.spec.ts @@ -5,13 +5,13 @@ describe(ConfiguredRetryStrategy.name, () => { const strategy = new ConfiguredRetryStrategy(5, (attempt) => attempt * 1000); const token = await strategy.acquireInitialRetryToken(""); - token.getRetryCount = () => 4; + token.getRetryCount = () => 3; const retryToken = await strategy.refreshRetryTokenForRetry(token, { errorType: "TRANSIENT", }); - expect(retryToken.getRetryCount()).toBe(5); - expect(retryToken.getRetryDelay()).toBe(5000); + expect(retryToken.getRetryCount()).toBe(4); + expect(retryToken.getRetryDelay()).toBe(4000); }); }); diff --git a/packages/util-retry/src/StandardRetryStrategy.spec.ts b/packages/util-retry/src/StandardRetryStrategy.spec.ts index cd6bd3e153d4..79d5f95f62a9 100644 --- a/packages/util-retry/src/StandardRetryStrategy.spec.ts +++ b/packages/util-retry/src/StandardRetryStrategy.spec.ts @@ -22,7 +22,7 @@ describe(StandardRetryStrategy.name, () => { }); afterEach(() => { - jest.clearAllMocks; + jest.clearAllMocks(); }); it("sets maxAttemptsProvider as a class member variable", () => { @@ -65,47 +65,49 @@ describe(StandardRetryStrategy.name, () => { expect(getRetryCount).toHaveBeenCalledTimes(3); }); - it("throws when attempts exceeds maxAttempts", async () => { + it("disables any retries when maxAttempts is 1", async () => { const mockRetryToken = { - getRetryCount: () => 2, + getRetryCount: () => 0, getRetryTokenCount: (errorInfo: any) => 1, }; (createDefaultRetryToken as jest.Mock).mockReturnValue(mockRetryToken); - const retryStrategy = new StandardRetryStrategy(() => Promise.resolve(1)); + const retryStrategy = new StandardRetryStrategy(1); const token = await retryStrategy.acquireInitialRetryToken(retryTokenScope); try { await retryStrategy.refreshRetryTokenForRetry(token, errorInfo); + fail(`expected ${noRetryTokenAvailableError}`); } catch (error) { expect(error).toStrictEqual(noRetryTokenAvailableError); } }); - it("throws when attempts exceeds default max attempts (3)", async () => { + it("throws when attempts exceeds maxAttempts", async () => { const mockRetryToken = { - getRetryCount: () => 5, + getRetryCount: () => 2, getRetryTokenCount: (errorInfo: any) => 1, }; (createDefaultRetryToken as jest.Mock).mockReturnValue(mockRetryToken); - const retryStrategy = new StandardRetryStrategy(() => Promise.resolve(5)); + const retryStrategy = new StandardRetryStrategy(() => Promise.resolve(1)); const token = await retryStrategy.acquireInitialRetryToken(retryTokenScope); try { await retryStrategy.refreshRetryTokenForRetry(token, errorInfo); + fail(`expected ${noRetryTokenAvailableError}`); } catch (error) { expect(error).toStrictEqual(noRetryTokenAvailableError); } }); - it("throws when no tokens are available", async () => { + it("throws when attempts exceeds default max attempts (3)", async () => { const mockRetryToken = { - getRetryCount: () => 0, + getRetryCount: () => 5, getRetryTokenCount: (errorInfo: any) => 1, - hasRetryTokens: (errorType: RetryErrorType) => false, }; (createDefaultRetryToken as jest.Mock).mockReturnValue(mockRetryToken); - const retryStrategy = new StandardRetryStrategy(() => Promise.resolve(maxAttempts)); + const retryStrategy = new StandardRetryStrategy(() => Promise.resolve(5)); const token = await retryStrategy.acquireInitialRetryToken(retryTokenScope); try { await retryStrategy.refreshRetryTokenForRetry(token, errorInfo); + fail(`expected ${noRetryTokenAvailableError}`); } catch (error) { expect(error).toStrictEqual(noRetryTokenAvailableError); } @@ -125,6 +127,7 @@ describe(StandardRetryStrategy.name, () => { } as RetryErrorInfo; try { await retryStrategy.refreshRetryTokenForRetry(token, errorInfo); + fail(`expected ${noRetryTokenAvailableError}`); } catch (error) { expect(error).toStrictEqual(noRetryTokenAvailableError); } diff --git a/packages/util-retry/src/StandardRetryStrategy.ts b/packages/util-retry/src/StandardRetryStrategy.ts index 05140f3462f7..c5c6093c5469 100644 --- a/packages/util-retry/src/StandardRetryStrategy.ts +++ b/packages/util-retry/src/StandardRetryStrategy.ts @@ -86,7 +86,7 @@ export class StandardRetryStrategy implements RetryStrategyV2 { } private shouldRetry(tokenToRenew: StandardRetryToken, errorInfo: RetryErrorInfo, maxAttempts: number): boolean { - const attempts = tokenToRenew.getRetryCount(); + const attempts = tokenToRenew.getRetryCount() + 1; return ( attempts < maxAttempts && diff --git a/private/aws-client-retry-test/src/ClientRetryTest.spec.ts b/private/aws-client-retry-test/src/ClientRetryTest.spec.ts index a4820d4db6a3..0f50599813b1 100644 --- a/private/aws-client-retry-test/src/ClientRetryTest.spec.ts +++ b/private/aws-client-retry-test/src/ClientRetryTest.spec.ts @@ -89,7 +89,7 @@ describe("util-retry integration tests", () => { } catch (error) { expect(error).toStrictEqual(expectedException); expect(error.$metadata.httpStatusCode).toBe(429); - expect(error.$metadata.attempts).toBe(4); + expect(error.$metadata.attempts).toBe(3); expect(error.$metadata.totalRetryDelay).toBeGreaterThan(0); } }); @@ -97,13 +97,15 @@ describe("util-retry integration tests", () => { it("should use a shared capacity for retries", async () => { const expectedInitialCapacity = 500; const expectedDrainPerAttempt = 5; - const expectedRetryAttemptsPerRequest = 7; + const expectedRetryAttemptsPerRequest = 6; + // Set maxAttempts to one more than the number of retries (initial request + retries) + const maxAttempts = 7; const delayPerRetry = 1; const expectedRequests = 4; const expectedRemainingCapacity = expectedInitialCapacity - expectedDrainPerAttempt * expectedRetryAttemptsPerRequest * expectedRequests; - const retryStrategy = new ConfiguredRetryStrategy(expectedRetryAttemptsPerRequest, delayPerRetry); + const retryStrategy = new ConfiguredRetryStrategy(maxAttempts, delayPerRetry); const s3 = new S3({ requestHandler: new MockRequestHandler(), retryStrategy,