Skip to content

chore: rename maxRetries to maxAttempts #1244

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Jun 5, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 6 additions & 6 deletions packages/middleware-retry/src/configurations.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,26 +3,26 @@ 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.
*/
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)
};
}
17 changes: 9 additions & 8 deletions packages/middleware-retry/src/defaultStrategy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -35,34 +35,35 @@ 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);
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.attempts = attempts + 1;
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;

Expand All @@ -74,7 +75,7 @@ export class ExponentialBackOffStrategy implements RetryStrategy {
err.$metadata = {};
}

err.$metadata.retries = retries;
err.$metadata.attempts = attempts;
err.$metadata.totalRetryDelay = totalDelay;
throw err;
}
Expand Down
30 changes: 15 additions & 15 deletions packages/middleware-retry/src/index.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,42 +12,42 @@ 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 }
} = 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);
});

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
);

await expect(
retryHandler({ input: {}, request: new HttpRequest({}) })
).rejects.toMatchObject(error);

expect(next.mock.calls.length).toBe(maxRetries + 1);
expect(next.mock.calls.length).toBe(maxAttempts);
});

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({}) })
Expand All @@ -69,24 +69,24 @@ 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);

await retryHandler({ input: {}, request: new HttpRequest({}) });

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]
]);
});
});
2 changes: 1 addition & 1 deletion packages/middleware-retry/src/retryMiddleware.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ export const getRetryPlugin = (
options: RetryResolvedConfig
): Pluggable<any, any> => ({
applyToStack: clientStack => {
if (options.maxRetries > 0) {
if (options.maxAttempts > 1) {
clientStack.add(retryMiddleware(options), retryMiddlewareOptions);
}
}
Expand Down
4 changes: 2 additions & 2 deletions packages/types/src/response.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion packages/types/src/util.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down