Make backup wait for the server refresh and fail when it fails (closes #99)
check / check (push) Successful in 1m11s
check / check (push) Successful in 1m11s
lib.backup() refreshed through the background loop's refresh, which returns at once when one is already running and swallows a failure, so a backup could run on the previous file list, or on an empty cache, and exit 0. It now uses the refresh fresh() uses: it joins a running refresh or starts one, and rejects before touching any file when it fails. The CLI's error wrapper prints that as one line and exits 1. Model: opus-5-5
This commit is contained in:
@@ -18,6 +18,13 @@ Tag v1.0.0.
|
||||
|
||||
# Completed Steps
|
||||
|
||||
- 2026-09-23: `quak backup` waits for the server refresh and fails when it fails
|
||||
(issue 99). `lib.backup()` joins a refresh already running or starts one, as
|
||||
`fresh()` does, and rejects before touching any file when it fails, leaving
|
||||
`failures.json` as it was, so `quak backup` prints the error as one line and
|
||||
exits 1 instead of backing up the previous run's file list, or nothing, and
|
||||
exiting 0.
|
||||
|
||||
- 2026-09-23: CLI errors print a message instead of a stack trace (issue 102).
|
||||
An error a command throws is printed as one `quak: MESSAGE` line on stderr and
|
||||
the CLI exits 1 once output has drained. The wrapper that does this moved from
|
||||
|
||||
+5
-4
@@ -1,9 +1,10 @@
|
||||
// The backup command, rebuilt on the library API (issue #51).
|
||||
//
|
||||
// `lib.backup()` refreshes the library, then, for every file in scope, gets its
|
||||
// original bytes onto disk under `downloadDirectory` and rebuilds the derived
|
||||
// views (per-file sidecars, per-collection symlink trees, per-collection JSON)
|
||||
// from the model. The on-disk layout is the historical one, unchanged:
|
||||
// `lib.backup()` waits for a completed refresh of the library (a failed one
|
||||
// fails the backup before any file is touched), then, for every file in scope,
|
||||
// gets its original bytes onto disk under `downloadDirectory` and rebuilds the
|
||||
// derived views (per-file sidecars, per-collection symlink trees,
|
||||
// per-collection JSON) from the model. The on-disk layout is the historical one, unchanged:
|
||||
//
|
||||
// <downloadDirectory>/
|
||||
// originals/<fileID>.<ext> the decrypted bytes
|
||||
|
||||
@@ -538,11 +538,13 @@ export class Library {
|
||||
}
|
||||
|
||||
// Back up every in-scope file to `downloadDirectory` in the historical
|
||||
// on-disk layout, with a durable failure ledger (issue #51). Refreshes
|
||||
// first, fetches pending originals (and optional thumbnails) through the
|
||||
// content cache and pools, then rebuilds the derived symlink/JSON views
|
||||
// from the model. Throws before any network work when no download directory
|
||||
// is available or no content cache backs the originals it must fetch.
|
||||
// on-disk layout, with a durable failure ledger (issue #51). Waits for a
|
||||
// completed refresh first, as `fresh()` does, joining one already running,
|
||||
// and rejects before touching any file when it fails. Then fetches pending
|
||||
// originals (and optional thumbnails) through the content cache and pools,
|
||||
// and rebuilds the derived symlink/JSON views from the model. Throws before
|
||||
// any network work when no download directory is available or no content
|
||||
// cache backs the originals it must fetch.
|
||||
backup(opts?: BackupOptions): Promise<BackupResult> {
|
||||
const downloadDirectory =
|
||||
opts?.downloadDirectory ?? this.downloadDirectory;
|
||||
@@ -566,7 +568,7 @@ export class Library {
|
||||
const cache = this.cache;
|
||||
return runBackup(
|
||||
{
|
||||
refresh: () => this.runRefresh(),
|
||||
refresh: () => this.refreshNow(),
|
||||
listCollections: () => this.store.listCollections(),
|
||||
listFiles: (id) => this.store.listFiles(id),
|
||||
original: (fileID) => cache!.original(fileID),
|
||||
|
||||
@@ -625,6 +625,93 @@ describe("lib.backup", () => {
|
||||
});
|
||||
});
|
||||
|
||||
// Every refresh fails, as with an expired session or no network.
|
||||
class FailingClient extends MockClient {
|
||||
override async collectionsSince(): Promise<CollectionsPage> {
|
||||
throw new Error("HTTP 401 from server");
|
||||
}
|
||||
}
|
||||
|
||||
// Holds its refresh open until `release()` is called, then reports a third
|
||||
// album, so a backup can be started while that refresh is still running.
|
||||
class HeldClient extends MockClient {
|
||||
release!: () => void;
|
||||
private held = new Promise<void>((resolve) => {
|
||||
this.release = resolve;
|
||||
});
|
||||
override async collectionsSince(): Promise<CollectionsPage> {
|
||||
await this.held;
|
||||
return {
|
||||
collections: [collection(3, "Later")],
|
||||
deleted: [],
|
||||
cursor: 2,
|
||||
};
|
||||
}
|
||||
override async filesSince(args: {
|
||||
collectionID: number;
|
||||
}): Promise<FilesPage> {
|
||||
if (args.collectionID !== 3) return super.filesSince(args);
|
||||
return { files: [file(300, 3, "late.jpg")], deleted: [], cursor: 2 };
|
||||
}
|
||||
}
|
||||
|
||||
// Fill the library cache on disk, so the next open starts its refresh in the
|
||||
// background instead of waiting for it.
|
||||
const fillCache = async (): Promise<void> => {
|
||||
const lib = await openLibrary(stubSource());
|
||||
await lib.close();
|
||||
};
|
||||
|
||||
describe("the refresh before a backup", () => {
|
||||
it("waits for a refresh already running and backs up what it found", async () => {
|
||||
await fillCache();
|
||||
const client = new HeldClient();
|
||||
const lib = await openLibrary(stubSource(), client);
|
||||
const outDir = join(root, "backup");
|
||||
|
||||
const backup = lib.backup({ downloadDirectory: outDir });
|
||||
client.release();
|
||||
const result = await backup;
|
||||
|
||||
expect(result.totalFiles).toBe(4);
|
||||
expect(existsSync(join(outDir, "originals", "300.jpg"))).toBe(true);
|
||||
await lib.close();
|
||||
});
|
||||
|
||||
it("fails before any download when the refresh fails, leaving failures.json as it was", async () => {
|
||||
await fillCache();
|
||||
const outDir = join(root, "backup");
|
||||
seedLedger(outDir, 100, "beach.jpg");
|
||||
const ledgerPath = join(outDir, "failures.json");
|
||||
const ledgerBefore = readFileSync(ledgerPath, "utf-8");
|
||||
const source = stubSource();
|
||||
const lib = await openLibrary(source, new FailingClient());
|
||||
|
||||
await expect(lib.backup({ downloadDirectory: outDir })).rejects.toThrow(
|
||||
"HTTP 401 from server",
|
||||
);
|
||||
|
||||
expect(source.originalCalls).toBe(0);
|
||||
expect(readFileSync(ledgerPath, "utf-8")).toBe(ledgerBefore);
|
||||
expect(existsSync(join(outDir, "originals"))).toBe(false);
|
||||
await lib.close();
|
||||
});
|
||||
|
||||
it("fails when the refresh fails on an empty cache, instead of backing up nothing", async () => {
|
||||
const source = stubSource();
|
||||
const lib = await openLibrary(source, new FailingClient());
|
||||
const outDir = join(root, "backup");
|
||||
|
||||
await expect(lib.backup({ downloadDirectory: outDir })).rejects.toThrow(
|
||||
"HTTP 401 from server",
|
||||
);
|
||||
|
||||
expect(source.originalCalls).toBe(0);
|
||||
expect(existsSync(outDir)).toBe(false);
|
||||
await lib.close();
|
||||
});
|
||||
});
|
||||
|
||||
// The album folders under collections/, driven through `runBackup` with a
|
||||
// stand-in library whose albums a test changes between runs.
|
||||
describe("backup album folders", () => {
|
||||
|
||||
@@ -20,6 +20,7 @@ import {
|
||||
} from "node:fs";
|
||||
import { join } from "node:path";
|
||||
import { tmpdir } from "node:os";
|
||||
import { PassThrough } from "node:stream";
|
||||
import { describe, it, expect, beforeAll, beforeEach, afterEach } from "vitest";
|
||||
|
||||
import {
|
||||
@@ -34,6 +35,7 @@ import {
|
||||
backupCommand,
|
||||
listMissingThumbnailsCommand,
|
||||
} from "../../src/cli-commands.js";
|
||||
import { run } from "../../src/cli-run.js";
|
||||
import { loadSession } from "../../src/cli-session.js";
|
||||
import type { Client, ClientSnapshot } from "../../src/client.js";
|
||||
import type { ContentSource } from "../../src/library/content.js";
|
||||
@@ -430,6 +432,34 @@ describe("backup", () => {
|
||||
expect(result.errors[0].fileID).toBe(101);
|
||||
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.
|
||||
const runStderr = new PassThrough();
|
||||
let runText = "";
|
||||
runStderr.on("data", (chunk: Buffer) => {
|
||||
runText += chunk.toString();
|
||||
});
|
||||
const code = await new Promise<number>((resolve) => {
|
||||
void run(
|
||||
backupCommand(context(client), dir, {}),
|
||||
new PassThrough(),
|
||||
runStderr,
|
||||
resolve,
|
||||
);
|
||||
});
|
||||
expect(code).toBe(1);
|
||||
expect(runText).toBe("quak: HTTP 401 from server\n");
|
||||
expect(stderr.text).toBe("Starting backup...\nRefreshing library...\n");
|
||||
expect(existsSync(join(dir, "originals"))).toBe(false);
|
||||
});
|
||||
});
|
||||
|
||||
describe("helper list-missing-thumbnails", () => {
|
||||
|
||||
Reference in New Issue
Block a user