diff --git a/README.md b/README.md index 0762343..6145269 100644 --- a/README.md +++ b/README.md @@ -169,6 +169,8 @@ quak/ model/ decrypted Collection, File, Metadata types + decrypt fns download/ streaming file/thumbnail download + decryption backup.ts resilient full-account backup with dedup + errors.ts error types shared across layers + retry.ts retry classifier + exponential backoff with jitter thumbnails.ts detect + regenerate missing thumbnails client.ts high-level Client class assembled from the above index.ts public library exports @@ -250,6 +252,87 @@ Endpoints used: - `POST /files/upload-url`: mint a presigned upload URL (for thumbnail repair). - `PUT /files/thumbnail`: register an uploaded thumbnail's object key. +### Retries and timeouts + +Every request in the library goes through one policy, in `src/retry.ts`. A +request is repeated only when repeating it could produce a different answer: + +- `ApiError` with a 5xx status: retried. So are `408` and `429`, the two 4xx + codes that are statements about timing rather than about the request. +- Every other 4xx: not retried. A 404 in particular is an answer, and + `listMissingThumbnails` depends on getting it promptly and once. +- Transport failures — a `fetch` rejection, `ECONNRESET`, `ETIMEDOUT`, a DNS or + TLS failure — and deadline aborts: retried. The errno is looked for in the + error's `cause` chain, because that is where Node's `fetch` puts it. +- A truncated download: retried. +- Anything else, including a secretstream authentication failure that is not + truncation: not retried. The default answer is no. For a backup tool, retrying + a permanent failure spends round trips and delays every remaining file, while + declining to retry a transient one costs a single file that the next run picks + up. + +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 +parallel downloads to one CDN blip does not send them all again at the same +instant. Defaults, configurable through `ApiClientOptions.retry`: + +| Option | Default | Meaning | +| ------------- | ------- | ----------------------------------- | +| `attempts` | `4` | total calls, not retries | +| `baseDelayMs` | `500` | ceiling for the first retry's delay | +| `maxDelayMs` | `10000` | upper bound on that ceiling | + +With those defaults a file that is going to fail gives up after at most three +and a half seconds of waiting. `sleep` and `random` are injectable through the +same option, which is how the test suite exercises the whole policy without +waiting. + +Two deadlines, applied with `AbortSignal.timeout()` and renewed for each +attempt: + +| Option | Default | Applies to | +| ------------------- | -------- | ------------------------------------------- | +| `requestTimeoutMs` | `30000` | `getJSON`, `postJSON`, `putJSON`, `putFile` | +| `downloadTimeoutMs` | `600000` | file and thumbnail body transfers | + +They are separate because one number cannot serve both: a value short enough to +keep a hung API call from stalling a backup would cancel a legitimate +multi-gigabyte download. The download deadline covers the body, not just the +headers — `getFileStream` returns as soon as headers arrive, so a deadline that +only guarded the initial request would leave the same hang one layer down. + +**Non-idempotent requests are not blindly replayed.** `postJSON` and `putJSON` +reach `/users/srp/create-session`, `/users/two-factor/verify` — which consumes +one of a small number of second-factor attempts — and `/files/thumbnail`. They +are retried only on failures that prove no request byte reached the server, +which means the connection was never established (`ECONNREFUSED`, `ENOTFOUND`, +and the like). A 5xx, a mid-flight reset and a deadline are all left to the +caller, because each of them can happen after the server has already acted. +`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 +download layer rather than in `ApiClient`, and that is the common failure for +multi-megabyte photos over a CDN. The secretstream pull state is not resumable +and these endpoints have no Range support, so a retry starts the file over. The +atomic write stays outside the retry, so a download that needed three attempts +still performs exactly one write and one rename. `runBackup` and +`runMetadataBackup` are unchanged: the retry sits below them, and a file that +fails after exhausting it is still logged, counted, and stepped over. + +One imprecision is deliberate and worth knowing about. When a body ends part-way +through a secretstream chunk, Poly1305 fails and carries no framing signal, so a +cut connection and genuinely corrupt bytes are indistinguishable. quak reports +that as truncation, which means it is retried. For a body of more than one chunk +the distinction is real — a chunk that failed while the stream carried on past +it stays an authentication failure and is not retried — but for a single-chunk +body, which is most thumbnails and every small file, a wrong key, server-side +corruption and a mid-chunk cutoff all present alike and all get retried. The +cost is bounded by the attempt count, and it buys never silently keeping a +truncated file. + ### Session handling The `Client` class holds the auth token, master key, secret key, and public key @@ -308,7 +391,7 @@ code is non-zero if any files failed. ## TODO -- [ ] Retry policy: no retry on 4xx, exponential backoff on 5xx and network +- [x] Retry policy: no retry on 4xx, exponential backoff on 5xx and network errors - [ ] Update the API reference section below to match the current implementation - [ ] `make docker` green @@ -333,7 +416,10 @@ are correct. The key types and their actual signatures can be found in: - `src/client.ts`: `Client`, `LoginOptions`, `ClientSnapshot` -- `src/api/client.ts`: `ApiClient`, `ApiClientOptions`, `ApiError` +- `src/api/client.ts`: `ApiClient`, `ApiClientOptions`, `ApiError`, + `StreamOptions` +- `src/errors.ts`: `ApiError`, `TruncatedStreamError` +- `src/retry.ts`: `withRetry`, `isRetryable`, `isSafeToReplay`, `RetryOptions` - `src/auth/types.ts`: `KeyAttributes`, `SRPAttributes`, `AuthorizationResponse`, `LoginChallenge` - `src/model/types.ts`: `Collection`, `EnteFile`, `FileMetadata`, `FileBlob`, diff --git a/TODO.md b/TODO.md index f1e961d..2ae41c8 100644 --- a/TODO.md +++ b/TODO.md @@ -14,12 +14,16 @@ pre-1.0 # Next Step -Implement the download retry policy from the README TODO: no retry on 4xx, -exponential backoff on 5xx and network errors. Apply it to file and thumbnail -downloads, cover it with mock-server tests, and update the README TODO checkbox. +Update the README API reference section to match the current implementation. # Completed Steps +- 2026-08-09: Retry policy: no retry on 4xx (except `408` and `429`), + exponential backoff with full jitter on 5xx, transport failures and truncated + transfers, under per-attempt deadlines that cover the response body as well as + the request. Downloads retry request, stream consumption and decryption as one + unit; `postJSON` and `putJSON` are replayed only when the connection was never + established. - 2026-08-09: Downloads verify the secretstream terminated on `TAG_FINAL` and write output atomically: a truncated body is rejected instead of landing on disk as a short file, and plaintext is staged in a sibling temp file and @@ -45,9 +49,6 @@ downloads, cover it with mock-server tests, and update the README TODO checkbox. # Future Steps -- Retry policy: no retry on 4xx, exponential backoff on 5xx and network errors - (the Next Step). -- Update the README API reference section to match the current implementation. - Make `make docker` green. - Tag v1.0.0. - Future desktop client, separate repo: diff --git a/src/api/client.ts b/src/api/client.ts index 32b1ae7..95ad995 100644 --- a/src/api/client.ts +++ b/src/api/client.ts @@ -1,8 +1,33 @@ +import { ApiError } from "../errors.js"; +import { + isSafeToReplay, + resolveRetryOptions, + withRetry, + type ResolvedRetryOptions, + type RetryOptions, +} from "../retry.js"; + +// `ApiError` is defined in `src/errors.ts` so that the retry classifier can +// recognise it without importing this module, which imports the classifier. +// It is re-exported here because this is where callers have always imported it +// from, and it must remain one class: a second copy would make `instanceof` +// fail in the classifier and every 5xx would look permanent. +export { ApiError }; + const DEFAULT_API_ORIGIN = "https://api.ente.io"; const DEFAULT_FILES_ORIGIN = "https://files.ente.io"; const DEFAULT_THUMBS_ORIGIN = "https://thumbnails.ente.io"; const CLIENT_PACKAGE = "berlin.sneak.quak"; +// Two deadlines rather than one, because a single number cannot serve both +// jobs. Thirty seconds is generous for a JSON call and short enough that a +// hung API connection cannot stall a backup for long. A file body is a +// different shape of problem: the deadline has to cover the whole transfer, +// which for a large video on a slow link is minutes, so a value sane for JSON +// would cancel legitimate downloads. +export const DEFAULT_REQUEST_TIMEOUT_MS = 30_000; +export const DEFAULT_DOWNLOAD_TIMEOUT_MS = 600_000; + export interface ApiClientOptions { apiOrigin?: string; filesOrigin?: string; @@ -10,33 +35,78 @@ export interface ApiClientOptions { authToken?: string; fetch?: typeof globalThis.fetch; userAgent?: string; + retry?: RetryOptions; + requestTimeoutMs?: number; + downloadTimeoutMs?: number; } -export class ApiError extends Error { - readonly status: number; - readonly code?: string; - readonly requestID?: string; - readonly body?: unknown; - constructor( - message: string, - status: number, - opts?: { code?: string; requestID?: string; body?: unknown }, - ) { - super(message); - this.name = "ApiError"; - this.status = status; - this.code = opts?.code; - this.requestID = opts?.requestID; - this.body = opts?.body; - } +export interface StreamOptions { + // Opt out of this client's own retry. Exactly one caller wants that: the + // download layer, which retries the request, the stream consumption and + // the decryption as one unit. Leaving both layers enabled would multiply + // the budgets — four attempts each becoming sixteen requests per file. + retry?: boolean; } +// Enforce a deadline over a response body, not merely over its headers. +// +// `getFileStream` returns as soon as headers arrive; the bytes are pulled +// later, in the download layer. Whether the signal passed to `fetch` also +// tears down the body afterwards is up to the fetch implementation, so this +// wrapper makes it a property of quak instead: every read races the signal, +// and an abort errors the stream with the abort reason — which the retry +// classifier recognises. +const deadlineStream = ( + body: ReadableStream, + signal: AbortSignal, +): ReadableStream => { + const reader = body.getReader(); + let rejectOnAbort: (reason: unknown) => void = () => undefined; + const aborted = new Promise((_resolve, reject) => { + rejectOnAbort = reject; + }); + // The abort may fire when nothing is awaiting `aborted` — after the body + // has been read in full, say. Without this, that rejection would surface + // as an unhandled rejection and take the process down. + void aborted.catch(() => undefined); + + const onAbort = (): void => rejectOnAbort(signal.reason); + if (signal.aborted) onAbort(); + else signal.addEventListener("abort", onAbort, { once: true }); + const release = (): void => signal.removeEventListener("abort", onAbort); + + return new ReadableStream({ + async pull(controller) { + try { + const next = await Promise.race([reader.read(), aborted]); + if (next.done) { + release(); + controller.close(); + return; + } + controller.enqueue(next.value); + } catch (err) { + release(); + await reader.cancel(err).catch(() => undefined); + throw err; + } + }, + async cancel(reason) { + release(); + await reader.cancel(reason); + }, + }); +}; + export class ApiClient { private readonly apiOrigin: string; private readonly isCustomOrigin: boolean; private readonly filesOrigin: string; private readonly thumbsOrigin: string; private readonly _fetch: typeof globalThis.fetch; + private readonly retry: ResolvedRetryOptions; + private readonly requestTimeoutMs: number; + private readonly downloadTimeoutMs: number; private token: string | undefined; constructor(opts?: ApiClientOptions) { @@ -53,6 +123,11 @@ export class ApiClient { opts?.thumbsOrigin ?? DEFAULT_THUMBS_ORIGIN ).replace(/\/+$/, ""); this._fetch = opts?.fetch ?? globalThis.fetch; + this.retry = resolveRetryOptions(opts?.retry); + this.requestTimeoutMs = + opts?.requestTimeoutMs ?? DEFAULT_REQUEST_TIMEOUT_MS; + this.downloadTimeoutMs = + opts?.downloadTimeoutMs ?? DEFAULT_DOWNLOAD_TIMEOUT_MS; this.token = opts?.authToken; } @@ -64,6 +139,13 @@ export class ApiClient { this.token = undefined; } + // The policy this client was configured with, so that a caller wrapping a + // whole operation in its own `withRetry` — the download layer — runs under + // the same settings rather than under the library defaults. + getRetryOptions(): ResolvedRetryOptions { + return this.retry; + } + private headers(extra?: Record): Record { const h: Record = { "X-Client-Package": CLIENT_PACKAGE, @@ -128,38 +210,53 @@ export class ApiClient { } } } - const resp = await this._fetch(url.href, { - method: "GET", - headers: this.headers(), - }); - await this.throwIfError(resp); - return (await resp.json()) as T; + // A GET changes nothing, so it is retried under the full policy. + return withRetry(async () => { + const resp = await this._fetch(url.href, { + method: "GET", + headers: this.headers(), + signal: AbortSignal.timeout(this.requestTimeoutMs), + }); + await this.throwIfError(resp); + return (await resp.json()) as T; + }, this.retry); } async postJSON(path: string, body: unknown): Promise { const url = `${this.apiOrigin}${path}`; - const resp = await this._fetch(url, { - method: "POST", - headers: this.headers({ "Content-Type": "application/json" }), - body: JSON.stringify(body), - }); - await this.throwIfError(resp); - return (await resp.json()) as T; + // Idempotency: this reaches `/users/srp/create-session`, + // `/users/two-factor/verify` and `/users/ott`, all of which change + // server state — verifying a second factor consumes one of a small + // number of attempts. So a POST is replayed only when the failure + // proves the request never reached the server, which in practice means + // the connection was never established. A 5xx, a mid-flight reset and + // a timeout are all left to the caller, because each of them can occur + // after the server has already acted. + return withRetry( + async () => { + const resp = await this._fetch(url, { + method: "POST", + headers: this.headers({ + "Content-Type": "application/json", + }), + body: JSON.stringify(body), + signal: AbortSignal.timeout(this.requestTimeoutMs), + }); + await this.throwIfError(resp); + return (await resp.json()) as T; + }, + { ...this.retry, isRetryable: isSafeToReplay }, + ); } - async getFileStream(fileID: number): Promise> { + async getFileStream( + fileID: number, + opts?: StreamOptions, + ): Promise> { const url = this.isCustomOrigin ? `${this.apiOrigin}/files/download/${fileID}` : `${this.filesOrigin}/?fileID=${fileID}`; - const resp = await this._fetch(url, { - method: "GET", - headers: this.headers(), - }); - await this.throwIfError(resp); - if (!resp.body) { - throw new Error("response body is null"); - } - return resp.body; + return this.streamRequest(url, opts); } async getUploadURL( @@ -173,28 +270,52 @@ export class ApiClient { } async putFile(presignedURL: string, data: Uint8Array): Promise { - const resp = await this._fetch(presignedURL, { - method: "PUT", - headers: { - "Content-Type": "application/octet-stream", - "Content-Length": String(data.length), - }, - body: data, - }); - if (!resp.ok) { - throw new Error(`PUT to presigned URL failed: HTTP ${resp.status}`); - } + // Idempotent despite being a write: a presigned PUT stores one whole + // object at one key in one request, so replaying it either overwrites + // the same bytes or lands them for the first time. There is no partial + // state to protect, hence the full policy rather than the POST rule. + await withRetry(async () => { + const resp = await this._fetch(presignedURL, { + method: "PUT", + headers: { + "Content-Type": "application/octet-stream", + "Content-Length": String(data.length), + }, + body: data, + signal: AbortSignal.timeout(this.requestTimeoutMs), + }); + if (!resp.ok) { + // An ApiError, not a bare Error: without the status on the + // error the upload path cannot be classified at all, and a + // 503 from S3 would be indistinguishable from a bug. + throw new ApiError( + `PUT to presigned URL failed: HTTP ${resp.status}`, + resp.status, + ); + } + }, this.retry); } async putJSON(path: string, body: unknown): Promise { const url = `${this.apiOrigin}${path}`; - const resp = await this._fetch(url, { - method: "PUT", - headers: this.headers({ "Content-Type": "application/json" }), - body: JSON.stringify(body), - }); - await this.throwIfError(resp); - return (await resp.json()) as T; + // Same idempotency rule as `postJSON`, for the same reason: this + // reaches `/files/thumbnail`, which registers an uploaded thumbnail + // against a file. + return withRetry( + async () => { + const resp = await this._fetch(url, { + method: "PUT", + headers: this.headers({ + "Content-Type": "application/json", + }), + body: JSON.stringify(body), + signal: AbortSignal.timeout(this.requestTimeoutMs), + }); + await this.throwIfError(resp); + return (await resp.json()) as T; + }, + { ...this.retry, isRetryable: isSafeToReplay }, + ); } async updateThumbnail( @@ -210,18 +331,36 @@ export class ApiClient { async getThumbnailStream( fileID: number, + opts?: StreamOptions, ): Promise> { const url = this.isCustomOrigin ? `${this.apiOrigin}/files/preview/${fileID}` : `${this.thumbsOrigin}/?fileID=${fileID}`; - const resp = await this._fetch(url, { - method: "GET", - headers: this.headers(), - }); - await this.throwIfError(resp); - if (!resp.body) { - throw new Error("response body is null"); - } - return resp.body; + return this.streamRequest(url, opts); + } + + private async streamRequest( + url: string, + opts?: StreamOptions, + ): Promise> { + const once = async (): Promise> => { + // A fresh deadline per attempt, so a retry gets the whole budget + // rather than the remainder of the one that just expired. + const signal = AbortSignal.timeout(this.downloadTimeoutMs); + const resp = await this._fetch(url, { + method: "GET", + headers: this.headers(), + signal, + }); + await this.throwIfError(resp); + if (!resp.body) { + // Carries the status, and is not retryable: a response that + // arrived without a body is malformed, and asking again + // produces the same malformed response. + throw new ApiError("response body is null", resp.status); + } + return deadlineStream(resp.body, signal); + }; + return opts?.retry === false ? once() : withRetry(once, this.retry); } } diff --git a/src/download/index.ts b/src/download/index.ts index 6e7a6b1..bda3ba7 100644 --- a/src/download/index.ts +++ b/src/download/index.ts @@ -9,6 +9,8 @@ import { STREAM_CHUNK_SIZE, streamTagFinal, } from "../crypto/index.js"; +import { TruncatedStreamError } from "../errors.js"; +import { withRetry } from "../retry.js"; import type { ApiClient } from "../api/client.js"; import type { EnteFile } from "../model/types.js"; @@ -65,7 +67,7 @@ const streamDecrypt = async ( try { pulled = pullStreamChunk(state, buffer); } catch (err) { - throw new Error( + 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)`, { cause: err }, ); @@ -85,13 +87,13 @@ const streamDecrypt = async ( // Returning a short plaintext here would put a corrupt file on disk that // later backup runs would treat as complete. if (chunksPulled === 0) { - throw new Error( + throw new TruncatedStreamError( "download: stream truncated: response body contained no secretstream chunks", ); } const tagFinal = streamTagFinal(); if (lastTag !== tagFinal) { - throw new Error( + throw new TruncatedStreamError( `download: stream truncated: last chunk tag ${lastTag}, expected TAG_FINAL (${tagFinal})`, ); } @@ -128,15 +130,49 @@ const writeAtomic = async ( } }; +// Fetch a stream and decrypt it, retrying the whole sequence. +// +// The request is only the first third of a download. `getXStream` returns as +// soon as headers arrive, and the bytes are pulled here, so a socket reset +// mid-body — the dominant failure mode for multi-megabyte photos over a CDN — +// throws in `streamDecrypt` and never reaches `ApiClient` at all. Retrying the +// request alone would miss it entirely. +// +// The client's own retry is therefore switched off for these two calls: with +// both layers active the budgets would multiply, and the library default of +// four attempts would mean sixteen requests for one file. The policy comes +// from the client so a caller that configured one gets it here too. +// +// A retry starts the file over from byte zero: the secretstream pull state is +// not resumable and there is no Range support on these endpoints. +const fetchAndDecrypt = async ( + api: ApiClient, + openStream: () => Promise>, + header: Uint8Array, + key: Uint8Array, +): Promise => + withRetry(async () => { + const stream = await openStream(); + return streamDecrypt(stream, header, key); + }, api.getRetryOptions()); + export const downloadFile = async ( api: ApiClient, file: EnteFile, outPath?: string, ): Promise => { const resolvedPath = outPath ?? file.metadata.title; - const stream = await api.getFileStream(file.id); const header = fromBase64(file.file.decryptionHeader); - const plaintext = await streamDecrypt(stream, header, file.key); + const plaintext = await fetchAndDecrypt( + api, + () => api.getFileStream(file.id, { retry: false }), + header, + file.key, + ); + // Outside the retry, deliberately: only the attempt that produced a + // complete, authenticated plaintext gets to stage a temporary file, so a + // download that needed three tries still performs exactly one write and + // one rename. await writeAtomic(resolvedPath, plaintext); return { path: resolvedPath, bytesWritten: plaintext.length }; }; @@ -147,9 +183,13 @@ export const downloadThumbnail = async ( outPath?: string, ): Promise => { const resolvedPath = outPath ?? `thumb_${file.metadata.title}`; - const stream = await api.getThumbnailStream(file.id); const header = fromBase64(file.thumbnail.decryptionHeader); - const plaintext = await streamDecrypt(stream, header, file.key); + const plaintext = await fetchAndDecrypt( + api, + () => api.getThumbnailStream(file.id, { retry: false }), + header, + file.key, + ); await writeAtomic(resolvedPath, plaintext); return { path: resolvedPath, bytesWritten: plaintext.length }; }; diff --git a/src/errors.ts b/src/errors.ts new file mode 100644 index 0000000..e001092 --- /dev/null +++ b/src/errors.ts @@ -0,0 +1,45 @@ +// Error types that more than one layer of quak needs to recognise. +// +// They 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. + +export class ApiError extends Error { + readonly status: number; + readonly code?: string; + readonly requestID?: string; + readonly body?: unknown; + constructor( + message: string, + status: number, + opts?: { code?: string; requestID?: string; body?: unknown }, + ) { + super(message); + this.name = "ApiError"; + this.status = status; + this.code = opts?.code; + this.requestID = opts?.requestID; + this.body = opts?.body; + } +} + +// A response body that ended before the secretstream did. +// +// This is a type rather than a message prefix because it is a decision, not a +// diagnostic: the retry policy asks "was this a short transfer?" and acts on +// the answer. Matching on the wording of an error message would make the next +// person to reword a diagnostic silently turn every truncated download into a +// permanent failure, and the failure would look like a corrupt file rather +// than like a bug. +// +// `cause` carries the underlying authentication failure on the one path where +// there is one — a body that stopped part-way through a chunk, which Poly1305 +// cannot distinguish from corruption. +export class TruncatedStreamError extends Error { + constructor(message: string, opts?: ErrorOptions) { + super(message, opts); + this.name = "TruncatedStreamError"; + } +} diff --git a/src/index.ts b/src/index.ts index d1ae701..c53ceb9 100644 --- a/src/index.ts +++ b/src/index.ts @@ -1,7 +1,25 @@ export const VERSION = "0.0.0"; export { Client, type LoginOptions, type ClientSnapshot } from "./client.js"; -export { ApiClient, ApiError, type ApiClientOptions } from "./api/client.js"; +export { + ApiClient, + ApiError, + DEFAULT_DOWNLOAD_TIMEOUT_MS, + DEFAULT_REQUEST_TIMEOUT_MS, + type ApiClientOptions, + type StreamOptions, +} from "./api/client.js"; +export { TruncatedStreamError } from "./errors.js"; +export { + DEFAULT_RETRY_OPTIONS, + isRetryable, + isSafeToReplay, + resolveRetryOptions, + withRetry, + type ResolvedRetryOptions, + type RetryOptions, + type WithRetryOptions, +} from "./retry.js"; export { unwrapAuth, type UnwrapResult } from "./auth/unwrap.js"; export { beginLogin, diff --git a/src/retry.ts b/src/retry.ts new file mode 100644 index 0000000..c425de8 --- /dev/null +++ b/src/retry.ts @@ -0,0 +1,193 @@ +// The retry policy shared by every network operation in quak. +// +// Two independent pieces: a classifier that decides whether an error is worth +// another attempt, and a loop that acts on that decision with exponential +// backoff. Keeping them apart is what lets the non-idempotent call sites reuse +// the loop under a stricter question (see `isSafeToReplay`). +// +// The classifier's default answer is no. For a backup tool, retrying a +// permanent failure spends round trips and delays every remaining file, while +// 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"; + +export interface RetryOptions { + // Total calls, not retries: `attempts: 1` disables retrying. + attempts?: number; + // Ceiling for the first retry's delay; doubles with each retry. + baseDelayMs?: number; + // Upper bound on that ceiling, so a long outage settles into a steady + // poll instead of growing without limit. + maxDelayMs?: number; + // Injected so tests exercise the whole policy without waiting. + sleep?: (ms: number) => Promise; + // Injected so the jitter is reproducible under test. + random?: () => number; +} + +export type ResolvedRetryOptions = Required; + +export const DEFAULT_RETRY_OPTIONS: ResolvedRetryOptions = { + attempts: 4, + baseDelayMs: 500, + maxDelayMs: 10_000, + sleep: (ms: number): Promise => + new Promise((resolve) => setTimeout(resolve, ms)), + random: Math.random, +}; + +export const resolveRetryOptions = ( + opts?: RetryOptions, +): ResolvedRetryOptions => ({ + attempts: opts?.attempts ?? DEFAULT_RETRY_OPTIONS.attempts, + baseDelayMs: opts?.baseDelayMs ?? DEFAULT_RETRY_OPTIONS.baseDelayMs, + maxDelayMs: opts?.maxDelayMs ?? DEFAULT_RETRY_OPTIONS.maxDelayMs, + sleep: opts?.sleep ?? DEFAULT_RETRY_OPTIONS.sleep, + random: opts?.random ?? DEFAULT_RETRY_OPTIONS.random, +}); + +// Transport failures: the request did not complete, for reasons below HTTP. +const TRANSPORT_CODES = new Set([ + "ECONNRESET", + "ECONNABORTED", + "ETIMEDOUT", + "EPIPE", + "ENOTFOUND", + "EAI_AGAIN", + "ECONNREFUSED", + "EHOSTUNREACH", + "ENETUNREACH", + "ENETRESET", + "ENETDOWN", +]); + +// The subset of the above that can only happen before any request byte was +// written: name resolution failed, or the connection was refused or never +// routed. See `isSafeToReplay`. +const CONNECT_CODES = new Set([ + "ENOTFOUND", + "EAI_AGAIN", + "ECONNREFUSED", + "EHOSTUNREACH", + "ENETUNREACH", + "ENETDOWN", +]); + +// `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 +// the top-level error would see a bare `Error` and call every dropped +// connection permanent. +const causeCodes = (err: unknown): string[] => { + const codes: string[] = []; + let current: unknown = err; + for (let depth = 0; depth < MAX_CAUSE_DEPTH; depth++) { + if (current === null || typeof current !== "object") break; + const { code, cause } = current as { code?: unknown; cause?: unknown }; + if (typeof code === "string") codes.push(code); + if (cause === current) break; + current = cause; + } + return codes; +}; + +const isAbort = (err: unknown): boolean => { + if (err === null || typeof err !== "object") return false; + const { name } = err as { name?: unknown }; + return name === "AbortError" || name === "TimeoutError"; +}; + +// Is another attempt capable of producing a different answer? +// +// A note on the truncation case, recorded on issue #2. `TruncatedStreamError` +// is retried, and for a body of more than one chunk that is exactly right: a +// chunk that failed to authenticate while the stream carried on past it is +// corruption, stays an ordinary authentication failure, and is not retried. +// +// For a single-chunk body — most thumbnails, every small file — the split is +// not achievable. A wrong file key, server-side corruption and a connection +// cut mid-chunk are cryptographically identical: Poly1305 fails and carries no +// framing signal. All three are reported as truncation and therefore retried. +// That imprecision is deliberate and bounded by the attempt count: one wasted +// round trip on a genuinely corrupt file is a fair price for never silently +// keeping a truncated one, and the alternative — treating a single-chunk +// authentication failure as permanent — would reintroduce exactly the +// silent-corruption risk the truncation check exists to remove. +export const isRetryable = (err: unknown): boolean => { + if (err instanceof ApiError) { + // 408 and 429 are the two 4xx codes that are statements about timing + // rather than about the request, and backoff is the right answer to + // both. Every other 4xx will answer the same way however often it is + // asked. + if (err.status === 408 || err.status === 429) return true; + return err.status >= 500 && err.status <= 599; + } + if (err instanceof TruncatedStreamError) return true; + // quak only ever aborts a request on its own deadline, so an abort means + // this attempt ran out of time — which a later one may not. + if (isAbort(err)) return true; + // Node's fetch rejects with `TypeError: fetch failed` for everything below + // HTTP: DNS failure, refused connection, TLS error, reset socket. Nothing + // on the object separates it from a TypeError thrown by a bug, so this is + // 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. + if (err instanceof TypeError) return true; + return causeCodes(err).some((code) => TRANSPORT_CODES.has(code)); +}; + +// Could the first attempt already have taken effect on the server? +// +// `isRetryable` is the wrong question for a request that changes state. +// quak's non-idempotent calls are `/users/srp/create-session`, +// `/users/two-factor/verify` — which consumes one of a small number of 2FA +// attempts — and `/files/thumbnail`. They are replayed only when the failure +// proves no request byte reached the server, which means the connection was +// never established. +// +// 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 deadline says nothing at all about the server's state. +export const isSafeToReplay = (err: unknown): boolean => + isRetryable(err) && causeCodes(err).some((code) => CONNECT_CODES.has(code)); + +export interface WithRetryOptions extends RetryOptions { + isRetryable?: (err: unknown) => boolean; +} + +// Exponential backoff with full jitter: the exponential term is the ceiling, +// and the actual wait is drawn uniformly below it. Full jitter, rather than a +// fixed delay plus noise, is what stops a client that lost a hundred parallel +// downloads to one CDN blip from re-sending all hundred at the same instant. +const backoffMs = (retryNumber: number, policy: ResolvedRetryOptions): number => + policy.random() * + Math.min(policy.maxDelayMs, policy.baseDelayMs * 2 ** (retryNumber - 1)); + +export const withRetry = async ( + fn: () => Promise, + opts?: WithRetryOptions, +): Promise => { + const policy = resolveRetryOptions(opts); + const retryable = opts?.isRetryable ?? isRetryable; + for (let attempt = 1; ; attempt++) { + try { + return await fn(); + } catch (err) { + if (attempt >= policy.attempts || !retryable(err)) { + // The error escapes unwrapped, and it is the one from the + // final attempt: callers classify what they catch, and + // `listMissingThumbnails` in particular needs the `ApiError` + // and its status intact. + throw err; + } + await policy.sleep(backoffMs(attempt, policy)); + } + } +}; diff --git a/src/thumbnails.ts b/src/thumbnails.ts index 4176f5e..254f53d 100644 --- a/src/thumbnails.ts +++ b/src/thumbnails.ts @@ -2,6 +2,7 @@ import { createHash } from "node:crypto"; import { readFileSync } from "node:fs"; import * as jpeg from "jpeg-js"; import type { Client } from "./client.js"; +import { ApiError } from "./api/client.js"; import { encryptBlob, toBase64 } from "./crypto/index.js"; import { downloadFile } from "./download/index.js"; import type { EnteFile } from "./model/types.js"; @@ -62,13 +63,31 @@ export const listMissingThumbnails = async ( reason: "empty thumbnail (0 bytes)", }); } - } catch { - missing.push({ - fileID: file.id, - title: file.metadata.title, - collection: col.name, - reason: "thumbnail fetch failed", - }); + } catch (err) { + // A 404 is the server stating the thumbnail is not there: + // that, and an empty body, are the only two answers that mean + // "missing". Anything else reaching this point is a failure + // that already exhausted its retries — a failing server, a + // dropped connection, a deadline — and says nothing about + // whether the thumbnail exists. + // + // The distinction is what stops `helper + // fix-missing-thumbnails` from downloading originals, + // regenerating thumbnails and uploading them over thumbnails + // that were fine all along, because the CDN was briefly + // returning 500s while this ran. + if (err instanceof ApiError && err.status === 404) { + missing.push({ + fileID: file.id, + title: file.metadata.title, + collection: col.name, + reason: "thumbnail not found (HTTP 404)", + }); + } else { + log( + `[${col.name}] Could not check ${file.metadata.title}: ${err instanceof Error ? err.message : String(err)} (not reported as missing)`, + ); + } } } } diff --git a/test/retry/retry.test.ts b/test/retry/retry.test.ts index 42eefed..7ddd24b 100644 --- a/test/retry/retry.test.ts +++ b/test/retry/retry.test.ts @@ -522,11 +522,13 @@ describe("withRetry backoff", () => { describe("retry defaults", () => { it("ships a bounded, documented default policy", () => { // These are the numbers the README documents. They are asserted here - // so the README and the code cannot drift apart silently: four - // attempts, half a second of base delay, ten seconds of ceiling — - // under 15 seconds of waiting in the worst case, which keeps a + // so the README and the code cannot drift apart silently. Four + // attempts at a 500ms base put the three ceilings at 500, 1000 and + // 2000ms, so a file that is going to fail gives up after at most + // three and a half seconds of waiting — which keeps a // several-thousand-file backup moving past a bad file rather than - // stalling on it. + // stalling on it. The 10s cap only comes into play for a caller that + // raises the attempt count. expect(DEFAULT_RETRY_OPTIONS.attempts).toBe(4); expect(DEFAULT_RETRY_OPTIONS.baseDelayMs).toBe(500); expect(DEFAULT_RETRY_OPTIONS.maxDelayMs).toBe(10_000);