Skip to content

Commit e50f619

Browse files
committed
Preserve double-knock request aborts
Keep aborts attached to the original Request or signed Request out of the transport retry path. This preserves custom abort reasons and avoids turning cancellation into FetchError during retry handling. Add regression coverage for pre-aborted requests and aborts that occur while waiting to retry an idempotent transport failure. Assisted-by: Codex:gpt-5.5
1 parent b0433aa commit e50f619

2 files changed

Lines changed: 92 additions & 12 deletions

File tree

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

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1827,6 +1827,62 @@ test("doubleKnock() does not retry non-idempotent transport errors", async () =>
18271827
}
18281828
});
18291829

1830+
test("doubleKnock() preserves Request signal abort reasons", async () => {
1831+
const controller = new AbortController();
1832+
const abortReason = "request aborted";
1833+
controller.abort(abortReason);
1834+
1835+
const request = new Request("https://example.com/request-abort", {
1836+
signal: controller.signal,
1837+
});
1838+
const error = await assertRejects(
1839+
() =>
1840+
doubleKnock(
1841+
request,
1842+
{
1843+
keyId: rsaPublicKey2.id!,
1844+
privateKey: rsaPrivateKey2,
1845+
},
1846+
),
1847+
);
1848+
1849+
assertEquals(error, abortReason);
1850+
});
1851+
1852+
test("doubleKnock() preserves Request signal aborts during retry delay", async () => {
1853+
fetchMock.spyGlobal();
1854+
1855+
try {
1856+
let requestCount = 0;
1857+
const controller = new AbortController();
1858+
const abortReason = "retry aborted";
1859+
fetchMock.get("https://example.com/aborted-retry", () => {
1860+
requestCount++;
1861+
setTimeout(() => controller.abort(abortReason));
1862+
throw new TypeError("temporary DNS failure");
1863+
});
1864+
1865+
const request = new Request("https://example.com/aborted-retry", {
1866+
signal: controller.signal,
1867+
});
1868+
const error = await assertRejects(
1869+
() =>
1870+
doubleKnock(
1871+
request,
1872+
{
1873+
keyId: rsaPublicKey2.id!,
1874+
privateKey: rsaPrivateKey2,
1875+
},
1876+
),
1877+
);
1878+
1879+
assertEquals(error, abortReason);
1880+
assertEquals(requestCount, 1);
1881+
} finally {
1882+
fetchMock.hardReset();
1883+
}
1884+
});
1885+
18301886
test("doubleKnock() async specDeterminer test", async () => {
18311887
// Install mock fetch handler
18321888
fetchMock.spyGlobal();

packages/fedify/src/sig/http.ts

Lines changed: 36 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1354,9 +1354,16 @@ async function fetchDoubleKnockRequest(
13541354
signal,
13551355
});
13561356
} catch (error) {
1357-
if (isAbortError(error, signal)) throw error;
1357+
if (isAbortError(error, signal, request.signal, signedRequest.signal)) {
1358+
throw error;
1359+
}
13581360
if (attempt >= maxAttempts) throw createFetchError(request.url, error);
1359-
await sleep(DOUBLE_KNOCK_TRANSPORT_RETRY_DELAY_MS, signal);
1361+
await sleep(
1362+
DOUBLE_KNOCK_TRANSPORT_RETRY_DELAY_MS,
1363+
signal,
1364+
request.signal,
1365+
signedRequest.signal,
1366+
);
13601367
}
13611368
}
13621369
throw new FetchError(request.url);
@@ -1369,28 +1376,45 @@ function createFetchError(url: string, cause: unknown): FetchError {
13691376
return error;
13701377
}
13711378

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

1377-
async function sleep(ms: number, signal?: AbortSignal): Promise<void> {
1378-
if (signal == null) {
1389+
async function sleep(
1390+
ms: number,
1391+
...signals: (AbortSignal | undefined)[]
1392+
): Promise<void> {
1393+
const abortSignals = signals.filter((signal) => signal != null);
1394+
const abortedSignal = abortSignals.find((signal) => signal.aborted);
1395+
if (abortedSignal != null) throw getAbortReason(abortedSignal);
1396+
if (abortSignals.length < 1) {
13791397
await new Promise<void>((resolve) => setTimeout(resolve, ms));
13801398
return;
13811399
}
1382-
const retrySignal = signal;
1383-
if (retrySignal.aborted) throw getAbortReason(retrySignal);
13841400
await new Promise<void>((resolve, reject) => {
1401+
const removeAbortListeners = () => {
1402+
for (const signal of abortSignals) {
1403+
signal.removeEventListener("abort", handleAbort);
1404+
}
1405+
};
13851406
const timeout = setTimeout(() => {
1386-
retrySignal.removeEventListener("abort", handleAbort);
1407+
removeAbortListeners();
13871408
resolve();
13881409
}, ms);
1389-
function handleAbort() {
1410+
function handleAbort(event: Event) {
13901411
clearTimeout(timeout);
1391-
reject(getAbortReason(retrySignal));
1412+
removeAbortListeners();
1413+
reject(getAbortReason(event.currentTarget as AbortSignal));
1414+
}
1415+
for (const signal of abortSignals) {
1416+
signal.addEventListener("abort", handleAbort, { once: true });
13921417
}
1393-
retrySignal.addEventListener("abort", handleAbort, { once: true });
13941418
});
13951419
}
13961420

0 commit comments

Comments
 (0)