Skip to content

Commit 6cfb9a3

Browse files
committed
Honor aborts in retry handling
Prefer cancellation over transport failure when a signal aborts while fetching or waiting to retry. This keeps doubleKnock() from wrapping a late cancellation as FetchError. Remove the now-unneeded fallback throw after the retry loop by making the loop explicit about running until it returns or throws. #763 (comment) #763 (comment) #763 (comment) Assisted-by: Codex:gpt-5.5
1 parent dfc4e29 commit 6cfb9a3

2 files changed

Lines changed: 52 additions & 12 deletions

File tree

packages/fedify/src/sig/http.test.ts

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1883,6 +1883,41 @@ test("doubleKnock() preserves Request signal aborts during retry delay", async (
18831883
}
18841884
});
18851885

1886+
test("doubleKnock() prefers Request aborts over transport errors", async () => {
1887+
fetchMock.spyGlobal();
1888+
1889+
try {
1890+
let requestCount = 0;
1891+
const controller = new AbortController();
1892+
const abortReason = "transport aborted";
1893+
fetchMock.get("https://example.com/abort-with-transport-error", () => {
1894+
requestCount++;
1895+
controller.abort(abortReason);
1896+
throw new TypeError("temporary DNS failure");
1897+
});
1898+
1899+
const request = new Request(
1900+
"https://example.com/abort-with-transport-error",
1901+
{ signal: controller.signal },
1902+
);
1903+
const error = await assertRejects(
1904+
() =>
1905+
doubleKnock(
1906+
request,
1907+
{
1908+
keyId: rsaPublicKey2.id!,
1909+
privateKey: rsaPrivateKey2,
1910+
},
1911+
),
1912+
);
1913+
1914+
assertEquals(error, abortReason);
1915+
assertEquals(requestCount, 1);
1916+
} finally {
1917+
fetchMock.hardReset();
1918+
}
1919+
});
1920+
18861921
test("doubleKnock() async specDeterminer test", async () => {
18871922
// Install mock fetch handler
18881923
fetchMock.spyGlobal();

packages/fedify/src/sig/http.ts

Lines changed: 17 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1344,7 +1344,7 @@ async function fetchDoubleKnockRequest(
13441344
const maxAttempts = request.method === "GET" || request.method === "HEAD"
13451345
? 2
13461346
: 1;
1347-
for (let attempt = 1; attempt <= maxAttempts; attempt++) {
1347+
for (let attempt = 1;; attempt++) {
13481348
try {
13491349
return await fetch(signedRequest, {
13501350
// Since Bun has a bug that ignores the `Request.redirect` option,
@@ -1354,7 +1354,13 @@ async function fetchDoubleKnockRequest(
13541354
signal,
13551355
});
13561356
} catch (error) {
1357-
if (isAbortError(error, signal, request.signal, signedRequest.signal)) {
1357+
const abortedSignal = getAbortedSignal(
1358+
signal,
1359+
request.signal,
1360+
signedRequest.signal,
1361+
);
1362+
if (abortedSignal != null) throw getAbortReason(abortedSignal);
1363+
if (isAbortError(error)) {
13581364
throw error;
13591365
}
13601366
if (attempt >= maxAttempts) throw createFetchError(request.url, error);
@@ -1366,7 +1372,6 @@ async function fetchDoubleKnockRequest(
13661372
);
13671373
}
13681374
}
1369-
throw new FetchError(request.url);
13701375
}
13711376

13721377
function createFetchError(url: string, cause: unknown): FetchError {
@@ -1376,22 +1381,16 @@ function createFetchError(url: string, cause: unknown): FetchError {
13761381
return error;
13771382
}
13781383

1379-
function isAbortError(
1380-
error: unknown,
1381-
...signals: (AbortSignal | undefined)[]
1382-
): boolean {
1383-
return error instanceof Error && error.name === "AbortError" ||
1384-
signals.some((signal) =>
1385-
signal?.aborted === true && error === signal.reason
1386-
);
1384+
function isAbortError(error: unknown): boolean {
1385+
return error instanceof Error && error.name === "AbortError";
13871386
}
13881387

13891388
async function sleep(
13901389
ms: number,
13911390
...signals: (AbortSignal | undefined)[]
13921391
): Promise<void> {
13931392
const abortSignals = signals.filter((signal) => signal != null);
1394-
const abortedSignal = abortSignals.find((signal) => signal.aborted);
1393+
const abortedSignal = getAbortedSignal(...abortSignals);
13951394
if (abortedSignal != null) throw getAbortReason(abortedSignal);
13961395
if (abortSignals.length < 1) {
13971396
await new Promise<void>((resolve) => setTimeout(resolve, ms));
@@ -1418,6 +1417,12 @@ async function sleep(
14181417
});
14191418
}
14201419

1420+
function getAbortedSignal(
1421+
...signals: (AbortSignal | undefined)[]
1422+
): AbortSignal | undefined {
1423+
return signals.find((signal) => signal?.aborted);
1424+
}
1425+
14211426
function getAbortReason(signal: AbortSignal): unknown {
14221427
return signal.reason ??
14231428
new DOMException("The operation was aborted.", "AbortError");

0 commit comments

Comments
 (0)