From d17a3e57edcb88ae3a98f78b2f76ccf6c1491b0c Mon Sep 17 00:00:00 2001 From: Trivikram Kamat <16024985+trivikr@users.noreply.github.com> Date: Fri, 5 Jun 2020 03:29:23 +0000 Subject: [PATCH 1/3] chore: rename maxRetries to maxAttempts --- .../middleware-retry/src/configurations.ts | 10 ++++---- .../middleware-retry/src/defaultStrategy.ts | 4 ++-- packages/middleware-retry/src/index.spec.ts | 24 +++++++++---------- .../middleware-retry/src/retryMiddleware.ts | 2 +- packages/types/src/util.ts | 2 +- 5 files changed, 21 insertions(+), 21 deletions(-) diff --git a/packages/middleware-retry/src/configurations.ts b/packages/middleware-retry/src/configurations.ts index da81a5e46a4ec..d169ea4b4416d 100644 --- a/packages/middleware-retry/src/configurations.ts +++ b/packages/middleware-retry/src/configurations.ts @@ -5,24 +5,24 @@ export interface RetryInputConfig { /** * The maximum number of times requests that encounter potentially transient failures should be retried */ - maxRetries?: number; + maxAttempts?: number; /** * The strategy to retry the request. Using built-in exponential backoff strategy by default. */ retryStrategy?: RetryStrategy; } export interface RetryResolvedConfig { - maxRetries: number; + maxAttempts: number; retryStrategy: RetryStrategy; } export function resolveRetryConfig<T>( input: T & RetryInputConfig ): T & RetryResolvedConfig { - const maxRetries = input.maxRetries === undefined ? 3 : input.maxRetries; + const maxAttempts = input.maxAttempts === undefined ? 3 : input.maxAttempts; return { ...input, - maxRetries, + maxAttempts, retryStrategy: - input.retryStrategy || new ExponentialBackOffStrategy(maxRetries) + input.retryStrategy || new ExponentialBackOffStrategy(maxAttempts) }; } diff --git a/packages/middleware-retry/src/defaultStrategy.ts b/packages/middleware-retry/src/defaultStrategy.ts index bb7d41a219604..adaa24ee7c340 100644 --- a/packages/middleware-retry/src/defaultStrategy.ts +++ b/packages/middleware-retry/src/defaultStrategy.ts @@ -35,12 +35,12 @@ export interface DelayDecider { export class ExponentialBackOffStrategy implements RetryStrategy { constructor( - public readonly maxRetries: number, + public readonly maxAttempts: number, private retryDecider: RetryDecider = defaultRetryDecider, private delayDecider: DelayDecider = defaultDelayDecider ) {} private shouldRetry(error: SdkError, retryAttempted: number) { - return retryAttempted < this.maxRetries && this.retryDecider(error); + return retryAttempted < this.maxAttempts && this.retryDecider(error); } async retry<Input extends object, Ouput extends MetadataBearer>( diff --git a/packages/middleware-retry/src/index.spec.ts b/packages/middleware-retry/src/index.spec.ts index f3863aec59873..52f715040f23a 100644 --- a/packages/middleware-retry/src/index.spec.ts +++ b/packages/middleware-retry/src/index.spec.ts @@ -12,9 +12,9 @@ import { SdkError } from "@aws-sdk/smithy-client"; describe("retryMiddleware", () => { it("should not retry when the handler completes successfully", async () => { const next = jest.fn().mockResolvedValue({ output: { $metadata: {} } }); - const retryHandler = retryMiddleware(resolveRetryConfig({ maxRetries: 0 }))( - next - ); + const retryHandler = retryMiddleware( + resolveRetryConfig({ maxAttempts: 0 }) + )(next); const { output: { $metadata } @@ -26,11 +26,11 @@ describe("retryMiddleware", () => { }); it("should stop retrying when the the maximum number of retries is reached", async () => { - const maxRetries = 3; + const maxAttempts = 3; const error = new Error(); error.name = "ProvisionedThroughputExceededException"; const next = jest.fn().mockRejectedValue(error); - const retryHandler = retryMiddleware(resolveRetryConfig({ maxRetries }))( + const retryHandler = retryMiddleware(resolveRetryConfig({ maxAttempts }))( next ); @@ -38,16 +38,16 @@ describe("retryMiddleware", () => { retryHandler({ input: {}, request: new HttpRequest({}) }) ).rejects.toMatchObject(error); - expect(next.mock.calls.length).toBe(maxRetries + 1); + expect(next.mock.calls.length).toBe(maxAttempts + 1); }); it("should not retry if the error is not transient", async () => { const error = new Error(); error.name = "ValidationException"; const next = jest.fn().mockRejectedValue(error); - const retryHandler = retryMiddleware(resolveRetryConfig({ maxRetries: 3 }))( - next - ); + const retryHandler = retryMiddleware( + resolveRetryConfig({ maxAttempts: 3 }) + )(next); await expect( retryHandler({ input: {}, request: new HttpRequest({}) }) @@ -69,15 +69,15 @@ describe("retryMiddleware", () => { jest.mock("./delayDecider"); - const maxRetries = 3; + const maxAttempts = 3; const delayDeciderMock = jest.spyOn( delayDeciderModule, "defaultDelayDecider" ); const retryDecider: RetryDecider = (error: SdkError) => true; - const strategy = new ExponentialBackOffStrategy(maxRetries, retryDecider); + const strategy = new ExponentialBackOffStrategy(maxAttempts, retryDecider); const retryHandler = retryMiddleware({ - maxRetries, + maxAttempts, retryStrategy: strategy })(next); diff --git a/packages/middleware-retry/src/retryMiddleware.ts b/packages/middleware-retry/src/retryMiddleware.ts index 33ded6cd0d66a..744220e217313 100644 --- a/packages/middleware-retry/src/retryMiddleware.ts +++ b/packages/middleware-retry/src/retryMiddleware.ts @@ -31,7 +31,7 @@ export const getRetryPlugin = ( options: RetryResolvedConfig ): Pluggable<any, any> => ({ applyToStack: clientStack => { - if (options.maxRetries > 0) { + if (options.maxAttempts > 0) { clientStack.add(retryMiddleware(options), retryMiddlewareOptions); } } diff --git a/packages/types/src/util.ts b/packages/types/src/util.ts index 92083efb29735..83512cdea7062 100644 --- a/packages/types/src/util.ts +++ b/packages/types/src/util.ts @@ -61,7 +61,7 @@ export interface RetryStrategy { * the maximum number of times requests that encounter potentially * transient failures should be retried */ - maxRetries: number; + 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. From 7d63dea111ba0e0f880ad309406f3165a613b43d Mon Sep 17 00:00:00 2001 From: Trivikram Kamat <16024985+trivikr@users.noreply.github.com> Date: Fri, 5 Jun 2020 21:07:00 +0000 Subject: [PATCH 2/3] chore: rename retries to attempts in defaultStrategy --- packages/middleware-retry/src/configurations.ts | 2 +- packages/middleware-retry/src/defaultStrategy.ts | 15 ++++++++------- packages/middleware-retry/src/index.spec.ts | 6 +++--- packages/middleware-retry/src/retryMiddleware.ts | 2 +- 4 files changed, 13 insertions(+), 12 deletions(-) diff --git a/packages/middleware-retry/src/configurations.ts b/packages/middleware-retry/src/configurations.ts index d169ea4b4416d..c0c7ddd8b492c 100644 --- a/packages/middleware-retry/src/configurations.ts +++ b/packages/middleware-retry/src/configurations.ts @@ -3,7 +3,7 @@ import { ExponentialBackOffStrategy } from "./defaultStrategy"; export interface RetryInputConfig { /** - * The maximum number of times requests that encounter potentially transient failures should be retried + * The maximum number of times requests that encounter retryable failures should be attempted. */ maxAttempts?: number; /** diff --git a/packages/middleware-retry/src/defaultStrategy.ts b/packages/middleware-retry/src/defaultStrategy.ts index adaa24ee7c340..90108bb93c3a0 100644 --- a/packages/middleware-retry/src/defaultStrategy.ts +++ b/packages/middleware-retry/src/defaultStrategy.ts @@ -39,30 +39,31 @@ export class ExponentialBackOffStrategy implements RetryStrategy { private retryDecider: RetryDecider = defaultRetryDecider, private delayDecider: DelayDecider = defaultDelayDecider ) {} - private shouldRetry(error: SdkError, retryAttempted: number) { - return retryAttempted < this.maxAttempts && this.retryDecider(error); + private shouldRetry(error: SdkError, attempts: number) { + return attempts < this.maxAttempts && this.retryDecider(error); } async retry<Input extends object, Ouput extends MetadataBearer>( next: FinalizeHandler<Input, Ouput>, args: FinalizeHandlerArguments<Input> ) { - let retries = 0; + let attempts = 0; let totalDelay = 0; while (true) { try { const { response, output } = await next(args); - output.$metadata.retries = retries; + output.$metadata.retries = attempts; output.$metadata.totalRetryDelay = totalDelay; return { response, output }; } catch (err) { - if (this.shouldRetry(err as SdkError, retries)) { + attempts++; + if (this.shouldRetry(err as SdkError, attempts)) { const delay = this.delayDecider( isThrottlingError(err) ? THROTTLING_RETRY_DELAY_BASE : DEFAULT_RETRY_DELAY_BASE, - retries++ + attempts ); totalDelay += delay; @@ -74,7 +75,7 @@ export class ExponentialBackOffStrategy implements RetryStrategy { err.$metadata = {}; } - err.$metadata.retries = retries; + err.$metadata.retries = attempts; err.$metadata.totalRetryDelay = totalDelay; throw err; } diff --git a/packages/middleware-retry/src/index.spec.ts b/packages/middleware-retry/src/index.spec.ts index 52f715040f23a..36889f6f45863 100644 --- a/packages/middleware-retry/src/index.spec.ts +++ b/packages/middleware-retry/src/index.spec.ts @@ -38,7 +38,7 @@ describe("retryMiddleware", () => { retryHandler({ input: {}, request: new HttpRequest({}) }) ).rejects.toMatchObject(error); - expect(next.mock.calls.length).toBe(maxAttempts + 1); + expect(next.mock.calls.length).toBe(maxAttempts); }); it("should not retry if the error is not transient", async () => { @@ -85,8 +85,8 @@ describe("retryMiddleware", () => { expect(next.mock.calls.length).toBe(3); expect(delayDeciderMock.mock.calls).toEqual([ - [DEFAULT_RETRY_DELAY_BASE, 0], - [THROTTLING_RETRY_DELAY_BASE, 1] + [DEFAULT_RETRY_DELAY_BASE, 1], + [THROTTLING_RETRY_DELAY_BASE, 2] ]); }); }); diff --git a/packages/middleware-retry/src/retryMiddleware.ts b/packages/middleware-retry/src/retryMiddleware.ts index 744220e217313..2bc454e805062 100644 --- a/packages/middleware-retry/src/retryMiddleware.ts +++ b/packages/middleware-retry/src/retryMiddleware.ts @@ -31,7 +31,7 @@ export const getRetryPlugin = ( options: RetryResolvedConfig ): Pluggable<any, any> => ({ applyToStack: clientStack => { - if (options.maxAttempts > 0) { + if (options.maxAttempts > 1) { clientStack.add(retryMiddleware(options), retryMiddlewareOptions); } } From f9c8f5f46b8b3121327da15aaeb013c5cc3ae2cc Mon Sep 17 00:00:00 2001 From: Trivikram Kamat <16024985+trivikr@users.noreply.github.com> Date: Fri, 5 Jun 2020 21:18:02 +0000 Subject: [PATCH 3/3] chore: update ResponseMetadata.retries to attempts --- packages/middleware-retry/src/defaultStrategy.ts | 4 ++-- packages/middleware-retry/src/index.spec.ts | 2 +- packages/types/src/response.ts | 4 ++-- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/packages/middleware-retry/src/defaultStrategy.ts b/packages/middleware-retry/src/defaultStrategy.ts index 90108bb93c3a0..ad95c87c462c9 100644 --- a/packages/middleware-retry/src/defaultStrategy.ts +++ b/packages/middleware-retry/src/defaultStrategy.ts @@ -52,7 +52,7 @@ export class ExponentialBackOffStrategy implements RetryStrategy { while (true) { try { const { response, output } = await next(args); - output.$metadata.retries = attempts; + output.$metadata.attempts = attempts + 1; output.$metadata.totalRetryDelay = totalDelay; return { response, output }; @@ -75,7 +75,7 @@ export class ExponentialBackOffStrategy implements RetryStrategy { err.$metadata = {}; } - err.$metadata.retries = attempts; + err.$metadata.attempts = attempts; err.$metadata.totalRetryDelay = totalDelay; throw err; } diff --git a/packages/middleware-retry/src/index.spec.ts b/packages/middleware-retry/src/index.spec.ts index 36889f6f45863..8762f29b941d0 100644 --- a/packages/middleware-retry/src/index.spec.ts +++ b/packages/middleware-retry/src/index.spec.ts @@ -19,7 +19,7 @@ describe("retryMiddleware", () => { const { output: { $metadata } } = await retryHandler({ input: {}, request: new HttpRequest({}) }); - expect($metadata.retries).toBe(0); + expect($metadata.attempts).toBe(1); expect($metadata.totalRetryDelay).toBe(0); expect(next.mock.calls.length).toBe(1); diff --git a/packages/types/src/response.ts b/packages/types/src/response.ts index 3bcbb9759a79e..4ab0525c63f07 100644 --- a/packages/types/src/response.ts +++ b/packages/types/src/response.ts @@ -28,9 +28,9 @@ export interface ResponseMetadata { cfId?: string; /** - * The number of times this operation was retried. + * The number of times this operation was attempted. */ - retries?: number; + attempts?: number; /** * The total amount of time (in milliseconds) that was spent waiting between