Compare commits

..
1 Commits
Author SHA1 Message Date
clawbot 4c9a42db49 The binary make install builds retries a refused or unresolvable connection (closes #206)
check / check (push) Canceled after 0s
Bun's `fetch`, which `~/bin/quak` runs under, rejects with a plain `Error`
that has no `cause` and a code of bun's own. Running the built binary against
local listeners gave `ConnectionRefused` for a refused connection and for a
failed TLS handshake, and `ConnectionRefused` or `FailedToOpenSocket` for a
name that does not resolve. Both codes join `TRANSPORT_CODES` and
`CONNECT_CODES` in `src/retry.ts`, so they are retried, replayed, and called
transient by `classify`. A reset already came as `ECONNRESET` and a deadline
as a `TimeoutError`; the tests pin every shape found.

Judgement call: a failed TLS handshake is replayed, as no request byte was
sent.
Not changed: a certificate bun rejects is not retried.

Model: opus-5-5
2026-10-16 01:43:48 +00:00
6 changed files with 54 additions and 98 deletions
+14 -15
View File
@@ -554,21 +554,20 @@ second-factor attempts. They are retried only when every errno in the error's
`cause` chain is one of the three that establish no TCP connection to the server `cause` chain is one of the three that establish no TCP connection to the server
ever existed, so no request byte can have been transmitted: `ENOTFOUND` and ever existed, so no request byte can have been transmitted: `ENOTFOUND` and
`EAI_AGAIN` (name resolution produced no address) and `ECONNREFUSED` (the peer `EAI_AGAIN` (name resolution produced no address) and `ECONNREFUSED` (the peer
refused the connection). A 5xx, a mid-flight reset and a deadline are all left refused the connection). Under bun these failures are `ConnectionRefused` and
to the caller, because each of them can happen after the server has already `FailedToOpenSocket`. Bun also gives `ConnectionRefused` for a TLS handshake
acted. These two do not follow redirects either: a redirect means the server that failed; the TCP connection existed then, but the request is only sent once
already received the request, so it is reported as an error and not retried. The the handshake is done. A 5xx, a mid-flight reset and a deadline are all left to
routing errnos `EHOSTUNREACH`, `ENETUNREACH` and `ENETDOWN` are excluded for the the caller, because each of them can happen after the server has already acted.
same reason, despite looking like connect-time failures: on Linux an ICMP These two do not follow redirects either: a redirect means the server already
unreachable arriving mid-flight, or a local interface going down after the received the request, so it is reported as an error and not retried. The routing
request was written, delivers them on an already-established socket. They stay errnos `EHOSTUNREACH`, `ENETUNREACH` and `ENETDOWN` are excluded for the same
retryable for the idempotent calls. So do bun's `ConnectionRefused` and reason, despite looking like connect-time failures: on Linux an ICMP unreachable
`FailedToOpenSocket`, on which a `POST` or `PUT` is not replayed either. When a arriving mid-flight, or a local interface going down after the request was
connection bun kept open from an earlier request closes before any answer, bun written, delivers them on an already-established socket. They stay retryable for
sends the request again on a new connection, and if that one fails it gives one the idempotent calls. `putFile` is exempt: a presigned PUT stores one whole
of these codes although the server may already have the request. `putFile` is object at one key in one request, so replaying it has no partial state to
exempt: a presigned PUT stores one whole object at one key in one request, so damage.
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
+3 -8
View File
@@ -33,14 +33,9 @@ declares one.
connection and a name that does not resolve, and the backup records either as 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, transient (issue 206). Bun's `fetch` reports them with codes of its own,
`ConnectionRefused` and `FailedToOpenSocket`, which `src/retry.ts` now counts `ConnectionRefused` and `FailedToOpenSocket`, which `src/retry.ts` now counts
as transport failures. A `POST` or `PUT` is not replayed on them, because bun as transport failures that are safe to replay. Bun already gave `ECONNRESET`
also gives them when it resends a request on a new connection after a for a reset and a `TimeoutError` for a deadline. The shapes were found by
connection it kept open closed, and the server may already have the request. running the binary against local listeners.
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
+1 -4
View File
@@ -223,13 +223,10 @@ 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. An error named `TimeoutError` is transient // where Node's `fetch` puts it.
// 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(
+25 -24
View File
@@ -61,11 +61,10 @@ export const resolveRetryOptions = (
// //
// The errnos are Node's. The binary `make install` builds runs under bun, // 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` // 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 // of bun's own; `CONNECT_CODES` says what bun's two codes mean. For a
// a TLS handshake that failed, and `ConnectionRefused` or `FailedToOpenSocket` // connection reset or closed before the response ended, bun gives Node's
// for a name that does not resolve. For a connection reset or closed before // `ECONNRESET`. These were found by running that binary, built with bun
// the response ended, it gives Node's `ECONNRESET`. These were found by // 1.3.13, against listeners on 127.0.0.1.
// 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",
@@ -83,10 +82,14 @@ const TRANSPORT_CODES = new Set([
"FailedToOpenSocket", "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 any request byte
// exists, and therefore before any request byte could have been written: name // could have been written. Node reports these before a TCP connection exists:
// resolution produced no address (`ENOTFOUND`, `EAI_AGAIN`) or the peer // name resolution produced no address (`ENOTFOUND`, `EAI_AGAIN`) or the peer
// refused the connection with an RST to the SYN (`ECONNREFUSED`). // refused the connection with an RST to the SYN (`ECONNREFUSED`). Bun gives
// `ConnectionRefused` for a refused connection, and `ConnectionRefused` or
// `FailedToOpenSocket` for a name that does not resolve. It also gives
// `ConnectionRefused` for a TLS handshake that failed, after the TCP
// connection exists, but the request is only sent once the handshake is done.
// //
// The routing errnos — `EHOSTUNREACH`, `ENETUNREACH`, `ENETDOWN` — are // The routing errnos — `EHOSTUNREACH`, `ENETUNREACH`, `ENETDOWN` — are
// deliberately absent even though they look like connect-time failures. On // deliberately absent even though they look like connect-time failures. On
@@ -98,13 +101,13 @@ 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`.
// const CONNECT_CODES = new Set([
// Bun's `ConnectionRefused` and `FailedToOpenSocket` are absent too, so a "ENOTFOUND",
// `POST` or `PUT` is not replayed on them. When a connection bun kept open from "EAI_AGAIN",
// an earlier request closes before any answer, bun sends the request again on "ECONNREFUSED",
// a new connection, and if that one fails it gives one of these codes although "ConnectionRefused",
// the server may already have the request. "FailedToOpenSocket",
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
// errno on the error it throws — it hangs the underlying socket error off // errno on the error it throws — it hangs the underlying socket error off
@@ -181,17 +184,15 @@ export const isRetryable = (err: unknown): boolean => {
// `postJSON` and `putJSON` use this for every `POST` and `PUT` listed in the // `postJSON` and `putJSON` use this for every `POST` and `PUT` listed in the
// README under "Endpoints used"; verifying a second factor, for one, consumes // README under "Endpoints used"; verifying a second factor, for one, consumes
// one of a small number of attempts. They are replayed only on the failures in // one of a small number of attempts. They are replayed only on the failures in
// `CONNECT_CODES`, which establish that no TCP connection to the server ever // `CONNECT_CODES`, which establish that no request byte was ever transmitted:
// existed: there was no address to connect to, or the peer refused the // there was no address to connect to, the peer refused the connection
// connection outright. A request byte cannot have been transmitted, so the // outright, or, under bun, the TLS handshake failed. The server cannot have
// server cannot have acted. // acted.
// //
// 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. Bun's // acted on. A routing errno can be delivered on an established socket. A
// `ConnectionRefused` and `FailedToOpenSocket` can come from a new connection // deadline says nothing at all about the server's state. So every code in the
// bun opened to resend a request that already went out on a connection it kept
// open. A deadline says nothing at all about the server's state. So every code in the
// cause chain must be one of `CONNECT_CODES`: one other code anywhere in the // 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 // 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. // doubt too: the links below the limit were never read.
+3 -37
View File
@@ -275,15 +275,13 @@ 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. A download is given up after `downloadTimeoutMs` with // a retry does not wait. Its cache is beside `dir`, so each run of one
// no bytes received, 60 seconds unless given. Its cache is beside `dir`, so // directory finds the cache the run before it left, as cron's runs find the
// each run of one directory finds the cache the run before it left, as cron's // user'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({
@@ -310,7 +308,6 @@ 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,
@@ -771,37 +768,6 @@ describe("quak backup", () => {
}, },
); );
// 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
+8 -10
View File
@@ -258,23 +258,21 @@ describe("isRetryable and isSafeToReplay: bun's fetch errors", () => {
// that binary, built with bun 1.3.13, against listeners on 127.0.0.1 and // that binary, built with bun 1.3.13, against listeners on 127.0.0.1 and
// a name under `.invalid`. // a name under `.invalid`.
it("retries a refused connection, and does not replay it", () => { it("retries and replays a refused connection", () => {
// Bun gives the same error when the TLS handshake fails, and when a // Bun gives the same error when the TLS handshake fails. The TCP
// new connection it opened to resend a request is refused. That // connection existed then, but the request is only sent once the
// request may already have reached the server on a connection bun // handshake is done, so replaying it is as safe.
// kept open from an earlier request.
const refused = bunError( const refused = bunError(
"ConnectionRefused", "ConnectionRefused",
"Unable to connect. Is the computer able to access the url?", "Unable to connect. Is the computer able to access the url?",
); );
expect(isRetryable(refused)).toBe(true); expect(isRetryable(refused)).toBe(true);
expect(isSafeToReplay(refused)).toBe(false); expect(isSafeToReplay(refused)).toBe(true);
}); });
it("retries a name that does not resolve, with either code bun gives, and does not replay it", () => { it("retries and replays a name that does not resolve, with either code bun gives", () => {
// For the same name bun gave `ConnectionRefused` on some requests and // For the same name bun gave `ConnectionRefused` on some requests and
// `FailedToOpenSocket` on others. Either can also come from a new // `FailedToOpenSocket` on others.
// connection bun opened to resend a request, as above.
for (const unresolved of [ for (const unresolved of [
bunError( bunError(
"ConnectionRefused", "ConnectionRefused",
@@ -286,7 +284,7 @@ describe("isRetryable and isSafeToReplay: bun's fetch errors", () => {
), ),
]) { ]) {
expect(isRetryable(unresolved)).toBe(true); expect(isRetryable(unresolved)).toBe(true);
expect(isSafeToReplay(unresolved)).toBe(false); expect(isSafeToReplay(unresolved)).toBe(true);
} }
}); });