Skip to content

Commit 1abeba4

Browse files
committed
Add preserveHooks option for retryWithMergedOptions
Fixes #2347
1 parent e899c07 commit 1abeba4

File tree

4 files changed

+267
-3
lines changed

4 files changed

+267
-3
lines changed

documentation/9-hooks.md

Lines changed: 38 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -224,9 +224,15 @@ Each function should return the response. This is especially useful when you wan
224224
> - When using the Stream API, this hook is ignored.
225225
226226
**Note:**
227-
> - Calling the `retryWithMergedOptions` function will trigger `beforeRetry` hooks. If the retry is successful, all remaining `afterResponse` hooks will be called. In case of an error, `beforeRetry` hooks will be called instead.
227+
> - Calling the `retryWithMergedOptions` function will trigger `beforeRetry` hooks. By default, remaining `afterResponse` hooks are removed to prevent duplicate execution. To preserve remaining hooks on retry, set `preserveHooks: true` in the options passed to `retryWithMergedOptions`. In case of an error, `beforeRetry` hooks will be called instead.
228228
Meanwhile the `init`, `beforeRequest` , `beforeRedirect` as well as already executed `afterResponse` hooks will be skipped.
229229

230+
**Note:**
231+
> - To preserve remaining `afterResponse` hooks after calling `retryWithMergedOptions`, set `preserveHooks: true` in the options passed to `retryWithMergedOptions`. This is useful when you want hooks to run on retried requests.
232+
233+
**Warning:**
234+
> - Be cautious when using `preserveHooks: true`. If a hook unconditionally calls `retryWithMergedOptions` with `preserveHooks: true`, it will create an infinite retry loop. Always ensure hooks have proper conditional logic to avoid infinite retries.
235+
230236
```js
231237
import got from 'got';
232238

@@ -264,6 +270,37 @@ const instance = got.extend({
264270
});
265271
```
266272

273+
**Example with `preserveHooks`:**
274+
275+
```js
276+
import got from 'got';
277+
278+
const instance = got.extend({
279+
hooks: {
280+
afterResponse: [
281+
(response, retryWithMergedOptions) => {
282+
if (response.statusCode === 401) {
283+
return retryWithMergedOptions({
284+
headers: {
285+
authorization: getNewToken()
286+
},
287+
preserveHooks: true // Keep remaining hooks
288+
});
289+
}
290+
291+
return response;
292+
},
293+
(response) => {
294+
// This hook will run on the retried request
295+
// (the original request is interrupted when the first hook triggers a retry)
296+
console.log('Response received:', response.statusCode);
297+
return response;
298+
}
299+
]
300+
}
301+
});
302+
```
303+
267304
#### `beforeError`
268305

269306
**Type: `BeforeErrorHook[]`**\

source/as-promise/index.ts

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -87,6 +87,8 @@ export default function asPromise<T>(firstRequest?: Request): CancelableRequest<
8787
// @ts-expect-error TS doesn't notice that CancelableRequest is a Promise
8888
// eslint-disable-next-line no-await-in-loop
8989
response = await hook(response, async (updatedOptions): CancelableRequest<Response> => {
90+
const preserveHooks = updatedOptions.preserveHooks ?? false;
91+
9092
options.merge(updatedOptions);
9193
options.prefixUrl = '';
9294

@@ -96,7 +98,10 @@ export default function asPromise<T>(firstRequest?: Request): CancelableRequest<
9698

9799
// Remove any further hooks for that request, because we'll call them anyway.
98100
// The loop continues. We don't want duplicates (asPromise recursion).
99-
options.hooks.afterResponse = options.hooks.afterResponse.slice(0, index);
101+
// Unless preserveHooks is true, in which case we keep the remaining hooks.
102+
if (!preserveHooks) {
103+
options.hooks.afterResponse = options.hooks.afterResponse.slice(0, index);
104+
}
100105

101106
throw new RetryError(request);
102107
});

source/core/options.ts

