diff --git a/README.md b/README.md index 44c4812..bf1d9ac 100644 --- a/README.md +++ b/README.md @@ -517,8 +517,17 @@ The CLI stores the snapshot at the platform-appropriate data directory via `0600`. The key material is stored in cleartext in the JSON; treat this file as you would treat the password itself. A missing file is reported as "not logged in"; a file that exists but is corrupt is reported as such, naming the bad -field. Both exit with status 1, except that `quak logout` with no file says -there is no session and exits 0. +field. When the refresh a command starts with gets HTTP 401 from the server, +because it no longer accepts the saved session's token, the command prints one +line, `quak: the saved session is no longer valid; run "quak login"`, with no +stack trace. All three exit with status 3, which means the user must run +`quak login` again. `quak logout` is the exception: with no file it says there +is no session and exits 0, and it handles a corrupt file or a failed server call +as described below. `quak backup` meets an expired session on the refresh that +starts every run, before it touches any file. A session that stops working +partway through a backup instead fails each remaining file into `failures.json`, +so that run exits 1 and the next one stops at its refresh with status 3. No +command but `quak login` ever prompts. `quak logout` ends the session on the server, so the token in `session.json` stops working even in a copy of the file, and then deletes the file. If the @@ -552,8 +561,9 @@ library. The read commands — `collections`, `files`, `get`, `get-thumb`, `helper fix-missing-thumbnails` — force a fresh server round-trip before they answer, so they report current account state rather than whatever the cache last held. If that round-trip fails, the command prints the error on one line and -exits 1. `--cache-dir` overrides where the cache lives; without it each account -gets its own directory under the per-user cache path. +exits 1, or 3 when the server no longer accepts the saved session (see "Session +handling"). `--cache-dir` overrides where the cache lives; without it each +account gets its own directory under the per-user cache path. `get` and `get-thumb` resolve the file by ID directly, so `--collection` is accepted for backward compatibility but ignored. For a live photo, `get` writes diff --git a/TODO.md b/TODO.md index 3721724..3fb0506 100644 --- a/TODO.md +++ b/TODO.md @@ -25,6 +25,16 @@ declares one. # Completed Steps +- 2026-10-06: When the server answers HTTP 401 and that ends a command that + loads the saved session, because the server no longer accepts its token, the + command prints one line, + `quak: the saved session is no longer valid; run "quak login"`, and exits 3 + (issue 164). `quak backup` meets that 401 on the refresh it starts with, + before it touches any file. For the same commands, a missing or corrupt + session file keeps its message and also exits 3, so a cron job can tell that + the user must log in again. `quak logout` is unchanged. `run` in + `src/cli-run.ts` recognises the 401, which reaches it unchanged. + - 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 diff --git a/src/cli-commands.ts b/src/cli-commands.ts index 3ea8f34..30aa4ee 100644 --- a/src/cli-commands.ts +++ b/src/cli-commands.ts @@ -73,7 +73,9 @@ export const saveSession = ( ); }; -// The saved client, or undefined after telling the user why there is none. +// The saved client, or undefined after telling the user why there is none. The +// command then exits 3, the code for "log in again", as `run` in `cli-run.ts` +// does when the server no longer accepts the saved session. const requireSession = (ctx: CliContext): Client | undefined => { let client: Client | null; try { @@ -150,7 +152,7 @@ export const loginCommand = async (ctx: CliContext): Promise => { export const whoamiCommand = async (ctx: CliContext): Promise => { await init(); const client = requireSession(ctx); - if (!client) return 1; + if (!client) return 3; const info = client.whoami(); ctx.stdout.write(JSON.stringify(info) + "\n"); return 0; @@ -201,7 +203,7 @@ export const collectionsCommand = async ( ): Promise => { await init(); const client = requireSession(ctx); - if (!client) return 1; + if (!client) return 3; const lib = await openReadLibrary(ctx, client); try { // Force a server round-trip and list in enumeration order (issue #36 @@ -243,7 +245,7 @@ export const filesCommand = async ( ): Promise => { await init(); const client = requireSession(ctx); - if (!client) return 1; + if (!client) return 3; const collectionID = Number(opts.collection); if (!Number.isFinite(collectionID)) { ctx.stderr.write("Invalid collection ID\n"); @@ -285,7 +287,7 @@ export const getCommand = async ( ): Promise => { await init(); const client = requireSession(ctx); - if (!client) return 1; + if (!client) return 3; const fileID = Number(fileIDStr); if (!Number.isFinite(fileID)) { ctx.stderr.write("Invalid file ID\n"); @@ -343,7 +345,7 @@ export const getThumbCommand = async ( ): Promise => { await init(); const client = requireSession(ctx); - if (!client) return 1; + if (!client) return 3; const fileID = Number(fileIDStr); if (!Number.isFinite(fileID)) { ctx.stderr.write("Invalid file ID\n"); @@ -380,7 +382,7 @@ export const backupMetadataCommand = async ( ): Promise => { await init(); const client = requireSession(ctx); - if (!client) return 1; + if (!client) return 3; const lib = await openReadLibrary(ctx, client); try { // Refresh first so the dump holds current account state, not what the @@ -403,7 +405,7 @@ export const backupCommand = async ( ): Promise => { await init(); const client = requireSession(ctx); - if (!client) return 1; + if (!client) return 3; ctx.stderr.write("Starting backup...\n"); // The precache is off: the backup fetches what it needs, and must not @@ -453,7 +455,7 @@ export const listMissingThumbnailsCommand = async ( ): Promise => { await init(); const client = requireSession(ctx); - if (!client) return 1; + if (!client) return 3; const lib = await openReadLibrary(ctx, client); try { // Refresh first so files added since the cache was written are @@ -491,7 +493,7 @@ export const fixMissingThumbnailsCommand = async ( ): Promise => { await init(); const client = requireSession(ctx); - if (!client) return 1; + if (!client) return 3; const lib = await openReadLibrary(ctx, client); try { // Refresh first so files added since the cache was written are found; diff --git a/src/cli-run.ts b/src/cli-run.ts index c668b49..027bc40 100644 --- a/src/cli-run.ts +++ b/src/cli-run.ts @@ -1,12 +1,15 @@ // Runs one CLI command for `bin/quak.ts` and exits with its code. import type { Writable } from "node:stream"; +import { ApiError } from "./api/client.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. +// 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. export const run = async ( command: Promise, stdout: Writable, @@ -17,10 +20,17 @@ export const run = async ( try { code = await command; } catch (err) { - stderr.write( - `quak: ${err instanceof Error ? err.message : String(err)}\n`, - ); - code = 1; + if (err instanceof ApiError && err.status === 401) { + stderr.write( + `quak: the saved session is no longer valid; run "quak login"\n`, + ); + code = 3; + } else { + stderr.write( + `quak: ${err instanceof Error ? err.message : String(err)}\n`, + ); + code = 1; + } } const pending = [stdout, stderr].filter((s) => s.writableLength > 0); if (pending.length === 0) { diff --git a/test/cli/commands.test.ts b/test/cli/commands.test.ts index fa24325..633cc35 100644 --- a/test/cli/commands.test.ts +++ b/test/cli/commands.test.ts @@ -230,20 +230,30 @@ describe("session file", () => { expect(JSON.parse(readFileSync(path, "utf-8"))).toEqual(snapshot); }); - it("a missing session exits 1 with 'Not logged in'", async () => { + it("a missing session exits 3 with 'Not logged in' from every command that needs one", async () => { const ctx = { ...context(), loadSession }; - expect(await whoamiCommand(ctx)).toBe(1); - expect(stderr.text).toBe( + const dir = join(root, "backup"); + expect(await whoamiCommand(ctx)).toBe(3); + expect(await collectionsCommand(ctx, {})).toBe(3); + expect(await filesCommand(ctx, { collection: "1" })).toBe(3); + expect(await getCommand(ctx, "100", {})).toBe(3); + expect(await getThumbCommand(ctx, "100", {})).toBe(3); + expect(await backupMetadataCommand(ctx, dir, {})).toBe(3); + expect(await backupCommand(ctx, dir, {})).toBe(3); + expect(await listMissingThumbnailsCommand(ctx, {})).toBe(3); + expect(await fixMissingThumbnailsCommand(ctx, {})).toBe(3); + const notLoggedIn = `Not logged in. Run "quak login" first.\n` + - `Session file: ${join(ctx.sessionDir, "session.json")}\n`, - ); + `Session file: ${join(ctx.sessionDir, "session.json")}\n`; + expect(stderr.text).toBe(notLoggedIn.repeat(9)); expect(stdout.text).toBe(""); + expect(existsSync(dir)).toBe(false); }); - it("a corrupt session exits 1 and says it is corrupt", async () => { + it("a corrupt session exits 3 and says it is corrupt", async () => { const ctx = { ...context(), loadSession }; saveSession(ctx.sessionDir, snapshot); - expect(await collectionsCommand(ctx, {})).toBe(1); + expect(await collectionsCommand(ctx, {})).toBe(3); expect(stderr.text).toContain("is corrupt"); expect(stderr.text).toContain( `Run "quak logout" and then "quak login" to replace it.\n`, @@ -748,15 +758,12 @@ describe("backup", () => { ); }); - it("exits 1 with the error on one line when the refresh fails", async () => { - const client = { - ...fakeClient(), - collectionsSince: async () => { - throw new Error("HTTP 401 from server"); - }, - } as unknown as Client; - const dir = join(root, "backup"); - // Through `run`, as `bin/quak.ts` does, which prints a thrown error. + // Runs `backup` through `run`, as `bin/quak.ts` does, which prints a thrown + // error; returns the exit code and what `run` printed. + const backupThroughRun = async ( + ctx: CliContext, + dir: string, + ): Promise<{ code: number; runText: string }> => { const runStderr = new PassThrough(); let runText = ""; runStderr.on("data", (chunk: Buffer) => { @@ -764,14 +771,57 @@ describe("backup", () => { }); const code = await new Promise((resolve) => { void run( - backupCommand(context(client), dir, {}), + backupCommand(ctx, dir, {}), new PassThrough(), runStderr, resolve, ); }); + return { code, runText }; + }; + + it("exits 1 with the error on one line when the refresh fails", async () => { + const client = { + ...fakeClient(), + collectionsSince: async () => { + throw new Error("HTTP 503 from server"); + }, + } as unknown as Client; + const dir = join(root, "backup"); + const { code, runText } = await backupThroughRun(context(client), dir); expect(code).toBe(1); - expect(runText).toBe("quak: HTTP 401 from server\n"); + expect(runText).toBe("quak: HTTP 503 from server\n"); + expect(stderr.text).toBe("Starting backup...\nRefreshing library...\n"); + expect(existsSync(dir)).toBe(false); + }); + + // A real saved session, read back by `loadSession`, whose server answers + // every request with 401, as it does once it no longer accepts the token. + // The context's prompts throw, so a prompt would end the run with another + // line and exit 1. + it("exits 3 with one line saying to log in again when the server answers 401", async () => { + const key = toBase64(new Uint8Array(32)); + saveSession(join(root, "session"), { + email: "cli@example.com", + userID: USER_ID, + token: "expired", + masterKey: key, + secretKey: key, + publicKey: key, + }); + const unauthorized = async (): Promise => + new Response(null, { status: 401 }); + const ctx = { + ...context(), + loadSession: (path: string) => + loadSession(path, { fetch: unauthorized }), + }; + const dir = join(root, "backup"); + const { code, runText } = await backupThroughRun(ctx, dir); + expect(code).toBe(3); + expect(runText).toBe( + `quak: the saved session is no longer valid; run "quak login"\n`, + ); expect(stderr.text).toBe("Starting backup...\nRefreshing library...\n"); expect(existsSync(dir)).toBe(false); }); diff --git a/test/cli/run.test.ts b/test/cli/run.test.ts index 29d0a92..cfad348 100644 --- a/test/cli/run.test.ts +++ b/test/cli/run.test.ts @@ -5,6 +5,7 @@ import { PassThrough } from "node:stream"; import { describe, it, expect } from "vitest"; +import { ApiError } from "../../src/api/client.js"; import { run } from "../../src/cli-run.js"; // A stream whose written text is kept in `text`; writes finish at once, so @@ -55,4 +56,26 @@ describe("run", () => { stderr: "quak: 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)), + ); + expect(result).toEqual({ + code: 3, + stdout: "", + stderr: `quak: the saved session is no longer valid; run "quak login"\n`, + }); + }); + + it("prints another HTTP error as it is and exits 1", async () => { + const result = await runToExit( + Promise.reject(new ApiError("forbidden", 403)), + ); + expect(result).toEqual({ + code: 1, + stdout: "", + stderr: "quak: forbidden\n", + }); + }); });