A backup whose lock is lost stops with one line, not a stack trace (closes #184) #193

Merged
clawbot merged 4 commits from issue-184-lost-lock into next 2026-10-14 07:37:27 +02:00
7 changed files with 87 additions and 75 deletions
Showing only changes of commit 9c921e95dc - Show all commits
+19 -18
View File
@@ -1070,23 +1070,24 @@ photos newest first). `lib.subscribe({ onChange })` delivers a `LibraryChange`
directory, and fails at once with an error whose `code` is `ELOCKED` while
another backup of it runs. A caller can instead take the lock itself, before
it opens its library, as `quak backup` does: `await lockBackupDirectory(dir)`
takes it, failing the same way, and returns the function that releases it, and
the caller passes `lockHeld: true` to the backup. It waits for a refresh as
`fresh()` does, puts every in-scope original not already at its save path
there as `photo.download()` does (and, with `includeThumbnails`, fetches
thumbnails) through the content cache, waits for an ML data fetch, and
rebuilds the on-disk backup tree with a durable failure ledger. Each file's
JSON holds its ML data and, for an image (for a live photo, its image), its
original's EXIF and XMP, with its dimensions for a JPEG only; a video's JSON
holds none of these three. A fetched original is written straight to its save
path and not into the cache, which then counts it as present; one the cache
already held is copied from there. `BackupOptions`: `downloadDirectory` (falls
back to the library's), `includeOriginals` (default `true`),
`includeThumbnails` (default `false`), `onlyAlbumNames`, `verify` (default
`false`), `onProgress`, and `lockHeld` (default `false`). See Backup layout
above for the tree it writes. Whether the backup took the lock or its caller
did, once the lock is lost the backup rejects with
`lost the lock on <dir>: <reason>` before it starts another file.
takes it, failing the same way, and returns a `BackupLock`, whose `release()`
removes the lock unless it was lost and whose `lost` holds why it was lost.
The caller passes it to the backup as `lock`, and releases it once the backup
ends. It waits for a refresh as `fresh()` does, puts every in-scope original
not already at its save path there as `photo.download()` does (and, with
`includeThumbnails`, fetches thumbnails) through the content cache, waits for
an ML data fetch, and rebuilds the on-disk backup tree with a durable failure
ledger. Each file's JSON holds its ML data and, for an image (for a live
photo, its image), its original's EXIF and XMP, with its dimensions for a JPEG
only; a video's JSON holds none of these three. A fetched original is written
straight to its save path and not into the cache, which then counts it as
present; one the cache already held is copied from there. `BackupOptions`:
`downloadDirectory` (falls back to the library's), `includeOriginals` (default
`true`), `includeThumbnails` (default `false`), `onlyAlbumNames`, `verify`
(default `false`), `onProgress`, and `lock`. See Backup layout above for the
tree it writes. Whether the backup took the lock or its caller did, once the
lock is lost the backup rejects with `lost the lock on <dir>: <reason>` before
it starts another file.
### Request pools
@@ -1146,7 +1147,7 @@ from a very old client, is stored unchecked.
- `src/library/mlsearch.ts`: `MLDataAPI`, `SimilarResult`
- `src/exif.ts`: `ExifTags`, `PhotoExif`
- `src/library/pools.ts`: `RequestPools`, `RequestPoolsOptions`, `BoundedPool`
- `src/backup.ts`: `BackupOptions`, `BackupResult`, `BackupError`
- `src/backup.ts`: `BackupLock`, `BackupOptions`, `BackupResult`, `BackupError`
- `src/client.ts`: `Client`, `LoginOptions`, `ClientSnapshot`
- `src/api/client.ts`: `ApiClient`, `ApiClientOptions`, `StreamOptions`
- `src/errors.ts`: `ApiError`, `TruncatedStreamError`
+7 -6
View File
@@ -27,12 +27,13 @@ declares one.
- 2026-10-14: A backup whose lock is lost no longer dies with a stack trace
(issue 184). `lockBackupDirectory` gives proper-lockfile an `onCompromised`
that records why the lock was lost instead of throwing from its timer. The
backup then stops before it starts another original, and before it writes
`failures.json`, with `lost the lock on DIR: REASON`, which `quak backup`
prints as one line before it exits 1. Releasing a lost lock leaves
`backup.lock` alone. Tests in `test/cli/backup-end-to-end.test.ts` remove the
lock, or put another backup's in its place, while a download is held open.
that records why the lock was lost instead of throwing from its timer, and
returns the lock, which `quak backup` passes to `lib.backup()` as `lock`. The
backup then stops before it starts another file with
`lost the lock on DIR: REASON`, which `quak backup` prints as one line before
it exits 1. Releasing a lost lock leaves `backup.lock` alone. Tests in
`test/cli/backup-end-to-end.test.ts` remove the lock, or put another backup's
in its place, while a download is held open.
- 2026-10-14: An album or file quak cannot decrypt, or whose metadata is not
JSON, no longer fails the refresh, so one bad record no longer stops
+46 -39
View File
@@ -1,7 +1,7 @@
// The backup command, rebuilt on the library API (issue #51).
//
// `lib.backup()` takes the lock in `downloadDirectory` (unless its caller
// already holds it), and fails at once when another backup of it holds the
// passes one in), and fails at once when another backup of it holds the
// lock; when the lock is lost while it runs, it fails before it starts another
// file. It 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, puts its
@@ -97,6 +97,14 @@ import type { Collection, EnteFile, FileMetadata } from "./model/types.js";
export type ProgressCallback = (message: string) => void;
// The lock in a backup directory, as `lockBackupDirectory` returns it.
export interface BackupLock {
// Removes `backup.lock`, unless the lock was lost.
release(): Promise<void>;
// Why the lock was lost, once it is.
lost?: Error;
}
export interface BackupOptions {
// Where the backup tree lives. `lib.backup()` defaults it to the library's
// download directory; `runBackup` with none throws before any network
@@ -113,11 +121,11 @@ export interface BackupOptions {
// bytes do not match the content hash its metadata records. Default false.
verify?: boolean;
onProgress?: ProgressCallback;
// The caller already holds the lock in `downloadDirectory`, taken with
// `lockBackupDirectory`, and releases it itself, so the backup does not
// take it, but still fails once that lock is lost. `quak backup` takes it
// The lock in `downloadDirectory`, when the caller took it with
// `lockBackupDirectory` and releases it itself. The backup then does not
// take the lock, and fails once this one is lost. `quak backup` takes it
// before it opens its library.
lockHeld?: boolean;
lock?: BackupLock;
}
export interface BackupError {
@@ -512,23 +520,24 @@ const writeAlbumJSON = (
writeFileSync(path, JSON.stringify(album, null, 2));
};
// The backup itself, which `runBackup` below runs while the lock is held.
// The backup itself, which `runBackup` below runs while `lock` is held. Once
// the lock is lost, another backup may be writing to `downloadDirectory`, so
// this one fails before it starts another file.
const runLockedBackup = async (
lib: BackupLibrary,
opts: BackupOptions,
downloadDirectory: string,
lock: BackupLock,
): Promise<BackupResult> => {
const includeOriginals = opts.includeOriginals ?? true;
const includeThumbnails = opts.includeThumbnails ?? false;
const log = opts.onProgress ?? (() => {});
const only = opts.onlyAlbumNames ? new Set(opts.onlyAlbumNames) : undefined;
const verify = opts.verify ?? false;
// The lock `lockBackupDirectory` took for this backup. Once it is lost,
// another backup may be writing to `downloadDirectory`.
const lock = heldLocks.get(resolve(downloadDirectory));
log("Refreshing library...");
await lib.refresh();
if (lock.lost) throw lock.lost;
const collectionsDir = join(downloadDirectory, "collections");
const thumbnailsDir = join(downloadDirectory, "thumbnails");
@@ -647,7 +656,7 @@ const runLockedBackup = async (
// that cannot be read is recorded as failed and left where it is.
if (includeOriginals) {
for (const [fileID, file] of distinct) {
if (lock?.lost) throw lock.lost;
if (lock.lost) throw lock.lost;
let stored = storedAtSavePath(downloadDirectory, file);
let mismatch = false;
if (stored !== undefined && verify) {
@@ -727,6 +736,7 @@ const runLockedBackup = async (
if (includeThumbnails) {
for (const [fileID, file] of distinct) {
if (lock.lost) throw lock.lost;
const dest = join(thumbnailsDir, `${fileID}.jpg`);
if (isPresent(dest)) continue;
try {
@@ -759,6 +769,7 @@ const runLockedBackup = async (
log(`FAILED ML data: ${mlDataError}`);
}
for (const file of distinct.values()) {
if (lock.lost) throw lock.lost;
const stored = storedAtSavePath(downloadDirectory, file);
if (stored === undefined) continue;
const path = withExtension(
@@ -802,6 +813,7 @@ const runLockedBackup = async (
const albumDirNames = new Map(
allCollections.map((c, i) => [c.id, dirNames[i]!]),
);
if (lock.lost) throw lock.lost;
try {
removeStaleAlbumDirs(
collectionsDir,
@@ -813,6 +825,7 @@ const runLockedBackup = async (
}
for (const c of collections) {
if (lock.lost) throw lock.lost;
const colDirName = albumDirNames.get(c.id)!;
const colDir = join(collectionsDir, colDirName);
mkdirSync(colDir, { recursive: true });
@@ -872,7 +885,7 @@ const runLockedBackup = async (
// stale entry would keep the exit code non-zero forever — a single
// since-deleted photo would fail every future scheduled backup. A backup
// whose lock was lost leaves the ledger as it is.
if (lock?.lost) throw lock.lost;
if (lock.lost) throw lock.lost;
for (const fileID of [...ledger.keys()]) {
if (!failedThisRun.has(fileID)) ledger.delete(fileID);
}
@@ -891,29 +904,30 @@ const runLockedBackup = async (
};
};
// The lock this process holds on each backup directory, by the directory's
// absolute path, until it is released. `lost` is set once the lock is lost.
const heldLocks = new Map<string, { lost?: Error }>();
// Only one backup of a directory runs at a time, in this process or another.
// Creates `downloadDirectory` if it is missing, takes the lock in it and
// returns the function that releases it; while another backup holds the lock,
// fails at once with an error whose `code` is `ELOCKED`. The lock is the
// directory `backup.lock`, whose modification time proper-lockfile keeps
// current while it is held. One it has not touched for 10 seconds was left by
// a run that could not remove it, and is taken over.
// returns it; while another backup holds the lock, fails at once with an error
// whose `code` is `ELOCKED`. The lock is the directory `backup.lock`, whose
// modification time proper-lockfile keeps current while it is held. One it has
// not touched for 10 seconds was left by a run that could not remove it, and
// is taken over.
//
// proper-lockfile finds the lock lost when `backup.lock` is gone or is another
// backup's, and calls `onCompromised` from a timer, where throwing would end
// the process. The backup holding the lock stops before it starts another file
// instead (see `runLockedBackup`), and releasing a lost lock leaves
// `backup.lock` alone.
// the process. The `onCompromised` given here sets the lock's `lost` instead,
// and the backup holding the lock stops before it starts another file (see
// `runLockedBackup`). Releasing a lost lock leaves `backup.lock` alone, since
// it may be another backup's.
export const lockBackupDirectory = async (
downloadDirectory: string,
): Promise<() => Promise<void>> => {
): Promise<BackupLock> => {
mkdirSync(downloadDirectory, { recursive: true });
const lock: { lost?: Error } = {};
let release: () => Promise<void>;
const lock: BackupLock = {
release: async () => {
if (lock.lost === undefined) await release();
},
};
try {
release = await lockfile.lock(downloadDirectory, {
lockfilePath: join(downloadDirectory, "backup.lock"),
@@ -930,18 +944,11 @@ export const lockBackupDirectory = async (
{ code: "ELOCKED" },
);
}
const path = resolve(downloadDirectory);
heldLocks.set(path, lock);
return async () => {
// Once this lock was lost, another backup in this process may have
// taken the directory.
if (heldLocks.get(path) === lock) heldLocks.delete(path);
if (lock.lost === undefined) await release();
};
return lock;
};
// Runs the backup holding the lock, which it takes and releases itself unless
// the caller already holds it (`opts.lockHeld`).
// the caller passes the lock it took (`opts.lock`).
export const runBackup = async (
lib: BackupLibrary,
opts: BackupOptions,
@@ -953,13 +960,13 @@ export const runBackup = async (
"open the library with one)",
);
}
if (opts.lockHeld) {
return runLockedBackup(lib, opts, downloadDirectory);
if (opts.lock) {
return runLockedBackup(lib, opts, downloadDirectory, opts.lock);
}
const release = await lockBackupDirectory(downloadDirectory);
const lock = await lockBackupDirectory(downloadDirectory);
try {
return await runLockedBackup(lib, opts, downloadDirectory);
return await runLockedBackup(lib, opts, downloadDirectory, lock);
} finally {
await release();
await lock.release();
}
};
+5 -5
View File
@@ -20,7 +20,7 @@ import {
type ClientSnapshot,
type LoginOptions,
} from "./client.js";
import { lockBackupDirectory } from "./backup.js";
import { lockBackupDirectory, type BackupLock } from "./backup.js";
import { init } from "./crypto/index.js";
import {
defaultCacheDirectory,
@@ -416,9 +416,9 @@ export const backupCommand = async (
ctx.stderr.write("Starting backup...\n");
// The lock is taken before the library opens and starts its refresh, so a
// run refused while another backup of `dir` runs sends no request.
let release: () => Promise<void>;
let lock: BackupLock;
try {
release = await lockBackupDirectory(dir);
lock = await lockBackupDirectory(dir);
} catch (err) {
if ((err as NodeJS.ErrnoException).code !== "ELOCKED") throw err;
ctx.stderr.write(`quak: ${(err as Error).message}\n`);
@@ -440,7 +440,7 @@ export const backupCommand = async (
try {
const result = await lib.backup({
downloadDirectory: dir,
lockHeld: true,
lock,
verify: opts.verify,
onProgress: (msg) => {
if (!opts.json) ctx.stderr.write(msg + "\n");
@@ -477,7 +477,7 @@ export const backupCommand = async (
await lib.close();
}
} finally {
await release();
await lock.release();
}
};
+1
View File
@@ -69,6 +69,7 @@ export {
type EnsureEvent,
lockBackupDirectory,
runBackup,
type BackupLock,
type BackupOptions,
type BackupResult,
type BackupError,
+5 -4
View File
@@ -96,6 +96,7 @@ import { runBackup, type BackupOptions, type BackupResult } from "../backup.js";
export {
lockBackupDirectory,
runBackup,
type BackupLock,
type BackupOptions,
type BackupResult,
type BackupError,
@@ -570,10 +571,10 @@ export class Library {
// Back up every in-scope file to `opts.downloadDirectory`, or else the
// library's, each original at its save path, with a durable failure
// ledger (issue #51). Takes the lock in that directory first, unless
// `opts.lockHeld` says the caller holds it, failing at once while another
// backup of it runs, and before it starts another file once the lock is
// lost (see `lockBackupDirectory`). Waits for a
// ledger (issue #51). Takes the lock in that directory first, unless the
// caller passes the lock it took as `opts.lock`, failing at once while
// another backup of it runs, and before it starts another file once the
// lock is lost (see `lockBackupDirectory`). Waits for a
// completed refresh, as `fresh()` does, joining one already running, and
// rejects before touching any file but the lock when it fails. Then puts
// pending originals at their save paths as `Photo.download()` does (and
+4 -3
View File
@@ -806,9 +806,10 @@ describe("a backup whose lock is lost", () => {
});
// A program can take the lock before it opens its library, as `quak
// backup` does. `lockBackupDirectory` returns the lock; the program passes
// it to `lib.backup()`, which checks it as it checks a lock it took, and
// releases it once the backup ends. The lock's `lost` says why it was lost.
// backup` does: `lockBackupDirectory` returns the lock, which the program
// passes to `lib.backup()` and releases once the backup ends. The backup
// checks it as it checks a lock it took, and the lock's `lost` says why it
// was lost.
it("rejects lib.backup() given its caller's lock once that lock is lost", async () => {
const lock = await lockBackupDirectory(dir);
const lib = await openLibrary();