From 0be9ec7b320b0964b29df341dec683bc2a84e188 Mon Sep 17 00:00:00 2001 From: sneak Date: Thu, 15 Oct 2026 23:13:31 +0000 Subject: [PATCH] A network failure is printed and recorded with its reason (closes #196) Node's `fetch` rejects every failure below HTTP as `fetch failed` and puts the reason on `cause`. `describeError` in `src/errors.ts` gives the message followed by the messages on the cause chain, and for an `AggregateError`, which Node gives when every address of a host fails, the message of each address. `run` prints it, and the backup logs and records it. `classify` reads that text and calls a failure transient when `hasTransportCode`, now exported by `src/retry.ts`, finds a transport errno on the chain. A failed ML data fetch keeps its error as the cause. The bounded walk along the chain moved from `src/retry.ts` to `src/errors.ts`. A truncated download's message leaves the authentication failure to its cause. Model: opus-5-5 --- README.md | 15 ++- TODO.md | 15 ++- src/backup.ts | 37 +++++--- src/cli-run.ts | 13 +-- src/download/index.ts | 8 +- src/errors.ts | 74 ++++++++++++++- src/retry.ts | 28 +++--- test/cli/backup-end-to-end.test.ts | 144 +++++++++++++++++++++++++++++ test/cli/run.test.ts | 80 ++++++++++++++++ test/download/download.test.ts | 11 ++- 10 files changed, 378 insertions(+), 47 deletions(-) diff --git a/README.md b/README.md index e474ac9..3fc1506 100644 --- a/README.md +++ b/README.md @@ -141,6 +141,18 @@ matters. | `2` | Another backup of the same directory is running. This run sent no request and changed nothing. | | `3` | There is no usable session: none is saved, the saved one is corrupt, or the server no longer accepts it. Run `quak login` as the user the cron job runs as. | +`` is the error's message, followed by the message of each error that +caused it. Under Node, as `yarn quak` runs quak, `fetch` reports every failure +below HTTP as just `fetch failed` and gives the reason as its cause, so a run +that could not reach the server prints, for example, +`quak: fetch failed: connect ECONNREFUSED 1.2.3.4:443`. When the server has +several addresses and none of them could be reached, the line gives the reason +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. + 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 decrypt, or a file in `failures.json` because quak could not decrypt it, keeps @@ -354,7 +366,8 @@ quak/ exif.ts EXIF read from an image's bytes with exifreader mldata-fetch.ts fetch + decrypt per-file ML data filename.ts safe file names from server metadata - errors.ts error types shared across layers + errors.ts error types shared across layers, and how an error + is described retry.ts retry classifier + exponential backoff with jitter thumbnails.ts detect + regenerate missing thumbnails client.ts high-level Client class assembled from the above diff --git a/TODO.md b/TODO.md index f077269..e6b7236 100644 --- a/TODO.md +++ b/TODO.md @@ -17,8 +17,7 @@ pre-1.0 # Next Step -Four bugs are open: https://git.eeqj.de/sneak/quak/issues/196, -https://git.eeqj.de/sneak/quak/issues/197, +Three bugs are open: https://git.eeqj.de/sneak/quak/issues/197, https://git.eeqj.de/sneak/quak/issues/198 and https://git.eeqj.de/sneak/quak/issues/199. The check of quak against a live Ente account, https://git.eeqj.de/sneak/quak/issues/187, needs an account only sneak @@ -29,6 +28,18 @@ declares one. # Completed Steps +- 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, + such as `fetch failed: connect ECONNREFUSED 1.2.3.4:443`, and each address's + reason when a host has several and none could be reached. `run` prints that, + and the backup logs it and records it in `failures.json`, where `classify` now + calls the failure transient when the chain holds an errno that + `hasTransportCode` in `src/retry.ts` recognises, for a failed ML data fetch + too. The walk along the chain moved from `src/retry.ts` to `src/errors.ts`, + which both use. Tests refuse every connection of a backup, then only one + file's download, and fail the ML data fetch. + - 2026-10-15: The README says that `make install` needs nix (issue 200), in Getting Started and in "Running the backup from cron": `make build-bin` runs bun through `nix-shell -p bun`. The `Makefile` is unchanged. diff --git a/src/backup.ts b/src/backup.ts index 488ff30..16d2da9 100644 --- a/src/backup.ts +++ b/src/backup.ts @@ -82,6 +82,7 @@ import { init, } from "./crypto/index.js"; import { removeLeftoverTempFiles } from "./download/index.js"; +import { describeError } from "./errors.js"; import { sanitizeFileName, withExtension } from "./filename.js"; import { copyAtomic, @@ -94,6 +95,7 @@ import type { Undecryptable } from "./library/store.js"; import { extractImageMetadata } from "./metadata-backup.js"; import type { MLData } from "./mldata-fetch.js"; import type { Collection, EnteFile, FileMetadata } from "./model/types.js"; +import { hasTransportCode } from "./retry.js"; export type ProgressCallback = (message: string) => void; @@ -219,11 +221,13 @@ const isPresent = (path: string): boolean => { // Best-effort classification for the ledger. Retryable server/network problems // are transient; refusals and local filesystem/decrypt errors are permanent; -// anything else is unknown. Both the error code and message are inspected. +// 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. const classify = (err: unknown): FailureClass => { + if (hasTransportCode(err)) return "transient"; const e = err as NodeJS.ErrnoException; - const text = - `${e?.code ?? ""} ${err instanceof Error ? err.message : String(err)}`.toLowerCase(); + 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( text, @@ -241,9 +245,6 @@ const classify = (err: unknown): FailureClass => { return "unknown"; }; -const errorMessage = (err: unknown): string => - err instanceof Error ? err.message : String(err); - // Ensure `linkPath` is a symlink to `target`, rebuilding a missing, wrong, or // non-symlink entry. Throws on failure (a directory in the way, no permission) // so the caller records it and moves on rather than aborting the run. @@ -470,7 +471,7 @@ const imageMetadataFor = async ( const bytes = await readFile(originalPath); return { imageMetadata: extractImageMetadata(bytes) ?? {} }; } catch (err) { - return { imageMetadataError: errorMessage(err) }; + return { imageMetadataError: describeError(err) }; } }; @@ -610,7 +611,7 @@ const runLockedBackup = async ( // and thumbnail both fail this run must not double its attempt count // or appear twice in errors. if (failedThisRun.has(fileID)) return; - const error = errorMessage(err); + const error = describeError(err); errors.push({ fileID, title, collection, error }); const prior = ledger.get(fileID); ledger.set(fileID, { @@ -681,7 +682,7 @@ const runLockedBackup = async ( } } catch (err) { log( - `FAILED verifying original ${file.metadata.title}: ${errorMessage(err)}`, + `FAILED verifying original ${file.metadata.title}: ${describeError(err)}`, ); recordFailure( file.id, @@ -717,7 +718,9 @@ const runLockedBackup = async ( } downloaded++; } catch (err) { - log(`FAILED original ${file.metadata.title}: ${errorMessage(err)}`); + log( + `FAILED original ${file.metadata.title}: ${describeError(err)}`, + ); recordFailure( file.id, file.metadata.title, @@ -785,12 +788,14 @@ const runLockedBackup = async ( // instead and is recorded as failed. The next run fetches its ML data // again because none is cached. if (includeOriginals) { + let mlDataFailure: unknown; let mlDataError: string | undefined; try { log("Fetching ML data..."); await lib.fetchMLData(); } catch (err) { - mlDataError = errorMessage(err); + mlDataFailure = err; + mlDataError = describeError(err); log(`FAILED ML data: ${mlDataError}`); } for (const file of distinct.values()) { @@ -814,7 +819,7 @@ const runLockedBackup = async ( file.id, file.metadata.title, collectionName.get(file.collectionID) ?? "", - new Error(`ML data: ${mlDataError}`), + new Error("ML data", { cause: mlDataFailure }), ); } else { writeSidecar(path, file, { mlData }, image); @@ -846,7 +851,7 @@ const runLockedBackup = async ( downloadDirectory, ); } catch (err) { - log(`FAILED removing old album directories: ${errorMessage(err)}`); + log(`FAILED removing old album directories: ${describeError(err)}`); } for (const c of collections) { @@ -868,7 +873,9 @@ const runLockedBackup = async ( try { removeStaleLinks(colDir, new Set(linkNames), downloadDirectory); } catch (err) { - log(`FAILED removing old links in ${c.name}: ${errorMessage(err)}`); + log( + `FAILED removing old links in ${c.name}: ${describeError(err)}`, + ); } const metaFiles = files.map((f) => ({ @@ -885,7 +892,7 @@ const runLockedBackup = async ( ); } catch (err) { log( - `FAILED symlink ${c.name}/${linkName}: ${errorMessage(err)}`, + `FAILED symlink ${c.name}/${linkName}: ${describeError(err)}`, ); recordFailure( link.file.id, diff --git a/src/cli-run.ts b/src/cli-run.ts index 027bc40..50bf822 100644 --- a/src/cli-run.ts +++ b/src/cli-run.ts @@ -2,14 +2,17 @@ import type { Writable } from "node:stream"; import { ApiError } from "./api/client.js"; +import { describeError } from "./errors.js"; // Run a command and exit with its code once stdout/stderr have drained. // Exiting before the drain can truncate piped output, and the library can keep // the event loop alive after a command returns, so a plain return could hang. // An error the command throws is printed as one `quak: MESSAGE` line, without -// the stack trace, and exits 1. A 401 from the server means it no longer -// accepts the saved session: that prints one line saying to log in again and -// exits 3, as a missing or corrupt session file does. +// the stack trace, and exits 1. MESSAGE is the error as `describeError` gives +// it, so it carries the reason Node's `fetch` puts on `cause`. A 401 from the +// server means it no longer accepts the saved session: that prints one line +// saying to log in again and exits 3, as a missing or corrupt session file +// does. export const run = async ( command: Promise, stdout: Writable, @@ -26,9 +29,7 @@ export const run = async ( ); code = 3; } else { - stderr.write( - `quak: ${err instanceof Error ? err.message : String(err)}\n`, - ); + stderr.write(`quak: ${describeError(err)}\n`); code = 1; } } diff --git a/src/download/index.ts b/src/download/index.ts index e27184b..1e8b30a 100644 --- a/src/download/index.ts +++ b/src/download/index.ts @@ -135,15 +135,17 @@ const streamDecrypt = async ( // cannot tell a partial chunk from a corrupt one, so this // is reported as the truncation it almost always is, with // the authentication failure kept as the error's cause. - // Only the pull is guarded: a sink failure on a chunk - // that did authenticate is a disk error, not a + // The message leaves that failure to the cause, which + // `describeError` appends to what `quak backup` prints and + // records. Only the pull is guarded: a sink failure on a + // chunk that did authenticate is a disk error, not a // truncation. let pulled; try { pulled = pullStreamChunk(state, buffer); } catch (err) { throw new TruncatedStreamError( - `download: stream truncated: response body ended with ${buffer.length} trailing bytes that did not authenticate as a final chunk (transfer stopped mid-chunk, or the data is corrupt)`, + `download: stream truncated: response body ended with ${buffer.length} trailing bytes (transfer stopped mid-chunk, or the data is corrupt)`, { cause: err }, ); } diff --git a/src/errors.ts b/src/errors.ts index e001092..d9970bd 100644 --- a/src/errors.ts +++ b/src/errors.ts @@ -1,10 +1,13 @@ -// Error types that more than one layer of quak needs to recognise. +// Error types that more than one layer of quak needs to recognise, and how an +// error is described in what quak prints and in `failures.json`. // -// They live here rather than beside the code that throws them so that the +// The types live here rather than beside the code that throws them so that the // retry classifier can identify them without importing the HTTP client or the // download layer — both of which import the classifier. `ApiError` is // re-exported from `src/api/client.ts`, which is where callers have always -// imported it from and where it still belongs conceptually. +// imported it from and where it still belongs conceptually. The walk along an +// error's `cause` chain is here too, because the retry classifier reads the +// chain for errnos and `describeError` for messages. export class ApiError extends Error { readonly status: number; @@ -43,3 +46,68 @@ export class TruncatedStreamError extends Error { this.name = "TruncatedStreamError"; } } + +// `cause` is an arbitrary user-settable property and nothing prevents it from +// forming a cycle, so the walk is bounded. Hanging the process would be worse +// than misclassifying an error or describing only part of it. +const MAX_CAUSE_DEPTH = 8; + +// The error and each error on its `cause` chain, top first, at most +// `MAX_CAUSE_DEPTH` of them. `complete` is false when the walk stopped at the +// depth limit with more of the chain still below it. +export const causeChain = ( + err: unknown, +): { chain: object[]; complete: boolean } => { + const chain: object[] = []; + let current: unknown = err; + for (let depth = 0; depth < MAX_CAUSE_DEPTH; depth++) { + if (current === null || typeof current !== "object") { + return { chain, complete: true }; + } + chain.push(current); + const { cause } = current as { cause?: unknown }; + if (cause === current) return { chain, complete: true }; + current = cause; + } + return { chain, complete: current === null || typeof current !== "object" }; +}; + +// An error as quak prints it and records it in `failures.json`: its message, +// then the message of each error on its `cause` chain, joined with ": ". Node's +// `fetch` rejects with just "fetch failed" and puts the reason on `cause`, so +// this gives "fetch failed: connect ECONNREFUSED 1.2.3.4:443". When the host +// has several addresses and every one fails, the cause is an `AggregateError` +// with an empty message and one error per address in `errors`, so an +// `AggregateError` is given as the messages of its errors, joined with ", ". +// A cause that is not an object, such as a thrown string, is given as +// `String(cause)`. Each message is trimmed of the white space around it, as +// OpenSSL ends its own with a newline. A message the text already holds is +// left out. +export const describeError = (err: unknown): string => { + if (!(err instanceof Error)) return String(err); + const { chain } = causeChain(err); + const messages = chain + .slice(1) + .map((cause) => + cause instanceof AggregateError + ? cause.errors + .map((e: unknown) => + (e instanceof Error ? e.message : String(e)).trim(), + ) + .join(", ") + : (cause as { message?: unknown }).message, + ); + // `causeChain` stops at a cause that is not an object, so that one is read + // from the last error on the chain. + const last = chain[chain.length - 1] as { cause?: unknown }; + if (last.cause !== undefined && typeof last.cause !== "object") { + messages.push(String(last.cause)); + } + let text = err.message.trim(); + for (const message of messages) { + if (typeof message !== "string") continue; + const trimmed = message.trim(); + if (!text.includes(trimmed)) text += `: ${trimmed}`; + } + return text; +}; diff --git a/src/retry.ts b/src/retry.ts index ffa68d9..09bb752 100644 --- a/src/retry.ts +++ b/src/retry.ts @@ -10,7 +10,7 @@ // declining to retry a transient one costs a single file that the next run // picks up anyway. When the evidence is ambiguous, fail fast. -import { ApiError, TruncatedStreamError } from "./errors.js"; +import { ApiError, causeChain, TruncatedStreamError } from "./errors.js"; export interface RetryOptions { // Total calls, not retries: `attempts: 1` disables retrying. @@ -89,11 +89,6 @@ const TRANSPORT_CODES = new Set([ // narrowed. See `isSafeToReplay`. const CONNECT_CODES = new Set(["ENOTFOUND", "EAI_AGAIN", "ECONNREFUSED"]); -// `cause` is an arbitrary user-settable property and nothing prevents it from -// forming a cycle, so the walk is bounded. Hanging the process would be a -// worse outcome than any misclassification. -const MAX_CAUSE_DEPTH = 8; - // 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 // `cause`, sometimes more than one level down — so a classifier that only read @@ -101,20 +96,21 @@ const MAX_CAUSE_DEPTH = 8; // connection permanent. `complete` is false when the walk stopped at the // depth limit with more of the chain still below it. const causeCodes = (err: unknown): { codes: string[]; complete: boolean } => { + const { chain, complete } = causeChain(err); const codes: string[] = []; - let current: unknown = err; - for (let depth = 0; depth < MAX_CAUSE_DEPTH; depth++) { - if (current === null || typeof current !== "object") { - return { codes, complete: true }; - } - const { code, cause } = current as { code?: unknown; cause?: unknown }; + for (const link of chain) { + const { code } = link as { code?: unknown }; if (typeof code === "string") codes.push(code); - if (cause === current) return { codes, complete: true }; - current = cause; } - return { codes, complete: current === null || typeof current !== "object" }; + return { codes, complete }; }; +// Does the error's cause chain hold the errno 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 => + causeCodes(err).codes.some((code) => TRANSPORT_CODES.has(code)); + const isAbort = (err: unknown): boolean => { if (err === null || typeof err !== "object") return false; const { name } = err as { name?: unknown }; @@ -158,7 +154,7 @@ export const isRetryable = (err: unknown): boolean => { // have succeeded; the cost of the imprecision is bounded by the attempt // count. if (err instanceof TypeError) return true; - return causeCodes(err).codes.some((code) => TRANSPORT_CODES.has(code)); + return hasTransportCode(err); }; // Could the first attempt already have taken effect on the server? diff --git a/test/cli/backup-end-to-end.test.ts b/test/cli/backup-end-to-end.test.ts index 65bf5b6..367251e 100644 --- a/test/cli/backup-end-to-end.test.ts +++ b/test/cli/backup-end-to-end.test.ts @@ -329,6 +329,41 @@ const downloadOf = (input: RequestInfo | URL): number | undefined => { : undefined; }; +// The error Node gives for a connection to `address` that was refused. +const refusedAt = (address: string): Error => + Object.assign(new Error(`connect ECONNREFUSED ${address}`), { + code: "ECONNREFUSED", + }); + +// What Node's `fetch` rejects with when the server refuses the connection, as +// for every failure below HTTP: a TypeError that says only "fetch failed", +// with the reason, and its errno, as its cause. When the server has several +// addresses and each one refuses, the cause is an AggregateError with an empty +// message, one error per address and the errno of the first. `reason` is the +// error as quak prints it and records it in `failures.json`. +const REFUSED = [ + { + addresses: "its one address", + error: (): TypeError => + new TypeError("fetch failed", { cause: refusedAt("1.2.3.4:443") }), + reason: "fetch failed: connect ECONNREFUSED 1.2.3.4:443", + }, + { + addresses: "each of its two addresses", + error: (): TypeError => + new TypeError("fetch failed", { + cause: Object.assign( + new AggregateError([ + refusedAt("2001:db8::1:443"), + refusedAt("1.2.3.4:443"), + ]), + { code: "ECONNREFUSED" }, + ), + }), + reason: "fetch failed: connect ECONNREFUSED 2001:db8::1:443, connect ECONNREFUSED 1.2.3.4:443", + }, +]; + // The fake server, pushing onto `downloads` the ID of each file whose original // is downloaded. const recordingDownloads = @@ -619,6 +654,115 @@ describe("quak backup", () => { expect(readdirSync(dir)).toEqual([]); }); + // The server refuses every connection, at its one address or at each of + // several. quak tries each request again, here without waiting, and then + // the run stops at the refresh it starts with. The line it prints gives + // the reason Node's `fetch` put on the error's cause, so a cron log tells + // a refused connection from a name that did not resolve, and the exit + // code is 1. + it.each(REFUSED)( + "prints why the refresh could not reach the server at $addresses, and exits 1", + async ({ error, reason }) => { + const dir = mkdtempSync(join(root, "backup-")); + const fetch: typeof globalThis.fetch = async () => { + throw error(); + }; + + const { code, stderr } = await backup(dir, {}, fetch); + + expect(code).toBe(1); + expect(stderr).toBe( + "Starting backup...\n" + + "Refreshing library...\n" + + `quak: ${reason}\n`, + ); + }, + ); + + // The files CDN refuses every connection for toast.mp4, at its one + // address or at each of several. Once its retries are spent, the file + // goes into `failures.json` with the reason, as a transient failure, + // since a later run may well reach the CDN, and the lines the backup + // prints for it give the same reason. The rest of the account is backed + // up, and the exit code is 1. + it.each(REFUSED)( + "records why a download could not reach the server at $addresses in failures.json, as transient, and exits 1", + async ({ error, reason }) => { + let attempts = 0; + const fetch: typeof globalThis.fetch = async (input, init) => { + if (downloadOf(input) === 200) { + attempts++; + throw error(); + } + return server(input, init); + }; + const dir = mkdtempSync(join(root, "backup-")); + + const { code, stderr } = await backup(dir, {}, fetch); + + expect(code).toBe(1); + expect(attempts).toBe(UNATTENDED_RETRY_OPTIONS.attempts); + const failures = readJSON(join(dir, "failures.json")); + expect(Object.keys(failures.files)).toEqual(["200"]); + expect(failures.files["200"]).toMatchObject({ + fileID: 200, + title: "toast.mp4", + classification: "transient", + attempts: 1, + error: reason, + }); + expect(stderr).toContain(`FAILED original toast.mp4: ${reason}\n`); + expect(stderr).toContain( + `[Friend's Wedding] toast.mp4 (id 200): ${reason}\n`, + ); + expect(readFileSync(join(dir, DAY, "2024-03-01.100.jpg"))).toEqual( + Buffer.from(JPEG_WITH_EXIF), + ); + expect(existsSync(join(dir, DAY, "2024-03-01.200.mp4"))).toBe( + false, + ); + }, + ); + + // 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 + // transient failure, as a download that could not reach the server does, + // and the exit code is 1. + it("records why the ML data fetch could not reach the server in failures.json, as transient, and exits 1", async () => { + const fetch: typeof globalThis.fetch = async (input, init) => { + if (new URL(String(input)).pathname === "/files/data/fetch") { + throw new TypeError("fetch failed", { + cause: Object.assign( + new Error("getaddrinfo ENOTFOUND api.ente.io"), + { code: "ENOTFOUND" }, + ), + }); + } + return server(input, init); + }; + const dir = mkdtempSync(join(root, "backup-")); + + const { code, stderr } = await backup(dir, {}, fetch); + + expect(code).toBe(1); + const error = + "ML data: fetch failed: getaddrinfo ENOTFOUND api.ente.io"; + expect(stderr).toContain(`FAILED ${error}\n`); + const failures = readJSON(join(dir, "failures.json")); + expect( + Object.values(failures.files).map((f) => [ + f.fileID, + f.classification, + f.error, + ]), + ).toEqual([ + [100, "transient", error], + [200, "transient", error], + [300, "transient", error], + ]); + }); + // Something changed beach.jpg on disk after it was backed up. A run with // --verify hashes every stored original, finds that one no longer matches // the hash Ente recorded, says so, and downloads it again; it put back the diff --git a/test/cli/run.test.ts b/test/cli/run.test.ts index cfad348..27db619 100644 --- a/test/cli/run.test.ts +++ b/test/cli/run.test.ts @@ -48,6 +48,78 @@ describe("run", () => { ); }); + // Node's `fetch` rejects every failure below HTTP with a TypeError that + // says only "fetch failed", and hangs the reason, with its errno, off + // `cause`. The printed line carries the reason, so a cron log tells a + // refused connection from a name that did not resolve. + it("prints the reason a fetch failed, from the error's cause", async () => { + const refused = new TypeError("fetch failed", { + cause: Object.assign( + new Error("connect ECONNREFUSED 1.2.3.4:443"), + { code: "ECONNREFUSED" }, + ), + }); + const result = await runToExit(Promise.reject(refused)); + expect(result).toEqual({ + code: 1, + stdout: "", + stderr: "quak: fetch failed: connect ECONNREFUSED 1.2.3.4:443\n", + }); + }); + + // When the host has several addresses and every one refuses, Node's + // `fetch` gives as the cause an AggregateError with an empty message and + // one error per address. The printed line gives each address's reason. + it("prints the reason for each address a fetch could not reach", async () => { + const refusedAt = (address: string): Error => + Object.assign(new Error(`connect ECONNREFUSED ${address}`), { + code: "ECONNREFUSED", + }); + const refused = new TypeError("fetch failed", { + cause: Object.assign( + new AggregateError([ + refusedAt("2001:db8::1:443"), + refusedAt("1.2.3.4:443"), + ]), + { code: "ECONNREFUSED" }, + ), + }); + const result = await runToExit(Promise.reject(refused)); + expect(result).toEqual({ + code: 1, + stdout: "", + stderr: "quak: fetch failed: connect ECONNREFUSED 2001:db8::1:443, connect ECONNREFUSED 1.2.3.4:443\n", + }); + }); + + // An error that already quotes its cause's message is printed as it is. + it("does not print a cause's message twice", async () => { + const reset = new Error("read ECONNRESET"); + const wrapped = new Error(`download: file 7: ${reset.message}`, { + cause: reset, + }); + const result = await runToExit(Promise.reject(wrapped)); + expect(result.stderr).toBe("quak: download: file 7: read ECONNRESET\n"); + }); + + // OpenSSL ends its message for a failed TLS connection, such as one to a + // server that answers in plain HTTP, with a newline. The printed line is + // still one line. + it("prints a cause's message without the white space around it", async () => { + const wrongVersion = new TypeError("fetch failed", { + cause: Object.assign( + new Error( + "C0B73A55A47F0000:error:0A00010B:SSL routines:ssl3_get_record:wrong version number:../deps/openssl/openssl/ssl/record/ssl3_record.c:354:\n", + ), + { code: "ERR_SSL_WRONG_VERSION_NUMBER" }, + ), + }); + const result = await runToExit(Promise.reject(wrongVersion)); + expect(result.stderr).toBe( + "quak: fetch failed: C0B73A55A47F0000:error:0A00010B:SSL routines:ssl3_get_record:wrong version number:../deps/openssl/openssl/ssl/record/ssl3_record.c:354:\n", + ); + }); + it("prints a thrown value that is not an Error", async () => { const result = await runToExit(Promise.reject("offline")); expect(result).toEqual({ @@ -57,6 +129,14 @@ describe("run", () => { }); }); + // A cause that is not an Error, such as a thrown string, is printed as a + // thrown value that is not an Error is. + it("prints a cause that is not an Error", async () => { + const wrapped = new Error("ML data", { cause: "offline" }); + const result = await runToExit(Promise.reject(wrapped)); + expect(result.stderr).toBe("quak: ML data: offline\n"); + }); + it("on a 401 from the server says to run quak login, on one line, and exits 3", async () => { const result = await runToExit( Promise.reject(new ApiError("unauthorized", 401)), diff --git a/test/download/download.test.ts b/test/download/download.test.ts index 06788fd..7333b36 100644 --- a/test/download/download.test.ts +++ b/test/download/download.test.ts @@ -74,7 +74,11 @@ import { } from "vitest"; import { init, toBase64, STREAM_CHUNK_SIZE } from "../../src/crypto/index.js"; import { ApiClient } from "../../src/api/client.js"; -import { ApiError, TruncatedStreamError } from "../../src/errors.js"; +import { + ApiError, + describeError, + TruncatedStreamError, +} from "../../src/errors.js"; import type { RetryOptions } from "../../src/retry.js"; import { downloadFile, @@ -834,6 +838,11 @@ describe.each(entryPoints)( expect(((err as Error).cause as Error).message).toMatch( /authentication failed/i, ); + // As quak prints it and records it in `failures.json`, the error + // states the authentication failure once, from its cause. + expect(describeError(err)).toMatch( + /^download: stream truncated: response body ended with \d+ trailing bytes \(transfer stopped mid-chunk, or the data is corrupt\): secretstream chunk authentication failed$/, + ); // And, as with every other failure, the destination is untouched // and no staged temp file survives.