Lines changed: 44 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -305,9 +305,15 @@ export type Hooks = {
305305
> - When using the Stream API, this hook is ignored.
306306
307307
**Note:**
308-
> - Calling the `retryWithMergedOptions` function will trigger `beforeRetry` hooks. If the retry is successful, all remaining `afterResponse` hooks will be called. In case of an error, `beforeRetry` hooks will be called instead.
308+
> - Calling the `retryWithMergedOptions` function will trigger `beforeRetry` hooks. By default, remaining `afterResponse` hooks are removed to prevent duplicate execution. To preserve remaining hooks on retry, set `preserveHooks: true` in the options passed to `retryWithMergedOptions`. In case of an error, `beforeRetry` hooks will be called instead.
309309
Meanwhile the `init`, `beforeRequest` , `beforeRedirect` as well as already executed `afterResponse` hooks will be skipped.
310310
311+
**Note:**
312+
> - To preserve remaining `afterResponse` hooks after calling `retryWithMergedOptions`, set `preserveHooks: true` in the options passed to `retryWithMergedOptions`. This is useful when you want hooks to run on retried requests.
313+
314+
**Warning:**
315+
> - Be cautious when using `preserveHooks: true`. If a hook unconditionally calls `retryWithMergedOptions` with `preserveHooks: true`, it will create an infinite retry loop. Always ensure hooks have proper conditional logic to avoid infinite retries.
316+
311317
@example
312318
```
313319
import got from 'got';
@@ -345,6 +351,37 @@ export type Hooks = {
345351
mutableDefaults: true
346352
});
347353
```
354+
355+
@example
356+
```
357+
// Example with preserveHooks
358+
import got from 'got';
359+
360+
const instance = got.extend({
361+
hooks: {
362+
afterResponse: [
363+
(response, retryWithMergedOptions) => {
364+
if (response.statusCode === 401) {
365+
return retryWithMergedOptions({
366+
headers: {
367+
authorization: getNewToken()
368+
},
369+
preserveHooks: true // Keep remaining hooks
370+
});
371+
}
372+
373+
return response;
374+
},
375+
(response) => {
376+
// This hook will run on the retried request
377+
// (the original request is interrupted when the first hook triggers a retry)
378+
console.log('Response received:', response.statusCode);
379+
return response;
380+
}
381+
]
382+
}
383+
});
384+
```
348385
*/
349386
afterResponse: AfterResponseHook[];
350387
};
@@ -678,6 +715,7 @@ export type OptionsInit =
678715
& {
679716
hooks?: Partial<Hooks>;
680717
retry?: Partial<RetryOptions>;
718+
preserveHooks?: boolean;
681719
};
682720

683721
const globalCache = new Map();
@@ -1135,6 +1173,11 @@ export default class Options {
11351173
continue;
11361174
}
11371175

1176+
// Never merge `preserveHooks` - it's a control flag, not a persistent option
1177+
if (key === 'preserveHooks') {
1178+
continue;
1179+
}
1180+
11381181
if (!(key in this)) {
11391182
throw new Error(`Unexpected option: ${key}`);
11401183
}

test/hooks.ts

Lines changed: 179 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -774,6 +774,185 @@ test('does not throw on afterResponse retry HTTP failure if throwHttpErrors is f
774774
t.is(statusCode, 500);
775775
});
776776

