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

proper-lockfile's default onCompromised throws from a timer, so a backup
whose backup.lock was removed or taken over by another backup died with
a stack trace. lockBackupDirectory now returns the lock, its release
function and why it was lost, and passes a handler that only records
the reason. The backup checks it after the refresh and before each
original, thumbnail, file's JSON and album, and before it writes
failures.json, and then fails with "lost the lock on DIR: REASON",
which quak backup prints as one line, exiting 1. lib.backup() takes a
caller's lock as lock, in place of lockHeld: true. Releasing a lost lock
leaves backup.lock alone, since it may be another backup's.

Model: opus-5-5

src/backup.ts

@@ -1,8 +1,9 @@
 // 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
-// lock. It waits for a completed refresh of the library (a failed one fails 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
 // original at its save path under `downloadDirectory`, as `Photo.download()`
 // does, waits for an ML data fetch, and rebuilds the derived views (per-file
@@ -96,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
@@ -112,10 +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. `quak backup` takes it before it opens its library.
-    lockHeld?: boolean;
+    // 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.
+    lock?: BackupLock;
 }
 
 export interface BackupError {
@@ -510,11 +520,14 @@ 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;
@@ -524,6 +537,7 @@ const runLockedBackup = async (
 
     log("Refreshing library...");
     await lib.refresh();
+    if (lock.lost) throw lock.lost;
 
     const collectionsDir = join(downloadDirectory, "collections");
     const thumbnailsDir = join(downloadDirectory, "thumbnails");
@@ -642,6 +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;
             let stored = storedAtSavePath(downloadDirectory, file);
             let mismatch = false;
             if (stored !== undefined && verify) {
@@ -721,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 {
@@ -753,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(
@@ -796,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,
@@ -807,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 });
@@ -864,7 +883,9 @@ const runLockedBackup = async (
     // its failure resolved; a file gone from the library (deleted) or outside
     // this run's scope is not something this run can resolve, so keeping its
     // stale entry would keep the exit code non-zero forever — a single
-    // since-deleted photo would fail every future scheduled backup.
+    // 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;
     for (const fileID of [...ledger.keys()]) {
         if (!failedThisRun.has(fileID)) ledger.delete(fileID);
     }
@@ -885,18 +906,36 @@ const runLockedBackup = async (
 
 // 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 `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 });
+    let release: () => Promise<void>;
+    const lock: BackupLock = {
+        release: async () => {
+            if (lock.lost === undefined) await release();
+        },
+    };
     try {
-        return await lockfile.lock(downloadDirectory, {
+        release = await lockfile.lock(downloadDirectory, {
             lockfilePath: join(downloadDirectory, "backup.lock"),
+            onCompromised: (err) => {
+                lock.lost = new Error(
+                    `lost the lock on ${downloadDirectory}: ${err.message}`,
+                );
+            },
         });
     } catch (err) {
         if ((err as NodeJS.ErrnoException).code !== "ELOCKED") throw err;
@@ -905,10 +944,11 @@ export const lockBackupDirectory = async (
             { code: "ELOCKED" },
         );
     }
+    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,
@@ -920,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();
     }
 };

src/cli-commands.ts

@@ -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();
     }
 };
 

src/index.ts

@@ -69,6 +69,7 @@ export {
     type EnsureEvent,
     lockBackupDirectory,
     runBackup,
+    type BackupLock,
     type BackupOptions,
     type BackupResult,
     type BackupError,

src/library/index.ts

@@ -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,9 +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 (see `runBackup`). 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