Retry transient network failures with exponential backoff (closes #2)
check / check (push) Successful in 20s
check / check (push) Successful in 20s
No retry on 4xx, backoff on 5xx and transport failures, and a deadline on every request. Before this, one transient 503 or TCP reset failed a file for good, and a CDN connection that went quiet after accepting the request blocked `quak backup` forever, because there was no timeout anywhere. src/retry.ts holds the policy: a classifier that decides whether another attempt could produce a different answer, and a loop that acts on it with exponential backoff and full jitter. Retried: 5xx, 408, 429, transport failures (the errno is read out of the cause chain, which is where Node's fetch puts it), deadline aborts, and truncated transfers. Not retried: every other 4xx, and anything unrecognised — a wrongly retried permanent failure delays every remaining file, while a wrongly abandoned transient one costs a single file the next run picks up. Attempt count, delays, sleep and jitter source are all configurable through ApiClientOptions; sleep being injectable is what lets the suite exercise the policy without waiting. Truncation needed a type before it could be classified. streamDecrypt threw plain Errors whose messages began "download: stream truncated", and classifying on message text would mean the next reword silently turned every truncated download into a permanent failure. It now throws TruncatedStreamError, which lives in src/errors.ts alongside ApiError so the classifier can recognise both without importing the modules that import it; api/client.ts re-exports ApiError, so it stays one class and every existing import path still resolves. Downloads retry the request, the stream consumption and the decryption together. Only the first of those happens inside ApiClient: a socket reset after the headers arrived throws in streamDecrypt, and retrying the request alone would never see it. The client's own retry is switched off for those two calls so the budgets do not multiply into sixteen requests per file, and the atomic write stays outside the loop so a download that took three attempts still performs one write and one rename. Non-idempotent requests are not blindly replayed. postJSON and putJSON reach create-session, two-factor/verify — which burns one of a few second-factor attempts — and files/thumbnail, so they retry only when the connection was never established and the server provably never saw the request. putFile is exempt and retries fully: a presigned PUT stores one whole object at one key, with no partial state to damage. It now throws ApiError with the status, as do the two null-body paths, which previously threw bare Errors that nothing could classify. Timeouts come from AbortSignal.timeout(), renewed per attempt: 30s for JSON and upload calls, 10 minutes for file bodies, since a value short enough to keep a hung API call from stalling a backup would cancel a legitimate multi-gigabyte download. The download deadline is enforced over the body rather than only the headers, by racing each read against the signal, so the guarantee does not depend on the fetch implementation tearing down a stream it already handed over. listMissingThumbnails now separates a genuine 404 from an exhausted retry. Its bare catch reported both as missing, which after this change would have let a few minutes of 500s talk fix-missing-thumbnails into regenerating and re-uploading thumbnails that were fine. runBackup and runMetadataBackup are untouched: the retry sits below them and their per-file resilience is unchanged.
This commit is contained in:
+193
@@ -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<void>;
|
||||
// Injected so the jitter is reproducible under test.
|
||||
random?: () => number;
|
||||
}
|
||||
|
||||
export type ResolvedRetryOptions = Required<RetryOptions>;
|
||||
|
||||
export const DEFAULT_RETRY_OPTIONS: ResolvedRetryOptions = {
|
||||
attempts: 4,
|
||||
baseDelayMs: 500,
|
||||
maxDelayMs: 10_000,
|
||||
sleep: (ms: number): Promise<void> =>
|
||||
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 <T>(
|
||||
fn: () => Promise<T>,
|
||||
opts?: WithRetryOptions,
|
||||
): Promise<T> => {
|
||||
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));
|
||||
}
|
||||
}
|
||||
};
|
||||
Reference in New Issue
Block a user