diff --git a/TODO.md b/TODO.md index 67edd09..1aa7f9b 100644 --- a/TODO.md +++ b/TODO.md @@ -32,8 +32,10 @@ declares one. the work for one original, its `--verify` hashing included, so no more originals than that are open at once. `BackupLibrary` says the limit as `contentConcurrency`. A test holds each download at the fake Ente server to - count those in flight. Each worker checks the backup's lock before it takes - the next file, so once the lock is lost no further original starts. + count those in flight. Once a worker has thrown, as one does when `onProgress` + throws, or the lock is lost, no worker takes another file; the backup fails + once the originals under way are in place, and a lost lock stops it before it + fetches ML data. - 2026-10-14: Next Step said the cache design, issue 36, waits on sneak's review, though that issue closed on 2026-10-03 (issue 194). It now names the diff --git a/src/backup.ts b/src/backup.ts index 8a21adf..488ff30 100644 --- a/src/backup.ts +++ b/src/backup.ts @@ -730,16 +730,24 @@ const runLockedBackup = async ( // Up to the content pool's limit of originals are worked on at once, // their `verify` hashing included, so however large the account is, no // more than that many are open. Each worker takes the next file from the - // iterator they all share until none is left, and throws instead once the - // lock is lost. A worker that throws, as one also does when `onProgress` - // throws, fails the backup only once every worker has stopped, so none - // writes into the backup after it has ended. + // iterator they all share until none is left. Once a worker has thrown, + // as one does when `onProgress` throws, or the lock is lost, no worker + // takes another file. The backup fails only once every worker has + // stopped, so none writes into the backup after it has ended. It checks + // the lock then too, so it does not go on to the thumbnails or the ML + // data fetch without it. if (includeOriginals) { const pending = distinct.values(); + let workerThrew = false; const worker = async (): Promise => { for (const file of pending) { - if (lock.lost) throw lock.lost; - await storeOriginal(file); + if (workerThrew || lock.lost) return; + try { + await storeOriginal(file); + } catch (err) { + workerThrew = true; + throw err; + } } }; const workers = await Promise.allSettled( @@ -748,6 +756,7 @@ const runLockedBackup = async ( for (const w of workers) { if (w.status === "rejected") throw w.reason; } + if (lock.lost) throw lock.lost; } if (includeThumbnails) { diff --git a/test/cli/backup-end-to-end.test.ts b/test/cli/backup-end-to-end.test.ts index 45865ba..65bf5b6 100644 --- a/test/cli/backup-end-to-end.test.ts +++ b/test/cli/backup-end-to-end.test.ts @@ -704,8 +704,8 @@ describe("a backup whose lock is lost", () => { } }; - // The backup downloads up to five originals at once, so all three are - // under way when the lock is lost, and beach.jpg is stored. The backup + // The backup downloads up to five originals at once, so it has started + // all three when the lock is lost, and beach.jpg is stored. The backup // then stops before it fetches ML data. `quak backup` prints why as one // line, with no stack trace, and exits 1. it("stops quak backup before it fetches ML data when backup.lock is removed, and exits 1", async () => {