The binary make install builds retries a refused or unresolvable connection (closes #206) #207

Merged
clawbot merged 1 commits from issue-206-bun-fetch-errors into next 2026-10-16 04:56:21 +02:00
6 changed files with 233 additions and 19 deletions
+21 -4
View File
@@ -151,7 +151,9 @@ for each, separated by `, `. A file whose download failed that way is in
`failures.json` with that error, as a transient failure. So is each file with no `failures.json` with that error, as a transient failure. So is each file with no
ML data cached when the ML data fetch failed that way, its error starting ML data cached when the ML data fetch failed that way, its error starting
`ML data: `. The `~/bin/quak` that `make install` builds runs under bun, and `ML data: `. The `~/bin/quak` that `make install` builds runs under bun, and
prints bun's own wording for such a failure. prints bun's own wording for such a failure, with no reason after it, such as
`quak: Unable to connect. Is the computer able to access the url?` for a refused
connection. It records such a failure in `failures.json` as transient too.
After a `1`, the next run fetches each missing original again, fetches the ML After a `1`, the next run fetches each missing original again, fetches the ML
data that is not cached, and rebuilds the album links. An album quak could not data that is not cached, and rebuilds the album links. An album quak could not
@@ -494,6 +496,17 @@ request is repeated only when repeating it could produce a different answer:
declining to retry a transient one costs a single file that the next run picks declining to retry a transient one costs a single file that the next run picks
up. up.
The `~/bin/quak` that `make install` builds runs under bun, whose `fetch`
rejects with a plain `Error` that has no `cause` and carries a code of bun's
own. quak retries a refused connection, a name that does not resolve, a reset
connection and a deadline under bun as it does under Node. Bun reports a refused
connection, and a TLS handshake that failed, as `ConnectionRefused`. For a name
that does not resolve it gives `ConnectionRefused` on some requests and
`FailedToOpenSocket` on others. A connection reset or closed before the response
ended is `ECONNRESET`, as under Node, and a deadline is the same `TimeoutError`.
A certificate bun rejects, reported with a code such as
`DEPTH_ZERO_SELF_SIGNED_CERT`, is not retried.
Backoff is exponential with full jitter: the delay before retry _n_ is Backoff is exponential with full jitter: the delay before retry _n_ is
`random() * min(maxDelayMs, baseDelayMs * 2 ** (n - 1))`. The exponential term `random() * min(maxDelayMs, baseDelayMs * 2 ** (n - 1))`. The exponential term
is the ceiling and the wait is drawn below it, so a client that lost many is the ceiling and the wait is drawn below it, so a client that lost many
@@ -549,9 +562,13 @@ routing errnos `EHOSTUNREACH`, `ENETUNREACH` and `ENETDOWN` are excluded for the
same reason, despite looking like connect-time failures: on Linux an ICMP same reason, despite looking like connect-time failures: on Linux an ICMP
unreachable arriving mid-flight, or a local interface going down after the unreachable arriving mid-flight, or a local interface going down after the
request was written, delivers them on an already-established socket. They stay request was written, delivers them on an already-established socket. They stay
retryable for the idempotent calls. `putFile` is exempt: a presigned PUT stores retryable for the idempotent calls. So do bun's `ConnectionRefused` and
one whole object at one key in one request, so replaying it has no partial state `FailedToOpenSocket`, on which a `POST` or `PUT` is not replayed either. When a
to damage. connection bun kept open from an earlier request closes before any answer, bun
sends the request again on a new connection, and if that one fails it gives one
of these codes although the server may already have the request. `putFile` is
exempt: a presigned PUT stores one whole object at one key in one request, so
replaying it has no partial state to damage.
A download is retried as a whole — request, stream consumption, and decryption — A download is retried as a whole — request, stream consumption, and decryption —
because a socket reset after the response headers have arrived surfaces in the because a socket reset after the response headers have arrived surfaces in the
+16 -4
View File
@@ -17,11 +17,10 @@ pre-1.0
# Next Step # Next Step
Five bugs are open: https://git.eeqj.de/sneak/quak/issues/197, Four bugs are open: https://git.eeqj.de/sneak/quak/issues/197,
https://git.eeqj.de/sneak/quak/issues/198, https://git.eeqj.de/sneak/quak/issues/198,
https://git.eeqj.de/sneak/quak/issues/199, https://git.eeqj.de/sneak/quak/issues/199 and
https://git.eeqj.de/sneak/quak/issues/205 and https://git.eeqj.de/sneak/quak/issues/205. The check of quak against a live Ente
https://git.eeqj.de/sneak/quak/issues/206. The check of quak against a live Ente
account, https://git.eeqj.de/sneak/quak/issues/187, needs an account only sneak account, https://git.eeqj.de/sneak/quak/issues/187, needs an account only sneak
has, so it waits on him. has, so it waits on him.
@@ -30,6 +29,19 @@ declares one.
# Completed Steps # Completed Steps
- 2026-10-16: The `~/bin/quak` that `make install` builds retries a refused
connection and a name that does not resolve, and the backup records either as
transient (issue 206). Bun's `fetch` reports them with codes of its own,
`ConnectionRefused` and `FailedToOpenSocket`, which `src/retry.ts` now counts
as transport failures. A `POST` or `PUT` is not replayed on them, because bun
also gives them when it resends a request on a new connection after a
connection it kept open closed, and the server may already have the request.
Bun already gave `ECONNRESET` for a reset and a `TimeoutError` for a deadline.
`classify` in `src/backup.ts` now records any `TimeoutError` as transient, so
a stalled download, whose error says only that it stalled, is no longer
`unknown`. The shapes were found by running the binary against local
listeners.
- 2026-10-15: A network failure is printed and recorded with its reason, not as - 2026-10-15: A network failure is printed and recorded with its reason, not as
just `fetch failed` (issue 196). `describeError` in `src/errors.ts` gives an just `fetch failed` (issue 196). `describeError` in `src/errors.ts` gives an
error's message followed by the message of each error on its `cause` chain, error's message followed by the message of each error on its `cause` chain,
+4 -1
View File
@@ -223,10 +223,13 @@ const isPresent = (path: string): boolean => {
// are transient; refusals and local filesystem/decrypt errors are permanent; // are transient; refusals and local filesystem/decrypt errors are permanent;
// anything else is unknown. The error code and the error as `describeError` // anything else is unknown. The error code and the error as `describeError`
// gives it are inspected, and so is each errno on the error's cause chain, // gives it are inspected, and so is each errno on the error's cause chain,
// where Node's `fetch` puts it. // where Node's `fetch` puts it. An error named `TimeoutError` is transient
// whatever its message says, as `isRetryable` retries it: each of quak's
// deadlines aborts with one, and the download's says only that it stalled.
const classify = (err: unknown): FailureClass => { const classify = (err: unknown): FailureClass => {
if (hasTransportCode(err)) return "transient"; if (hasTransportCode(err)) return "transient";
const e = err as NodeJS.ErrnoException; const e = err as NodeJS.ErrnoException;
if (e?.name === "TimeoutError") return "transient";
const text = `${e?.code ?? ""} ${describeError(err)}`.toLowerCase(); const text = `${e?.code ?? ""} ${describeError(err)}`.toLowerCase();
if ( if (
/timeout|timed out|econnreset|econnrefused|econnaborted|network|socket|eai_again|throttl|temporarily|429|500|502|503|504/.test( /timeout|timed out|econnreset|econnrefused|econnaborted|network|socket|eai_again|throttl|temporarily|429|500|502|503|504/.test(
+27 -7
View File
@@ -58,6 +58,14 @@ export const resolveRetryOptions = (
}); });
// Transport failures: the request did not complete, for reasons below HTTP. // Transport failures: the request did not complete, for reasons below HTTP.
//
// The errnos are Node's. The binary `make install` builds runs under bun,
// whose `fetch` rejects with a plain `Error` that has no `cause` and a `code`
// of bun's own. Bun gives `ConnectionRefused` for a refused connection and for
// a TLS handshake that failed, and `ConnectionRefused` or `FailedToOpenSocket`
// for a name that does not resolve. For a connection reset or closed before
// the response ended, it gives Node's `ECONNRESET`. These were found by
// running that binary, built with bun 1.3.13, against listeners on 127.0.0.1.
const TRANSPORT_CODES = new Set([ const TRANSPORT_CODES = new Set([
"ECONNRESET", "ECONNRESET",
"ECONNABORTED", "ECONNABORTED",
@@ -70,6 +78,9 @@ const TRANSPORT_CODES = new Set([
"ENETUNREACH", "ENETUNREACH",
"ENETRESET", "ENETRESET",
"ENETDOWN", "ENETDOWN",
// Bun's.
"ConnectionRefused",
"FailedToOpenSocket",
]); ]);
// The subset of the above that can only be reported before a TCP connection // The subset of the above that can only be reported before a TCP connection
@@ -87,6 +98,12 @@ const TRANSPORT_CODES = new Set([
// ambiguity this set exists to exclude. They stay in `TRANSPORT_CODES`, so // ambiguity this set exists to exclude. They stay in `TRANSPORT_CODES`, so
// they remain retryable for idempotent calls; only replay eligibility is // they remain retryable for idempotent calls; only replay eligibility is
// narrowed. See `isSafeToReplay`. // narrowed. See `isSafeToReplay`.
//
// Bun's `ConnectionRefused` and `FailedToOpenSocket` are absent too, so a
// `POST` or `PUT` is not replayed on them. When a connection bun kept open from
// an earlier request closes before any answer, bun sends the request again on
// a new connection, and if that one fails it gives one of these codes although
// the server may already have the request.
const CONNECT_CODES = new Set(["ENOTFOUND", "EAI_AGAIN", "ECONNREFUSED"]); const CONNECT_CODES = new Set(["ENOTFOUND", "EAI_AGAIN", "ECONNREFUSED"]);
// Collect every `code` in an error's cause chain. undici does not put the // Collect every `code` in an error's cause chain. undici does not put the
@@ -105,7 +122,7 @@ const causeCodes = (err: unknown): { codes: string[]; complete: boolean } => {
return { codes, complete }; return { codes, complete };
}; };
// Does the error's cause chain hold the errno of a transport failure, one of // Does the error's cause chain hold the code of a transport failure, one of
// `TRANSPORT_CODES`? `isRetryable` retries such an error, and `classify` in // `TRANSPORT_CODES`? `isRetryable` retries such an error, and `classify` in
// `src/backup.ts` calls it transient. // `src/backup.ts` calls it transient.
export const hasTransportCode = (err: unknown): boolean => export const hasTransportCode = (err: unknown): boolean =>
@@ -152,7 +169,8 @@ export const isRetryable = (err: unknown): boolean => {
// deliberately literal. Demanding a recognised `cause` instead would // deliberately literal. Demanding a recognised `cause` instead would
// classify real network failures as permanent and fail backups that should // classify real network failures as permanent and fail backups that should
// have succeeded; the cost of the imprecision is bounded by the attempt // have succeeded; the cost of the imprecision is bounded by the attempt
// count. // count. Bun's fetch rejects with a plain `Error` instead, which is
// recognised by its code (see `TRANSPORT_CODES`).
if (err instanceof TypeError) return true; if (err instanceof TypeError) return true;
return hasTransportCode(err); return hasTransportCode(err);
}; };
@@ -170,11 +188,13 @@ export const isRetryable = (err: unknown): boolean => {
// //
// Everything else is ambiguous. A 5xx proves the server did process the // Everything else is ambiguous. A 5xx proves the server did process the
// request. A reset or a broken pipe can arrive after it was fully sent and // request. A reset or a broken pipe can arrive after it was fully sent and
// acted on. A routing errno can be delivered on an established socket. A // acted on. A routing errno can be delivered on an established socket. Bun's
// deadline says nothing at all about the server's state. So every errno in the // `ConnectionRefused` and `FailedToOpenSocket` can come from a new connection
// cause chain must be a connect errno: one other errno anywhere in the chain // bun opened to resend a request that already went out on a connection it kept
// is doubt, and doubt is not replayed. A chain longer than the walk is doubt // open. A deadline says nothing at all about the server's state. So every code in the
// too: the links below the limit were never read. // cause chain must be one of `CONNECT_CODES`: one other code anywhere in the
// chain is doubt, and doubt is not replayed. A chain longer than the walk is
// doubt too: the links below the limit were never read.
export const isSafeToReplay = (err: unknown): boolean => { export const isSafeToReplay = (err: unknown): boolean => {
const { codes, complete } = causeCodes(err); const { codes, complete } = causeCodes(err);
return ( return (
+81 -3
View File
@@ -275,13 +275,15 @@ afterAll(() => {
// Run `quak backup <dir>` as `bin/quak.ts` runs it, sending its requests // Run `quak backup <dir>` as `bin/quak.ts` runs it, sending its requests
// through `fetch`, and return its exit code and everything it printed on // through `fetch`, and return its exit code and everything it printed on
// stderr. Its requests are retried as those of `quak backup` are, except that // stderr. Its requests are retried as those of `quak backup` are, except that
// a retry does not wait. Its cache is beside `dir`, so each run of one // a retry does not wait. A download is given up after `downloadTimeoutMs` with
// directory finds the cache the run before it left, as cron's runs find the // no bytes received, 60 seconds unless given. Its cache is beside `dir`, so
// user's. // each run of one directory finds the cache the run before it left, as cron's
// runs find the user's.
const backup = async ( const backup = async (
dir: string, dir: string,
opts: { verify?: boolean } = {}, opts: { verify?: boolean } = {},
fetch = server, fetch = server,
downloadTimeoutMs?: number,
): Promise<{ code: number; stderr: string }> => { ): Promise<{ code: number; stderr: string }> => {
let printed = ""; let printed = "";
const stderr = new Writable({ const stderr = new Writable({
@@ -308,6 +310,7 @@ const backup = async (
loadSession(path, { loadSession(path, {
fetch, fetch,
retry: { ...UNATTENDED_RETRY_OPTIONS, sleep: async () => {} }, retry: { ...UNATTENDED_RETRY_OPTIONS, sleep: async () => {} },
downloadTimeoutMs,
}), }),
login: prompted, login: prompted,
prompt: prompted, prompt: prompted,
@@ -724,6 +727,81 @@ describe("quak backup", () => {
}, },
); );
// The download of toast.mp4 fails as bun's `fetch` reports a failure, in
// the binary `make install` builds: a plain `Error` with bun's own code
// and message, and no cause. These are what bun gave for a refused
// connection, a name that did not resolve and a reset connection. The
// file is tried as often as under Node, and goes into `failures.json`
// with that message, as a transient failure.
it.each([
[
"ConnectionRefused",
"Unable to connect. Is the computer able to access the url?",
],
["FailedToOpenSocket", "Was there a typo in the url or port?"],
[
"ECONNRESET",
"The socket connection was closed unexpectedly. For more information, pass `verbose: true` in the second argument to fetch()",
],
])(
"records a download that failed with bun's %s in failures.json, as transient",
async (bunCode, message) => {
let attempts = 0;
const fetch: typeof globalThis.fetch = async (input, init) => {
if (downloadOf(input) === 200) {
attempts++;
throw Object.assign(new Error(message), {
code: bunCode,
errno: 0,
});
}
return server(input, init);
};
const dir = mkdtempSync(join(root, "backup-"));
const { code } = await backup(dir, {}, fetch);
expect(code).toBe(1);
expect(attempts).toBe(UNATTENDED_RETRY_OPTIONS.attempts);
const failures = readJSON(join(dir, "failures.json"));
expect(failures.files["200"]).toMatchObject({
classification: "transient",
error: message,
});
},
);
// The download of toast.mp4 stalls: the server sends the headers and then
// never a byte of the body. Each attempt is given up after 50 ms, quak's
// own deadline for this run, with a `TimeoutError` whose message says
// only that the download stalled. The file goes into `failures.json` as a
// transient failure, as a request that ran out of time is retried.
it("records a stalled download in failures.json, as transient", async () => {
let attempts = 0;
const fetch: typeof globalThis.fetch = async (input, init) => {
if (downloadOf(input) === 200) {
attempts++;
return new Response(
new ReadableStream<Uint8Array>({
pull: () => new Promise<void>(() => {}),
}),
);
}
return server(input, init);
};
const dir = mkdtempSync(join(root, "backup-"));
const { code } = await backup(dir, {}, fetch, 50);
expect(code).toBe(1);
expect(attempts).toBe(UNATTENDED_RETRY_OPTIONS.attempts);
const failures = readJSON(join(dir, "failures.json"));
expect(failures.files["200"]).toMatchObject({
classification: "transient",
error: "download stalled: no bytes received for 50 ms",
});
});
// The name of Ente's API server does not resolve when quak fetches the ML // The name of Ente's API server does not resolve when quak fetches the ML
// data, after every original has been backed up. No file has ML data // data, after every original has been backed up. No file has ML data
// cached yet, so each goes into `failures.json` with the reason, as a // cached yet, so each goes into `failures.json` with the reason, as a
+84
View File
@@ -77,6 +77,14 @@ const recordingSleep = (): {
const errnoError = (code: string, message = code): Error => const errnoError = (code: string, message = code): Error =>
Object.assign(new Error(message), { code }); Object.assign(new Error(message), { code });
/**
* An error shaped like what bun's `fetch` rejects with for a failure below
* HTTP: a plain `Error` with bun's own message, a `code`, an `errno` of 0, and
* no `cause`.
*/
const bunError = (code: string, message: string): Error =>
Object.assign(new Error(message), { code, errno: 0 });
// --------------------------------------------------------------------------- // ---------------------------------------------------------------------------
// Classification // Classification
// --------------------------------------------------------------------------- // ---------------------------------------------------------------------------
@@ -244,6 +252,82 @@ describe("isRetryable: transport failures", () => {
}); });
}); });
describe("isRetryable and isSafeToReplay: bun's fetch errors", () => {
// The binary `make install` builds runs quak under bun, whose `fetch`
// does not reject as Node's does. Each shape here was found by running
// that binary, built with bun 1.3.13, against listeners on 127.0.0.1 and
// a name under `.invalid`.
it("retries a refused connection, and does not replay it", () => {
// Bun gives the same error when the TLS handshake fails, and when a
// new connection it opened to resend a request is refused. That
// request may already have reached the server on a connection bun
// kept open from an earlier request.
const refused = bunError(
"ConnectionRefused",
"Unable to connect. Is the computer able to access the url?",
);
expect(isRetryable(refused)).toBe(true);
expect(isSafeToReplay(refused)).toBe(false);
});
it("retries a name that does not resolve, with either code bun gives, and does not replay it", () => {
// For the same name bun gave `ConnectionRefused` on some requests and
// `FailedToOpenSocket` on others. Either can also come from a new
// connection bun opened to resend a request, as above.
for (const unresolved of [
bunError(
"ConnectionRefused",
"Unable to connect. Is the computer able to access the url?",
),
bunError(
"FailedToOpenSocket",
"Was there a typo in the url or port?",
),
]) {
expect(isRetryable(unresolved)).toBe(true);
expect(isSafeToReplay(unresolved)).toBe(false);
}
});
it("retries a connection closed before the response ended, and does not replay it", () => {
// Bun gives Node's errno, `ECONNRESET`, for a reset and for a close,
// before the response or part-way through the body. The request may
// already have reached the server.
const reset = bunError(
"ECONNRESET",
"The socket connection was closed unexpectedly. For more information, pass `verbose: true` in the second argument to fetch()",
);
expect(isRetryable(reset)).toBe(true);
expect(isSafeToReplay(reset)).toBe(false);
});
it("retries a request that ran out of time, and does not replay it", () => {
// What bun's fetch rejects with when `AbortSignal.timeout()`, which
// bounds each JSON request, fires.
const timedOut = new DOMException(
"The operation timed out.",
"TimeoutError",
);
expect(isRetryable(timedOut)).toBe(true);
expect(isSafeToReplay(timedOut)).toBe(false);
});
it("does not retry a certificate bun rejects", () => {
// A certificate that fails verification fails the same way however
// often it is asked. Under Node it is retried only because every
// `TypeError` is.
expect(
isRetryable(
bunError(
"DEPTH_ZERO_SELF_SIGNED_CERT",
"self signed certificate",
),
),
).toBe(false);
});
});
describe("isRetryable: stream truncation versus corruption", () => { describe("isRetryable: stream truncation versus corruption", () => {
it("retries a truncated stream", () => { it("retries a truncated stream", () => {
// Truncation is a transfer that stopped early. The bytes that did // Truncation is a transfer that stopped early. The bytes that did