Skip to content

Commit b0433aa

Browse files
committed
Handle double-knock transport failures
Wrap transport failures from doubleKnock() in FetchError so callers get a consistent document loading error instead of raw TypeError values from the runtime fetch implementation. Retry idempotent signed GET and HEAD requests once after a short delay, which covers transient DNS and connection hiccups without replaying non-idempotent inbox deliveries. Fixes #762 Assisted-by: Codex:gpt-5.5
1 parent ff5b755 commit b0433aa

3 files changed

Lines changed: 178 additions & 15 deletions

File tree

CHANGES.md

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,15 @@ Version 2.0.17
88

99
To be released.
1010

11+
### @fedify/fedify
12+
13+
- Fixed `doubleKnock()` so transient transport failures such as DNS hiccups
14+
no longer leak raw `TypeError`s. Idempotent authenticated document
15+
fetches are retried once, and remaining transport failures are reported as
16+
`FetchError` with the original error as the cause. [[#762]]
17+
18+
[#762]: https://github.com/fedify-dev/fedify/issues/762
19+
1120

1221
Version 2.0.16
1322
--------------

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

Lines changed: 103 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
import { mockDocumentLoader, test } from "@fedify/fixture";
22
import type { CryptographicKey, Multikey } from "@fedify/vocab";
3-
import { exportSpki } from "@fedify/vocab-runtime";
3+
import { exportSpki, FetchError } from "@fedify/vocab-runtime";
44
import {
55
assert,
66
assertEquals,
@@ -1725,6 +1725,108 @@ test("doubleKnock() detects redirect loops", async () => {
17251725
fetchMock.hardReset();
17261726
});
17271727

1728+
test("doubleKnock() retries idempotent request transport errors", async () => {
1729+
fetchMock.spyGlobal();
1730+
1731+
try {
1732+
let requestCount = 0;
1733+
fetchMock.get("https://example.com/flaky-document", () => {
1734+
requestCount++;
1735+
if (requestCount === 1) {
1736+
throw new TypeError("temporary DNS failure");
1737+
}
1738+
return new Response("Success", { status: 200 });
1739+
});
1740+
1741+
const request = new Request("https://example.com/flaky-document");
1742+
const response = await doubleKnock(
1743+
request,
1744+
{
1745+
keyId: rsaPublicKey2.id!,
1746+
privateKey: rsaPrivateKey2,
1747+
},
1748+
);
1749+
1750+
assertEquals(response.status, 200);
1751+
assertEquals(await response.text(), "Success");
1752+
assertEquals(requestCount, 2);
1753+
} finally {
1754+
fetchMock.hardReset();
1755+
}
1756+
});
1757+
1758+
test("doubleKnock() wraps repeated transport errors", async () => {
1759+
fetchMock.spyGlobal();
1760+
1761+
try {
1762+
let requestCount = 0;
1763+
const failure = new TypeError("DNS lookup failed");
1764+
fetchMock.get("https://example.com/unreachable-document", () => {
1765+
requestCount++;
1766+
throw failure;
1767+
});
1768+
1769+
const request = new Request("https://example.com/unreachable-document");
1770+
const error = await assertRejects(
1771+
() =>
1772+
doubleKnock(
1773+
request,
1774+
{
1775+
keyId: rsaPublicKey2.id!,
1776+
privateKey: rsaPrivateKey2,
1777+
},
1778+
),
1779+
FetchError,
1780+
"DNS lookup failed",
1781+
);
1782+
1783+
assertEquals(error.url.href, "https://example.com/unreachable-document");
1784+
assertEquals(error.cause, failure);
1785+
assertEquals(requestCount, 2);
1786+
} finally {
1787+
fetchMock.hardReset();
1788+
}
1789+
});
1790+
1791+
test("doubleKnock() does not retry non-idempotent transport errors", async () => {
1792+
fetchMock.spyGlobal();
1793+
1794+
try {
1795+
let requestCount = 0;
1796+
const failure = new TypeError("connection reset");
1797+
fetchMock.post("https://example.com/flaky-inbox", () => {
1798+
requestCount++;
1799+
throw failure;
1800+
});
1801+
1802+
const request = new Request("https://example.com/flaky-inbox", {
1803+
method: "POST",
1804+
body: "Test activity content",
1805+
headers: {
1806+
"Content-Type": "application/activity+json",
1807+
},
1808+
});
1809+
const error = await assertRejects(
1810+
() =>
1811+
doubleKnock(
1812+
request,
1813+
{
1814+
keyId: rsaPublicKey2.id!,
1815+
privateKey: rsaPrivateKey2,
1816+
},
1817+
),
1818+
FetchError,
1819+
"connection reset",
1820+
);
1821+
1822+
assertEquals(error.url.href, "https://example.com/flaky-inbox");
1823+
assertEquals(error.cause, failure);
1824+
assertEquals(requestCount, 1);
1825+
} finally {
1826+
fetchMock.hardReset();
1827+
}
1828+
});
1829+
17281830
test("doubleKnock() async specDeterminer test", async () => {
17291831
// Install mock fetch handler
17301832
fetchMock.spyGlobal();

packages/fedify/src/sig/http.ts

Lines changed: 66 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ import metadata from "../../deno.json" with { type: "json" };
2424
import { fetchKey, type KeyCache, validateCryptoKey } from "./key.ts";
2525

2626
const DEFAULT_MAX_REDIRECTION = 20;
27+
const DOUBLE_KNOCK_TRANSPORT_RETRY_DELAY_MS = 100;
2728

2829
/**
2930
* The standard to use for signing and verifying HTTP signatures.
@@ -1335,6 +1336,69 @@ function createRedirectRequest(
13351336
});
13361337
}
13371338

1339+
async function fetchDoubleKnockRequest(
1340+
request: Request,
1341+
signedRequest: Request,
1342+
signal?: AbortSignal,
1343+
): Promise<Response> {
1344+
const maxAttempts = request.method === "GET" || request.method === "HEAD"
1345+
? 2
1346+
: 1;
1347+
for (let attempt = 1; attempt <= maxAttempts; attempt++) {
1348+
try {
1349+
return await fetch(signedRequest, {
1350+
// Since Bun has a bug that ignores the `Request.redirect` option,
1351+
// to work around it we specify `redirect: "manual"` here too:
1352+
// https://github.com/oven-sh/bun/issues/10754
1353+
redirect: "manual",
1354+
signal,
1355+
});
1356+
} catch (error) {
1357+
if (isAbortError(error, signal)) throw error;
1358+
if (attempt >= maxAttempts) throw createFetchError(request.url, error);
1359+
await sleep(DOUBLE_KNOCK_TRANSPORT_RETRY_DELAY_MS, signal);
1360+
}
1361+
}
1362+
throw new FetchError(request.url);
1363+
}
1364+
1365+
function createFetchError(url: string, cause: unknown): FetchError {
1366+
const message = cause instanceof Error ? cause.message : String(cause);
1367+
const error = new FetchError(url, message);
1368+
error.cause = cause;
1369+
return error;
1370+
}
1371+
1372+
function isAbortError(error: unknown, signal?: AbortSignal): boolean {
1373+
return error instanceof Error && error.name === "AbortError" ||
1374+
signal?.aborted === true && error === signal.reason;
1375+
}
1376+
1377+
async function sleep(ms: number, signal?: AbortSignal): Promise<void> {
1378+
if (signal == null) {
1379+
await new Promise<void>((resolve) => setTimeout(resolve, ms));
1380+
return;
1381+
}
1382+
const retrySignal = signal;
1383+
if (retrySignal.aborted) throw getAbortReason(retrySignal);
1384+
await new Promise<void>((resolve, reject) => {
1385+
const timeout = setTimeout(() => {
1386+
retrySignal.removeEventListener("abort", handleAbort);
1387+
resolve();
1388+
}, ms);
1389+
function handleAbort() {
1390+
clearTimeout(timeout);
1391+
reject(getAbortReason(retrySignal));
1392+
}
1393+
retrySignal.addEventListener("abort", handleAbort, { once: true });
1394+
});
1395+
}
1396+
1397+
function getAbortReason(signal: AbortSignal): unknown {
1398+
return signal.reason ??
1399+
new DOMException("The operation was aborted.", "AbortError");
1400+
}
1401+
13381402
/**
13391403
* Performs a double-knock request to the given URL. For the details of
13401404
* double-knocking, see
@@ -1381,13 +1445,7 @@ async function doubleKnockInternal(
13811445
{ spec: firstTrySpec, tracerProvider, body },
13821446
);
13831447
log?.(signedRequest);
1384-
let response = await fetch(signedRequest, {
1385-
// Since Bun has a bug that ignores the `Request.redirect` option,
1386-
// to work around it we specify `redirect: "manual"` here too:
1387-
// https://github.com/oven-sh/bun/issues/10754
1388-
redirect: "manual",
1389-
signal,
1390-
});
1448+
let response = await fetchDoubleKnockRequest(request, signedRequest, signal);
13911449
// Follow redirects manually to get the final URL:
13921450
if (
13931451
response.status >= 300 && response.status < 400 &&
@@ -1444,13 +1502,7 @@ async function doubleKnockInternal(
14441502
{ spec, tracerProvider, body },
14451503
);
14461504
log?.(signedRequest);
1447-
response = await fetch(signedRequest, {
1448-
// Since Bun has a bug that ignores the `Request.redirect` option,
1449-
// to work around it we specify `redirect: "manual"` here too:
1450-
// https://github.com/oven-sh/bun/issues/10754
1451-
redirect: "manual",
1452-
signal,
1453-
});
1505+
response = await fetchDoubleKnockRequest(request, signedRequest, signal);
14541506
// Follow redirects manually to get the final URL:
14551507
if (
14561508
response.status >= 300 && response.status < 400 &&

0 commit comments

Comments
 (0)