Skip to content

Commit 8f3fdc0

Browse files
trivikrtrivikr
authored
feat: read maxAttempts value from retry-config (#1286)
1 parent 9e2614e commit 8f3fdc0

File tree

7 files changed

+215
-83
lines changed

7 files changed

+215
-83
lines changed

Diff for: ‎packages/middleware-retry/src/configurations.spec.ts

+57-20
Original file line numberDiff line numberDiff line change
@@ -1,46 +1,83 @@
11
import { resolveRetryConfig } from "./configurations";
22
import { StandardRetryStrategy } from "./defaultStrategy";
33

4+
jest.mock("./defaultStrategy", () => ({
5+
StandardRetryStrategy: jest.fn().mockReturnValue({})
6+
}));
7+
48
describe("resolveRetryConfig", () => {
9+
const maxAttemptsDefaultProvider = jest.fn();
10+
11+
afterEach(() => {
12+
jest.clearAllMocks();
13+
});
14+
515
describe("maxAttempts", () => {
6-
it("uses passed maxAttempts value if present", () => {
7-
[1, 2, 3].forEach(maxAttempts => {
8-
expect(resolveRetryConfig({ maxAttempts }).maxAttempts).toEqual(
9-
maxAttempts
10-
);
11-
});
16+
it("assigns maxAttempts value if present", async () => {
17+
for (const maxAttempts of [1, 2, 3]) {
18+
const output = await resolveRetryConfig({
19+
maxAttempts,
20+
maxAttemptsDefaultProvider
21+
}).maxAttempts();
22+
expect(output).toStrictEqual(maxAttempts.toString());
23+
expect(maxAttemptsDefaultProvider).not.toHaveBeenCalled();
24+
}
1225
});
1326

14-
it("assigns default value of 3 if maxAttempts not passed", () => {
15-
expect(resolveRetryConfig({}).maxAttempts).toEqual(3);
27+
it("assigns maxAttemptsDefaultProvider if maxAttempts not present", () => {
28+
const mockMaxAttempts = jest.fn();
29+
maxAttemptsDefaultProvider.mockReturnValueOnce(mockMaxAttempts);
30+
31+
const input = { maxAttemptsDefaultProvider };
32+
expect(resolveRetryConfig(input).maxAttempts).toStrictEqual(
33+
mockMaxAttempts
34+
);
35+
36+
expect(maxAttemptsDefaultProvider).toHaveBeenCalledTimes(1);
37+
expect(maxAttemptsDefaultProvider).toHaveBeenCalledWith(input);
1638
});
1739
});
1840

1941
describe("retryStrategy", () => {
20-
it("uses passed retryStrategy if present", () => {
42+
it("passes retryStrategy if present", () => {
2143
const mockRetryStrategy = {
22-
maxAttempts: 2,
2344
retry: jest.fn()
2445
};
2546
const { retryStrategy } = resolveRetryConfig({
26-
retryStrategy: mockRetryStrategy
47+
retryStrategy: mockRetryStrategy,
48+
maxAttemptsDefaultProvider
2749
});
2850
expect(retryStrategy).toEqual(mockRetryStrategy);
2951
});
3052

3153
describe("creates StandardRetryStrategy if retryStrategy not present", () => {
32-
describe("uses maxAttempts if present", () => {
33-
[1, 2, 3].forEach(maxAttempts => {
34-
const { retryStrategy } = resolveRetryConfig({ maxAttempts });
35-
expect(retryStrategy).toBeInstanceOf(StandardRetryStrategy);
36-
expect(retryStrategy.maxAttempts).toBe(maxAttempts);
37-
});
54+
describe("passes maxAttempts if present", () => {
55+
for (const maxAttempts of [1, 2, 3]) {
56+
it(`when maxAttempts=${maxAttempts}`, async () => {
57+
const { retryStrategy } = resolveRetryConfig({
58+
maxAttempts,
59+
maxAttemptsDefaultProvider
60+
});
61+
expect(retryStrategy).toBeInstanceOf(StandardRetryStrategy);
62+
expect(StandardRetryStrategy as jest.Mock).toHaveBeenCalledTimes(1);
63+
const output = await (StandardRetryStrategy as jest.Mock).mock.calls[0][0]();
64+
expect(output).toStrictEqual(maxAttempts.toString());
65+
});
66+
}
3867
});
3968

40-
it("uses default 3 if maxAttempts is not present", () => {
41-
const { retryStrategy } = resolveRetryConfig({});
69+
it("passes maxAttemptsDefaultProvider if maxAttempts is not present", () => {
70+
const mockMaxAttempts = jest.fn();
71+
maxAttemptsDefaultProvider.mockReturnValueOnce(mockMaxAttempts);
72+
73+
const { retryStrategy } = resolveRetryConfig({
74+
maxAttemptsDefaultProvider
75+
});
4276
expect(retryStrategy).toBeInstanceOf(StandardRetryStrategy);
43-
expect(retryStrategy.maxAttempts).toBe(3);
77+
expect(StandardRetryStrategy as jest.Mock).toHaveBeenCalledTimes(1);
78+
expect((StandardRetryStrategy as jest.Mock).mock.calls[0][0]).toEqual(
79+
mockMaxAttempts
80+
);
4481
});
4582
});
4683
});

Diff for: ‎packages/middleware-retry/src/configurations.ts

+18-4
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { RetryStrategy } from "@aws-sdk/types";
1+
import { RetryStrategy, Provider } from "@aws-sdk/types";
22
import { StandardRetryStrategy } from "./defaultStrategy";
33

44
export interface RetryInputConfig {
@@ -12,18 +12,32 @@ export interface RetryInputConfig {
1212
retryStrategy?: RetryStrategy;
1313
}
1414

15+
interface PreviouslyResolved {
16+
maxAttemptsDefaultProvider: (input: any) => Provider<string>;
17+
}
1518
export interface RetryResolvedConfig {
16-
maxAttempts: number;
19+
maxAttempts: Provider<string>;
1720
retryStrategy: RetryStrategy;
1821
}
1922

2023
export const resolveRetryConfig = <T>(
21-
input: T & RetryInputConfig
24+
input: T & PreviouslyResolved & RetryInputConfig
2225
): T & RetryResolvedConfig => {
23-
const maxAttempts = input.maxAttempts ?? 3;
26+
const maxAttempts =
27+
normalizeMaxAttempts(input.maxAttempts) ??
28+
input.maxAttemptsDefaultProvider(input as any);
2429
return {
2530
...input,
2631
maxAttempts,
2732
retryStrategy: input.retryStrategy || new StandardRetryStrategy(maxAttempts)
2833
};
2934
};
35+
36+
const normalizeMaxAttempts = (
37+
maxAttempts?: number
38+
): Provider<string> | undefined => {
39+
if (maxAttempts) {
40+
const promisified = Promise.resolve(maxAttempts.toString());
41+
return () => promisified;
42+
}
43+
};

Diff for: ‎packages/middleware-retry/src/defaultStrategy.spec.ts

+113-25
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,9 @@ describe("defaultStrategy", () => {
5555
output: { $metadata: {} }
5656
});
5757

58-
const retryStrategy = new StandardRetryStrategy(maxAttempts);
58+
const retryStrategy = new StandardRetryStrategy(() =>
59+
Promise.resolve(maxAttempts.toString())
60+
);
5961
return retryStrategy.retry(next, { request: { headers: {} } } as any);
6062
};
6163

@@ -66,7 +68,9 @@ describe("defaultStrategy", () => {
6668
const mockError = options?.mockError ?? new Error("mockError");
6769
next = jest.fn().mockRejectedValue(mockError);
6870

69-
const retryStrategy = new StandardRetryStrategy(maxAttempts);
71+
const retryStrategy = new StandardRetryStrategy(() =>
72+
Promise.resolve(maxAttempts.toString())
73+
);
7074
try {
7175
await retryStrategy.retry(next, { request: { headers: {} } } as any);
7276
} catch (error) {
@@ -90,7 +94,9 @@ describe("defaultStrategy", () => {
9094
.mockRejectedValueOnce(mockError)
9195
.mockResolvedValueOnce(mockResponse);
9296

93-
const retryStrategy = new StandardRetryStrategy(maxAttempts);
97+
const retryStrategy = new StandardRetryStrategy(() =>
98+
Promise.resolve(maxAttempts.toString())
99+
);
94100
return retryStrategy.retry(next, { request: { headers: {} } } as any);
95101
};
96102

@@ -110,81 +116,109 @@ describe("defaultStrategy", () => {
110116
.mockRejectedValueOnce(mockError)
111117
.mockResolvedValueOnce(mockResponse);
112118

113-
const retryStrategy = new StandardRetryStrategy(maxAttempts);
119+
const retryStrategy = new StandardRetryStrategy(() =>
120+
Promise.resolve(maxAttempts.toString())
121+
);
114122
return retryStrategy.retry(next, { request: { headers: {} } } as any);
115123
};
116124

117125
afterEach(() => {
118126
jest.clearAllMocks();
119127
});
120128

121-
it("sets maxAttempts as class member variable", () => {
122-
[1, 2, 3].forEach(maxAttempts => {
123-
const retryStrategy = new StandardRetryStrategy(maxAttempts);
124-
expect(retryStrategy.maxAttempts).toBe(maxAttempts);
129+
it("sets maxAttemptsProvider as class member variable", () => {
130+
["1", "2", "3"].forEach(maxAttempts => {
131+
const retryStrategy = new StandardRetryStrategy(() =>
132+
Promise.resolve(maxAttempts)
133+
);
134+
expect(retryStrategy["maxAttemptsProvider"]()).resolves.toBe(maxAttempts);
125135
});
126136
});
127137

128138
describe("retryDecider init", () => {
129139
it("sets defaultRetryDecider if options is undefined", () => {
130-
const retryStrategy = new StandardRetryStrategy(maxAttempts);
140+
const retryStrategy = new StandardRetryStrategy(() =>
141+
Promise.resolve(maxAttempts.toString())
142+
);
131143
expect(retryStrategy["retryDecider"]).toBe(defaultRetryDecider);
132144
});
133145

134146
it("sets defaultRetryDecider if options.retryDecider is undefined", () => {
135-
const retryStrategy = new StandardRetryStrategy(maxAttempts, {});
147+
const retryStrategy = new StandardRetryStrategy(
148+
() => Promise.resolve(maxAttempts.toString()),
149+
{}
150+
);
136151
expect(retryStrategy["retryDecider"]).toBe(defaultRetryDecider);
137152
});
138153

139154
it("sets options.retryDecider if defined", () => {
140155
const retryDecider = jest.fn();
141-
const retryStrategy = new StandardRetryStrategy(maxAttempts, {
142-
retryDecider
143-
});
156+
const retryStrategy = new StandardRetryStrategy(
157+
() => Promise.resolve(maxAttempts.toString()),
158+
{
159+
retryDecider
160+
}
161+
);
144162
expect(retryStrategy["retryDecider"]).toBe(retryDecider);
145163
});
146164
});
147165

148166
describe("delayDecider init", () => {
149167
it("sets defaultDelayDecider if options is undefined", () => {
150-
const retryStrategy = new StandardRetryStrategy(maxAttempts);
168+
const retryStrategy = new StandardRetryStrategy(() =>
169+
Promise.resolve(maxAttempts.toString())
170+
);
151171
expect(retryStrategy["delayDecider"]).toBe(defaultDelayDecider);
152172
});
153173

154174
it("sets defaultDelayDecider if options.delayDecider undefined", () => {
155-
const retryStrategy = new StandardRetryStrategy(maxAttempts, {});
175+
const retryStrategy = new StandardRetryStrategy(
176+
() => Promise.resolve(maxAttempts.toString()),
177+
{}
178+
);
156179
expect(retryStrategy["delayDecider"]).toBe(defaultDelayDecider);
157180
});
158181

159182
it("sets options.delayDecider if defined", () => {
160183
const delayDecider = jest.fn();
161-
const retryStrategy = new StandardRetryStrategy(maxAttempts, {
162-
delayDecider
163-
});
184+
const retryStrategy = new StandardRetryStrategy(
185+
() => Promise.resolve(maxAttempts.toString()),
186+
{
187+
delayDecider
188+
}
189+
);
164190
expect(retryStrategy["delayDecider"]).toBe(delayDecider);
165191
});
166192
});
167193

168194
describe("retryQuota init", () => {
169195
it("sets getDefaultRetryQuota if options is undefined", () => {
170-
const retryStrategy = new StandardRetryStrategy(maxAttempts);
196+
const retryStrategy = new StandardRetryStrategy(() =>
197+
Promise.resolve(maxAttempts.toString())
198+
);
171199
expect(retryStrategy["retryQuota"]).toBe(
172200
getDefaultRetryQuota(INITIAL_RETRY_TOKENS)
173201
);
174202
});
175203

176204
it("sets getDefaultRetryQuota if options.delayDecider undefined", () => {
177-
const retryStrategy = new StandardRetryStrategy(maxAttempts, {});
205+
const retryStrategy = new StandardRetryStrategy(
206+
() => Promise.resolve(maxAttempts.toString()),
207+
{}
208+
);
178209
expect(retryStrategy["retryQuota"]).toBe(
179210
getDefaultRetryQuota(INITIAL_RETRY_TOKENS)
180211
);
181212
});
182213

183214
it("sets options.retryQuota if defined", () => {
184215
const retryQuota = {} as RetryQuota;
185-
const retryStrategy = new StandardRetryStrategy(maxAttempts, {
186-
retryQuota
187-
});
216+
const retryStrategy = new StandardRetryStrategy(
217+
() => Promise.resolve(maxAttempts.toString()),
218+
{
219+
retryQuota
220+
}
221+
);
188222
expect(retryStrategy["retryQuota"]).toBe(retryQuota);
189223
});
190224
});
@@ -483,7 +517,9 @@ describe("defaultStrategy", () => {
483517
output: { $metadata: {} }
484518
});
485519

486-
const retryStrategy = new StandardRetryStrategy(maxAttempts);
520+
const retryStrategy = new StandardRetryStrategy(() =>
521+
Promise.resolve(maxAttempts.toString())
522+
);
487523
await retryStrategy.retry(next, { request: { headers: {} } } as any);
488524
await retryStrategy.retry(next, { request: { headers: {} } } as any);
489525

@@ -565,7 +601,9 @@ describe("defaultStrategy", () => {
565601
throw mockError;
566602
});
567603

568-
const retryStrategy = new StandardRetryStrategy(maxAttempts);
604+
const retryStrategy = new StandardRetryStrategy(() =>
605+
Promise.resolve(maxAttempts.toString())
606+
);
569607
try {
570608
await retryStrategy.retry(next, { request: { headers: {} } } as any);
571609
} catch (error) {
@@ -577,4 +615,54 @@ describe("defaultStrategy", () => {
577615
((isInstance as unknown) as jest.Mock).mockReturnValue(false);
578616
});
579617
});
618+
619+
describe("defaults maxAttempts to 3", () => {
620+
it("when maxAttemptsProvider throws error", async () => {
621+
const maxAttempts = 3;
622+
const { isInstance } = HttpRequest;
623+
((isInstance as unknown) as jest.Mock).mockReturnValue(true);
624+
625+
next = jest.fn(args => {
626+
expect(args.request.headers["amz-sdk-request"]).toBe(
627+
`attempt=1; max=${maxAttempts}`
628+
);
629+
return Promise.resolve({
630+
response: "mockResponse",
631+
output: { $metadata: {} }
632+
});
633+
});
634+
635+
const retryStrategy = new StandardRetryStrategy(() =>
636+
Promise.reject("ERROR")
637+
);
638+
await retryStrategy.retry(next, { request: { headers: {} } } as any);
639+
640+
expect(next).toHaveBeenCalledTimes(1);
641+
((isInstance as unknown) as jest.Mock).mockReturnValue(false);
642+
});
643+
644+
it("when parseInt fails on maxAttemptsProvider", async () => {
645+
const maxAttempts = 3;
646+
const { isInstance } = HttpRequest;
647+
((isInstance as unknown) as jest.Mock).mockReturnValue(true);
648+
649+
next = jest.fn(args => {
650+
expect(args.request.headers["amz-sdk-request"]).toBe(
651+
`attempt=1; max=${maxAttempts}`
652+
);
653+
return Promise.resolve({
654+
response: "mockResponse",
655+
output: { $metadata: {} }
656+
});
657+
});
658+
659+
const retryStrategy = new StandardRetryStrategy(() =>
660+
Promise.resolve("not-a-number")
661+
);
662+
await retryStrategy.retry(next, { request: { headers: {} } } as any);
663+
664+
expect(next).toHaveBeenCalledTimes(1);
665+
((isInstance as unknown) as jest.Mock).mockReturnValue(false);
666+
});
667+
});
580668
});

0 commit comments

Comments
 (0)