From 0f7574b4ca1f7b735a2de703668b0b899df8adfe Mon Sep 17 00:00:00 2001 From: sneak Date: Mon, 5 Oct 2026 23:22:36 +0000 Subject: [PATCH] An expired session exits 3 with one line saying to run quak login (closes #164) A 401 from the server reaches run in src/cli-run.ts as the ApiError the refresh threw, so run recognises it there, prints one line saying to run "quak login" and exits 3. A missing or corrupt session file keeps its message and also exits 3. quak backup meets the 401 on its first refresh, before it touches any file. Every other error still exits 1. Judgement call: quak logout is unchanged; it handles its own errors and deletes the session file whatever the server answers. Judgement call: quak backup still prints its two progress lines before the error line. Model: opus-5-5 --- README.md | 15 ++++++--- TODO.md | 9 +++++ src/cli-commands.ts | 22 ++++++------ src/cli-run.ts | 20 ++++++++--- test/cli/commands.test.ts | 70 ++++++++++++++++++++++++++++++--------- test/cli/run.test.ts | 23 +++++++++++++ 6 files changed, 125 insertions(+), 34 deletions(-) diff --git a/README.md b/README.md index 984fc0f..ae92150 100644 --- a/README.md +++ b/README.md @@ -516,8 +516,14 @@ 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 server answers a command's request with HTTP 401, 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. 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 @@ -551,8 +557,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 f4ab28f..55e8a92 100644 --- a/TODO.md +++ b/TODO.md @@ -25,6 +25,15 @@ declares one. # Completed Steps +- 2026-10-05: A command whose saved session the server no longer accepts + (HTTP 401) prints one line, + `quak: the saved session is no longer valid; run "quak login"`, and exits 3 + (issue 164). A missing or corrupt session file keeps its message and now also + exits 3, so a cron job can tell that the user must log in again. `run` in + `src/cli-run.ts` recognises the 401, which reaches it unchanged from the + refresh, so `quak backup` stops there before it touches any file. Every other + error still exits 1. + - 2026-10-03: The download-albums example test no longer fails on a disk with under 50 GiB free (issue 160). It opens its libraries with `freeBelowBytes: 0`, so the free space of the disk it runs on cannot shrink 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 52d6069..8577824 100644 --- a/test/cli/commands.test.ts +++ b/test/cli/commands.test.ts @@ -230,9 +230,9 @@ 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'", async () => { const ctx = { ...context(), loadSession }; - expect(await whoamiCommand(ctx)).toBe(1); + expect(await whoamiCommand(ctx)).toBe(3); expect(stderr.text).toBe( `Not logged in. Run "quak login" first.\n` + `Session file: ${join(ctx.sessionDir, "session.json")}\n`, @@ -240,10 +240,10 @@ describe("session file", () => { expect(stdout.text).toBe(""); }); - 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`, @@ -732,15 +732,12 @@ describe("backup", () => { expect(stderr.text).toBe("Starting backup...\n"); }); - 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) => { @@ -748,14 +745,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", + }); + }); });