diff --git a/README.md b/README.md index dbd05f2..44c4812 100644 --- a/README.md +++ b/README.md @@ -415,18 +415,24 @@ 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`: +instant. The numbers, 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 | +| Option | Default | `quak backup` | Meaning | +| ------------- | ------- | ------------- | ----------------------------------- | +| `attempts` | `4` | `10` | total calls, not retries | +| `baseDelayMs` | `500` | `1000` | ceiling for the first retry's delay | +| `maxDelayMs` | `10000` | `60000` | 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. +With the defaults a file that is going to fail gives up after at most three and +a half seconds of waiting. `quak backup` usually runs from cron with nobody +watching, so every request it makes uses the `quak backup` column instead, +exported as `UNATTENDED_RETRY_OPTIONS`: a request that keeps failing gives up +after at most 243 seconds of waiting, and usually after about half that, since +each wait is drawn at random below its ceiling. Every other command uses the +defaults. A library user gets the same budget by passing +`UNATTENDED_RETRY_OPTIONS` as `ApiClientOptions.retry`. `sleep` and `random` are +injectable through the same option, which is how the test suite exercises the +whole policy without waiting. Two deadlines, renewed for each attempt: diff --git a/TODO.md b/TODO.md index 59e3228..3721724 100644 --- a/TODO.md +++ b/TODO.md @@ -25,6 +25,14 @@ declares one. # Completed Steps +- 2026-10-06: `quak backup` retries a failed request for longer than the other + commands do (issue 165). `src/retry.ts` exports `UNATTENDED_RETRY_OPTIONS` + beside the unchanged default: 10 attempts, a 1 s base delay and a 60 s cap, so + a request that keeps failing waits at most 243 s before it gives up. + `bin/quak.ts` loads the backup's session with them, so its refresh, ML data + and downloads all use them. What is retried and the backoff formula are + unchanged. + - 2026-10-06: The files this repository copies from `sneak/prompts` are copied again from its commit `dd4027b` (issue 171). `.gitignore` and `.dockerignore` keep out more secret files, and this repository's build artifacts follow the diff --git a/bin/quak.ts b/bin/quak.ts index 4c02183..f974348 100644 --- a/bin/quak.ts +++ b/bin/quak.ts @@ -23,6 +23,7 @@ import { run as runCommand } from "../src/cli-run.js"; import { loadSession } from "../src/cli-session.js"; import { Client } from "../src/client.js"; import { VERSION } from "../src/index.js"; +import { UNATTENDED_RETRY_OPTIONS } from "../src/retry.js"; const paths = envPaths("quak", { suffix: "" }); @@ -129,8 +130,20 @@ program ) .argument("", "Output directory") .option("--json", "Print result as JSON instead of human-readable summary") + // A backup usually runs from cron with nobody watching, so every request + // it makes retries for longer than the other commands' requests do. .action((dir: string, opts: { json?: boolean }) => - run(backupCommand(context(), dir, opts)), + run( + backupCommand( + { + ...context(), + loadSession: (path) => + loadSession(path, { retry: UNATTENDED_RETRY_OPTIONS }), + }, + dir, + opts, + ), + ), ); const helper = program diff --git a/src/index.ts b/src/index.ts index 60d90ee..68abb3b 100644 --- a/src/index.ts +++ b/src/index.ts @@ -27,6 +27,7 @@ export { isRetryable, isSafeToReplay, resolveRetryOptions, + UNATTENDED_RETRY_OPTIONS, withRetry, type ResolvedRetryOptions, type RetryOptions, diff --git a/src/retry.ts b/src/retry.ts index 1198a79..ffa68d9 100644 --- a/src/retry.ts +++ b/src/retry.ts @@ -37,6 +37,16 @@ export const DEFAULT_RETRY_OPTIONS: ResolvedRetryOptions = { random: Math.random, }; +// For a run nobody is watching, such as `quak backup` from cron. A request that +// keeps failing waits at most 243 s in all before it gives up, usually about +// half that, since each wait is drawn at random below its ceiling. +export const UNATTENDED_RETRY_OPTIONS: ResolvedRetryOptions = { + ...DEFAULT_RETRY_OPTIONS, + attempts: 10, + baseDelayMs: 1_000, + maxDelayMs: 60_000, +}; + export const resolveRetryOptions = ( opts?: RetryOptions, ): ResolvedRetryOptions => ({ diff --git a/test/cli/bin.test.ts b/test/cli/bin.test.ts new file mode 100644 index 0000000..bbcbe79 --- /dev/null +++ b/test/cli/bin.test.ts @@ -0,0 +1,71 @@ +/** + * Tests for the retry options `bin/quak.ts` loads the saved session with: + * `quak backup` gets the unattended ones, every other command the default. + * + * Each test runs `bin/quak.ts`, as the smoke test does, with its session loader + * replaced by one that records the options it is given and reports no session, + * so the command stops before it makes any request. + */ + +import { mkdtempSync, rmSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { afterAll, afterEach, describe, expect, it, vi } from "vitest"; +import type { ApiClientOptions } from "../../src/api/client.js"; +import { UNATTENDED_RETRY_OPTIONS } from "../../src/retry.js"; + +const loaded = vi.hoisted(() => [] as (ApiClientOptions | undefined)[]); + +vi.mock("../../src/cli-session.js", () => ({ + loadSession: (_path: string, apiOptions?: ApiClientOptions) => { + loaded.push(apiOptions); + return null; + }, +})); + +const argv = process.argv; +const dir = mkdtempSync(join(tmpdir(), "quak-bin-test-")); + +// Run `quak ` to completion. The "Not logged in" message and the exit +// are swallowed. +const quak = async (...args: string[]): Promise => { + vi.spyOn(process.stderr, "write").mockImplementation(() => true); + vi.spyOn(process, "exit").mockImplementation(() => undefined as never); + process.argv = ["node", "quak", ...args]; + vi.resetModules(); + await import("../../bin/quak.js"); +}; + +afterEach(() => { + process.argv = argv; + loaded.length = 0; + vi.restoreAllMocks(); +}); + +afterAll(() => { + rmSync(dir, { recursive: true, force: true }); +}); + +describe("bin/quak.ts session loading", () => { + it("loads the session for backup with the unattended retry options", async () => { + await quak("backup", dir); + // `vi.resetModules()` gave `bin/quak.ts` its own copy of + // `src/retry.ts`, whose `sleep` is a different function, so the + // options are compared by their numbers. + const { attempts, baseDelayMs, maxDelayMs } = UNATTENDED_RETRY_OPTIONS; + expect(loaded).toEqual([ + { + retry: expect.objectContaining({ + attempts, + baseDelayMs, + maxDelayMs, + }), + }, + ]); + }); + + it("loads the session for another command with the default options", async () => { + await quak("collections"); + expect(loaded).toEqual([undefined]); + }); +}); diff --git a/test/client/session.test.ts b/test/client/session.test.ts index 7aa2359..5a14775 100644 --- a/test/client/session.test.ts +++ b/test/client/session.test.ts @@ -12,6 +12,10 @@ import { afterAll, beforeAll, describe, expect, it } from "vitest"; import { init, toBase64 } from "../../src/crypto/index.js"; import { Client, type ClientSnapshot } from "../../src/client.js"; import { loadSession } from "../../src/cli-session.js"; +import { + DEFAULT_RETRY_OPTIONS, + UNATTENDED_RETRY_OPTIONS, +} from "../../src/retry.js"; const validSnapshot = (): ClientSnapshot => { const kp = sodium.crypto_box_keypair(); @@ -176,6 +180,20 @@ describe("loadSession", () => { }); }); + it("gives the restored client the retry options it is passed", () => { + const path = join(dir, "retry.json"); + writeFileSync(path, JSON.stringify(validSnapshot())); + const retryOptions = (client: Client | null) => + client!.getApiClient().getRetryOptions(); + + expect( + retryOptions( + loadSession(path, { retry: UNATTENDED_RETRY_OPTIONS }), + ), + ).toEqual(UNATTENDED_RETRY_OPTIONS); + expect(retryOptions(loadSession(path))).toEqual(DEFAULT_RETRY_OPTIONS); + }); + it("says the file is corrupt when it is not JSON", () => { const path = join(dir, "truncated.json"); writeFileSync(path, '{"email": "user@exa'); diff --git a/test/retry/retry.test.ts b/test/retry/retry.test.ts index 394d96e..8f2213f 100644 --- a/test/retry/retry.test.ts +++ b/test/retry/retry.test.ts @@ -43,6 +43,7 @@ import { isRetryable, isSafeToReplay, resolveRetryOptions, + UNATTENDED_RETRY_OPTIONS, withRetry, } from "../../src/retry.js"; import { ApiError, TruncatedStreamError } from "../../src/errors.js"; @@ -639,3 +640,33 @@ describe("retry defaults", () => { } }); }); + +describe("unattended retry options", () => { + it("allow ten attempts, a 1 s base delay and a 60 s cap", () => { + // The numbers the README documents for `quak backup`. + expect(UNATTENDED_RETRY_OPTIONS.attempts).toBe(10); + expect(UNATTENDED_RETRY_OPTIONS.baseDelayMs).toBe(1_000); + expect(UNATTENDED_RETRY_OPTIONS.maxDelayMs).toBe(60_000); + }); + + it("give up on a request that keeps failing after at most 243 s of waiting", async () => { + // `random: () => 1` makes every wait its ceiling, the worst case. + const { sleep, delays } = recordingSleep(); + let calls = 0; + await expect( + withRetry( + () => { + calls++; + return Promise.reject(new ApiError("HTTP 503", 503)); + }, + { ...UNATTENDED_RETRY_OPTIONS, sleep, random: () => 1 }, + ), + ).rejects.toThrow("HTTP 503"); + + expect(calls).toBe(10); + expect(delays).toEqual([ + 1_000, 2_000, 4_000, 8_000, 16_000, 32_000, 60_000, 60_000, 60_000, + ]); + expect(delays.reduce((sum, ms) => sum + ms, 0)).toBe(243_000); + }); +});