777+
test('afterResponse preserveHooks keeps remaining hooks on retry', withServer, async (t, server, got) => {
778+
server.get('/', (request, response) => {
779+
if (request.headers.token !== 'unicorn') {
780+
response.statusCode = 401;
781+
}
782+
783+
response.end();
784+
});
785+
786+
let firstHookCalls = 0;
787+
let secondHookCalls = 0;
788+
789+
const {statusCode} = await got({
790+
hooks: {
791+
afterResponse: [
792+
(response, retryWithMergedOptions) => {
793+
firstHookCalls++;
794+
795+
if (response.statusCode === 401) {
796+
return retryWithMergedOptions({
797+
headers: {
798+
token: 'unicorn',
799+
},
800+
preserveHooks: true,
801+
});
802+
}
803+
804+
return response;
805+
},
806+
response => {
807+
secondHookCalls++;
808+
return response;
809+
},
810+
],
811+
},
812+
});
813+
814+
t.is(statusCode, 200);
815+
t.is(firstHookCalls, 2); // Called for both original and retry
816+
t.is(secondHookCalls, 1); // Called only on retry (original was interrupted by RetryError)
817+
});
818+
819+
test('afterResponse without preserveHooks skips remaining hooks on retry', withServer, async (t, server, got) => {
820+
server.get('/', (request, response) => {
821+
if (request.headers.token !== 'unicorn') {
822+
response.statusCode = 401;
823+
}
824+
825+
response.end();
826+
});
827+
828+
let firstHookCalls = 0;
829+
let secondHookCalls = 0;
830+
831+
const {statusCode} = await got({
832+
hooks: {
833+
afterResponse: [
834+
(response, retryWithMergedOptions) => {
835+
firstHookCalls++;
836+
837+
if (response.statusCode === 401) {
838+
return retryWithMergedOptions({
839+
headers: {
840+
token: 'unicorn',
841+
},
842+
});
843+
}
844+
845+
return response;
846+
},
847+
response => {
848+
secondHookCalls++;
849+
return response;
850+
},
851+
],
852+
},
853+
});
854+
855+
t.is(statusCode, 200);
856+
t.is(firstHookCalls, 1); // Called only on original request (removed from retry by default)
857+
t.is(secondHookCalls, 0); // Never called (removed by slice before it could run)
858+
});
859+
860+
test('afterResponse preserveHooks with three hooks', withServer, async (t, server, got) => {
861+
server.get('/', (request, response) => {
862+
if (request.headers.token !== 'unicorn') {
863+
response.statusCode = 401;
864+
}
865+
866+
response.end();
867+
});
868+
869+
let firstHookCalls = 0;
870+
let secondHookCalls = 0;
871+
let thirdHookCalls = 0;
872+
873+
const {statusCode} = await got({
874+
hooks: {
875+
afterResponse: [
876+
(response, retryWithMergedOptions) => {
877+
firstHookCalls++;
878+
879+
if (response.statusCode === 401) {
880+
return retryWithMergedOptions({
881+
headers: {
882+
token: 'unicorn',
883+
},
884+
preserveHooks: true,
885+
});
886+
}
887+
888+
return response;
889+
},
890+
response => {
891+
secondHookCalls++;
892+
return response;
893+
},
894+
response => {
895+
thirdHookCalls++;
896+
return response;
897+
},
898+
],
899+
},
900+
});
901+
902+
t.is(statusCode, 200);
903+
t.is(firstHookCalls, 2); // Called for both original and retry
904+
t.is(secondHookCalls, 1); // Called only on retry
905+
t.is(thirdHookCalls, 1); // Called only on retry
906+
});
907+
908+
test('afterResponse preserveHooks when second hook triggers retry', withServer, async (t, server, got) => {
909+
server.get('/', (request, response) => {
910+
if (request.headers.token !== 'unicorn') {
911+
response.statusCode = 401;
912+
}
913+
914+
response.end();
915+
});
916+
917+
let firstHookCalls = 0;
918+
let secondHookCalls = 0;
919+
let thirdHookCalls = 0;
920+
921+
const {statusCode} = await got({
922+
hooks: {
923+
afterResponse: [
924+
response => {
925+
firstHookCalls++;
926+
return response;
927+
},
928+
(response, retryWithMergedOptions) => {
929+
secondHookCalls++;
930+
931+
if (response.statusCode === 401) {
932+
return retryWithMergedOptions({
933+
headers: {
934+
token: 'unicorn',
935+
},
936+
preserveHooks: true,
937+
});
938+
}
939+
940+
return response;
941+
},
942+
response => {
943+
thirdHookCalls++;
944+
return response;
945+
},
946+
],
947+
},
948+
});
949+
950+
t.is(statusCode, 200);
951+
t.is(firstHookCalls, 2); // Called for both original and retry (preserved by preserveHooks)
952+
t.is(secondHookCalls, 2); // Called for both original and retry
953+
t.is(thirdHookCalls, 1); // Called only on retry
954+
});
955+
777956
test('throwing in a beforeError hook - promise', withServer, async (t, server, got) => {
778957
server.get('/', (_request, response) => {
779958
response.end('ok');

0 commit comments

Comments
 (0)