From 20410f8f71ebe38fbe7f68b57039e10f6d511249 Mon Sep 17 00:00:00 2001 From: clawbot <35+clawbot@noreply.example.org> Date: Fri, 16 Oct 2026 04:56:20 +0200 Subject: [PATCH] The binary `make install` builds retries a refused or unresolvable connection (closes #206) 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 a failed TLS handshake, and `ConnectionRefused` or `FailedToOpenSocket` for a name that does not resolve. Both join `TRANSPORT_CODES` in `src/retry.ts`, so they are retried and called transient. They stay out of `CONNECT_CODES`, so a `POST` or `PUT` is not replayed on them, because bun also gives them when it resends a request on a new connection. `classify` in `src/backup.ts` records a `TimeoutError` as transient, so a stalled download is no longer `unknown`. Not changed: a certificate bun rejects is not retried. Model: opus-5-5 --- README.md | 25 +++++++-- TODO.md | 20 +++++-- src/backup.ts | 5 +- src/retry.ts | 34 +++++++++--- test/cli/backup-end-to-end.test.ts | 84 ++++++++++++++++++++++++++++-- test/retry/retry.test.ts | 84 ++++++++++++++++++++++++++++++ 6 files changed, 233 insertions(+), 19 deletions(-) diff --git a/README.md b/README.md index 3fc1506..5a26fe9 100644 --- a/README.md +++ b/README.md @@ -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 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 -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 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 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 `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 @@ -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 unreachable arriving mid-flight, or a local interface going down after the request was written, delivers them on an already-established socket. They stay -retryable for the idempotent calls. `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. +retryable for the idempotent calls. So do bun's `ConnectionRefused` and +`FailedToOpenSocket`, on which a `POST` or `PUT` is not replayed either. 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. `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 — because a socket reset after the response headers have arrived surfaces in the diff --git a/TODO.md b/TODO.md index 6ef08dc..a8e0a9d 100644 --- a/TODO.md +++ b/TODO.md @@ -17,11 +17,10 @@ pre-1.0 # 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/199, -https://git.eeqj.de/sneak/quak/issues/205 and -https://git.eeqj.de/sneak/quak/issues/206. The check of quak against a live Ente +https://git.eeqj.de/sneak/quak/issues/199 and +https://git.eeqj.de/sneak/quak/issues/205. The check of quak against a live Ente account, https://git.eeqj.de/sneak/quak/issues/187, needs an account only sneak has, so it waits on him. @@ -30,6 +29,19 @@ declares one. # 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 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, diff --git a/src/backup.ts b/src/backup.ts index 16d2da9..3c2b026 100644 --- a/src/backup.ts +++ b/src/backup.ts @@ -223,10 +223,13 @@ const isPresent = (path: string): boolean => { // are transient; refusals and local filesystem/decrypt errors are permanent; // 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, -// 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 => { if (hasTransportCode(err)) return "transient"; const e = err as NodeJS.ErrnoException; + if (e?.name === "TimeoutError") return "transient"; const text = `${e?.code ?? ""} ${describeError(err)}`.toLowerCase(); if ( /timeout|timed out|econnreset|econnrefused|econnaborted|network|socket|eai_again|throttl|temporarily|429|500|502|503|504/.test( diff --git a/src/retry.ts b/src/retry.ts index 09bb752..c210bf7 100644 --- a/src/retry.ts +++ b/src/retry.ts @@ -58,6 +58,14 @@ export const resolveRetryOptions = ( }); // 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([ "ECONNRESET", "ECONNABORTED", @@ -70,6 +78,9 @@ const TRANSPORT_CODES = new Set([ "ENETUNREACH", "ENETRESET", "ENETDOWN", + // Bun's. + "ConnectionRefused", + "FailedToOpenSocket", ]); // 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 // they remain retryable for idempotent calls; only replay eligibility is // 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"]); // 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 }; }; -// 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 // `src/backup.ts` calls it transient. export const hasTransportCode = (err: unknown): boolean => @@ -152,7 +169,8 @@ export const isRetryable = (err: unknown): boolean => { // deliberately literal. Demanding a recognised `cause` instead would // classify real network failures as permanent and fail backups that should // 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; 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 // 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 -// deadline says nothing at all about the server's state. So every errno in the -// cause chain must be a connect errno: one other errno 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. +// acted on. A routing errno can be delivered on an established socket. Bun's +// `ConnectionRefused` and `FailedToOpenSocket` can come from a new connection +// 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 +// 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 => { const { codes, complete } = causeCodes(err); return ( diff --git a/test/cli/backup-end-to-end.test.ts b/test/cli/backup-end-to-end.test.ts index 367251e..1009cc0 100644 --- a/test/cli/backup-end-to-end.test.ts +++ b/test/cli/backup-end-to-end.test.ts @@ -275,13 +275,15 @@ afterAll(() => { // Run `quak backup ` as `bin/quak.ts` runs it, sending its requests // 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 -// a retry does not wait. Its cache is beside `dir`, so each run of one -// directory finds the cache the run before it left, as cron's runs find the -// user's. +// a retry does not wait. A download is given up after `downloadTimeoutMs` with +// no bytes received, 60 seconds unless given. Its cache is beside `dir`, so +// each run of one directory finds the cache the run before it left, as cron's +// runs find the user's. const backup = async ( dir: string, opts: { verify?: boolean } = {}, fetch = server, + downloadTimeoutMs?: number, ): Promise<{ code: number; stderr: string }> => { let printed = ""; const stderr = new Writable({ @@ -308,6 +310,7 @@ const backup = async ( loadSession(path, { fetch, retry: { ...UNATTENDED_RETRY_OPTIONS, sleep: async () => {} }, + downloadTimeoutMs, }), login: 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({ + pull: () => new Promise(() => {}), + }), + ); + } + 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 // 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 diff --git a/test/retry/retry.test.ts b/test/retry/retry.test.ts index 8f2213f..b943985 100644 --- a/test/retry/retry.test.ts +++ b/test/retry/retry.test.ts @@ -77,6 +77,14 @@ const recordingSleep = (): { const errnoError = (code: string, message = code): Error => 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 // --------------------------------------------------------------------------- @@ -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", () => { it("retries a truncated stream", () => { // Truncation is a transfer that stopped early. The bytes that did