ML data that does not decrypt is recorded with its reason (closes #198)

`fetchMLDataBatch` dropped an entry that did not decrypt, gunzip or parse,
so the file looked like one with no ML data and the run exited 0. It now
returns the reason beside the payloads, also for ML data that is not a
JSON object. The library gives the reasons from its last ML data fetch
that succeeded in `status().mlDataErrors` and the `fetchMLData` "done"
event; a fetch that fails leaves them as they were. `quak backup` writes
the reason as `mlDataError` for a file with no ML data cached and records
the file in `failures.json`. `backup-metadata` writes `mlDataError` for it
and exits 1; `runMetadataBackup` returns `filesWithMLDataError`.

Judgement call: a file whose ML data is cached keeps it in its JSON when a
newer copy does not decode, as when the fetch fails.

Model: opus-5-5

test/cli/backup-end-to-end.test.ts

@@ -9,7 +9,7 @@
  * data this file encrypts with libsodium, as Ente's clients encrypt it. So
  * quak's own code fetches each original, decrypts it, checks it against the
  * hash its uploader recorded and unpacks a live photo, and quak's own retries
- * repeat a failed request. Two tests run `lib.backup()` instead, as a program
+ * repeat a failed request. Three tests run `lib.backup()` instead, as a program
  * using quak as a library does.
  *
  * The account, user 42, has three albums:
@@ -877,6 +877,133 @@ describe("quak backup", () => {
         ]);
     });
 
+    // The server sends beach.jpg's ML data encrypted under a key that is not
+    // the file's, so quak cannot decrypt it. The file's JSON has the reason
+    // in place of the ML data, and the file goes into `failures.json` with
+    // it, so the exit code is 1. The other two files have no ML data, which
+    // is no failure.
+    it("records a file whose ML data does not decrypt in its JSON and in failures.json, and exits 1", async () => {
+        const otherKey = sodium.crypto_secretstream_xchacha20poly1305_keygen();
+        const mlData = encryptStreamBlob(
+            gzipSync(JSON.stringify(ML_DATA)),
+            otherKey,
+        );
+        const fetch: typeof globalThis.fetch = async (input, init) => {
+            if (new URL(String(input)).pathname !== "/files/data/fetch") {
+                return server(input, init);
+            }
+            const data = [
+                {
+                    fileID: BEACH.id,
+                    encryptedData: toBase64(mlData.ciphertext),
+                    decryptionHeader: toBase64(mlData.header),
+                },
+            ];
+            return new Response(JSON.stringify({ data }), {
+                headers: { "content-type": "application/json" },
+            });
+        };
+        const dir = mkdtempSync(join(root, "backup-"));
+
+        const { code, stderr } = await backup(dir, {}, fetch);
+
+        expect(code).toBe(1);
+        const reason = "secretstream chunk authentication failed";
+        const beachJSON = readJSON(join(dir, DAY, "2024-03-01.100.json"));
+        expect(beachJSON).not.toHaveProperty("mlData");
+        expect(beachJSON.mlDataError).toBe(reason);
+        const failures = readJSON(join(dir, "failures.json"));
+        expect(Object.keys(failures.files)).toEqual(["100"]);
+        expect(failures.files["100"]).toMatchObject({
+            fileID: 100,
+            title: "beach.jpg",
+            error: `ML data: ${reason}`,
+        });
+        expect(stderr).toContain(`beach.jpg (id 100): ML data: ${reason}\n`);
+        for (const fileID of [200, 300]) {
+            const json = readJSON(join(dir, DAY, `2024-03-01.${fileID}.json`));
+            expect(json).not.toHaveProperty("mlData");
+            expect(json).not.toHaveProperty("mlDataError");
+        }
+    });
+
+    // A program using quak as a library backs the account up, and another ML
+    // data fetch, as one the library's refresh starts, fails while the backup
+    // writes the JSON files. The backup's own fetch got beach.jpg's ML data
+    // under a key that is not the file's; the fetch that fails leaves that
+    // reason in place, so beach.jpg's JSON and `failures.json` still give it.
+    it("keeps the reason in beach.jpg's JSON and in failures.json when an ML data fetch fails while lib.backup() writes the JSON files", async () => {
+        const otherKey = sodium.crypto_secretstream_xchacha20poly1305_keygen();
+        const mlData = encryptStreamBlob(
+            gzipSync(JSON.stringify(ML_DATA)),
+            otherKey,
+        );
+        let mlDataFetchFails = false;
+        const fetch: typeof globalThis.fetch = async (input, init) => {
+            if (new URL(String(input)).pathname !== "/files/data/fetch") {
+                return server(input, init);
+            }
+            if (mlDataFetchFails) throw new Error("the connection was reset");
+            const data = [
+                {
+                    fileID: BEACH.id,
+                    encryptedData: toBase64(mlData.ciphertext),
+                    decryptionHeader: toBase64(mlData.header),
+                },
+            ];
+            return new Response(JSON.stringify({ data }), {
+                headers: { "content-type": "application/json" },
+            });
+        };
+        const dir = mkdtempSync(join(root, "backup-"));
+        const lib = await Library.open({
+            client: loadSession(join(sessionDir, "session.json"), {
+                fetch,
+                retry: { ...UNATTENDED_RETRY_OPTIONS, sleep: async () => {} },
+            })!,
+            cacheDirectory: `${dir}-cache`,
+            downloadDirectory: dir,
+            refreshIntervalSeconds: Infinity,
+            precacheThumbnails: false,
+            precacheOriginals: false,
+        });
+        try {
+            // The backup reads a file's cached ML data just before it writes
+            // that file's JSON. The first time, a refresh starts an ML data
+            // fetch whose request fails, and the backup goes on once that
+            // fetch has failed.
+            const forFile = lib.mldata.forFile.bind(lib.mldata);
+            vi.spyOn(lib.mldata, "forFile").mockImplementationOnce(
+                async (args) => {
+                    mlDataFetchFails = true;
+                    await lib.fresh();
+                    await vi.waitFor(
+                        () => expect(lib.status().lastMLError).toBeDefined(),
+                        { timeout: 2000, interval: 5 },
+                    );
+                    return forFile(args);
+                },
+            );
+
+            const result = await lib.backup();
+
+            expect(result.failed).toBe(1);
+            const reason = "secretstream chunk authentication failed";
+            const beachJSON = readJSON(join(dir, DAY, "2024-03-01.100.json"));
+            expect(beachJSON).not.toHaveProperty("mlData");
+            expect(beachJSON.mlDataError).toBe(reason);
+            const failures = readJSON(join(dir, "failures.json"));
+            expect(Object.keys(failures.files)).toEqual(["100"]);
+            expect(failures.files["100"]).toMatchObject({
+                fileID: 100,
+                title: "beach.jpg",
+                error: `ML data: ${reason}`,
+            });
+        } finally {
+            await lib.close();
+        }
+    });
+
     // Something changed beach.jpg on disk after it was backed up. A run with
     // --verify hashes every stored original, finds that one no longer matches
     // the hash Ente recorded, says so, and downloads it again; it put back the

test/cli/backup.test.ts

@@ -952,13 +952,14 @@ const ML_PAYLOAD = { face: { faces: [] }, clip: { embedding: [0.5, 0.25] } };
 
 class MLClient extends MockClient {
     mlError?: string;
-    async fetchMLData(args: {
-        fileIDs: number[];
-    }): Promise<Map<number, MLData>> {
+    async fetchMLData(args: { fileIDs: number[] }): Promise<{
+        payloads: Map<number, MLData>;
+        errors: Map<number, unknown>;
+    }> {
         if (this.mlError) throw new Error(this.mlError);
-        const result = new Map<number, MLData>();
-        if (args.fileIDs.includes(100)) result.set(100, ML_PAYLOAD);
-        return result;
+        const payloads = new Map<number, MLData>();
+        if (args.fileIDs.includes(100)) payloads.set(100, ML_PAYLOAD);
+        return { payloads, errors: new Map() };
     }
 }
 
@@ -1573,6 +1574,7 @@ describe("backup album folders", () => {
         contentConcurrency: 5,
         fetchMLData: async () => {},
         mlData: async () => undefined,
+        mlDataError: () => undefined,
         whoami: () => ({ email: "backup@example.com", userID: USER_ID }),
     });
 

test/cli/metadata-backup.test.ts

@@ -489,15 +489,15 @@ describe("quak backup-metadata", () => {
     });
 });
 
-describe("quak backup-metadata when an ML data request fails", () => {
-    // Run the CLI command against the mock and return its exit code, stderr
-    // and output directory.
-    const runCommand = async (failMLDataFor?: number) => {
+describe("quak backup-metadata when a file's ML data cannot be read", () => {
+    // Run the CLI command against `fetch`, the mock unless given, and return
+    // its exit code, stderr and output directory.
+    const runCommand = async (fetch = buildMetaFetch(mock)) => {
         const client = await Client.login({
             email: TEST_EMAIL,
             password: TEST_PASSWORD,
             apiOptions: {
-                fetch: buildMetaFetch(mock, failMLDataFor),
+                fetch,
                 retry: { sleep: async () => {} },
             },
         });
@@ -518,7 +518,9 @@ describe("quak backup-metadata when an ML data request fails", () => {
     };
 
     it("writes every file, marks the failed batch's files, and exits 1", async () => {
-        const { code, stderr, outDir } = await runCommand(200);
+        const { code, stderr, outDir } = await runCommand(
+            buildMetaFetch(mock, 200),
+        );
 
         expect(code).toBe(1);
         expect(stderr).toContain("ML data request for 1 file(s) failed");
@@ -543,6 +545,49 @@ describe("quak backup-metadata when an ML data request fails", () => {
         expect(failed.mlDataError).toContain("500");
     });
 
+    // The server sends ML data for diagram.png encrypted under a key that is
+    // not the file's, so quak cannot decrypt it.
+    it("writes the reason for ML data that does not decrypt, and exits 1", async () => {
+        const otherKey = sodium.crypto_secretstream_xchacha20poly1305_keygen();
+        const { header, ciphertext } = encryptBlob(
+            new Uint8Array(gzipSync(Buffer.from("{}"))),
+            otherKey,
+        );
+        const { code, stderr, outDir } = await runCommand(
+            buildMetaFetch({
+                ...mock,
+                encryptedMLData: {
+                    ...mock.encryptedMLData,
+                    200: {
+                        encryptedData: toBase64(ciphertext),
+                        decryptionHeader: toBase64(header),
+                    },
+                },
+            }),
+        );
+
+        expect(code).toBe(1);
+        const reason = "secretstream chunk authentication failed";
+        expect(stderr).toContain(
+            `ML data of file 200 could not be read: ${reason}\n`,
+        );
+        const failed = JSON.parse(
+            readFileSync(
+                join(outDir, "collections", "20-__Work", "200.json"),
+                "utf-8",
+            ),
+        );
+        expect(failed.mlData).toBeUndefined();
+        expect(failed.mlDataError).toBe(reason);
+        const ok = JSON.parse(
+            readFileSync(
+                join(outDir, "collections", "10-Vacation", "100.json"),
+                "utf-8",
+            ),
+        );
+        expect(ok.mlData.clip.embedding).toEqual([0.5, 0.6, 0.7]);
+    });
+
     it("exits 0 when every ML data request succeeds", async () => {
         const { code } = await runCommand();
         expect(code).toBe(0);

test/library/mldata.test.ts

@@ -12,9 +12,11 @@
  *
  *   2. `Library` wiring: after each refresh the library fetches ML data through
  *      the metadata pool for every known file not yet cached, is incremental on
- *      later refreshes, and refetches a file whose `updationTime` advanced.
- *      Opening a cache directory written by another account starts empty,
- *      its ML data included (issue #104).
+ *      later refreshes, and refetches a file whose `updationTime` advanced. A
+ *      file whose ML data did not decode is reported with the reason, which
+ *      stays listed until the next pass ends, and asked for again. Opening a
+ *      cache directory written by another account starts empty, its ML data
+ *      included (issue #104).
  *
  * Embedding values are chosen to be exactly representable as float32 so the
  * round-trip through `clip.f32` compares equal.
@@ -26,7 +28,7 @@ import { tmpdir } from "node:os";
 import { join } from "node:path";
 
 import { MLDataStore } from "../../src/library/mldata.js";
-import { Library } from "../../src/library/index.js";
+import { Library, type RefreshEvent } from "../../src/library/index.js";
 import type { CollectionsPage, FilesPage } from "../../src/client.js";
 import type { MLData } from "../../src/mldata-fetch.js";
 import type { Collection, EnteFile } from "../../src/model/types.js";
@@ -256,12 +258,14 @@ const file = (
 
 // A mock client that serves scripted collection/file pages and per-file ML
 // payloads, recording every ML fetch request so incremental behaviour is
-// provable.
+// provable. A file in `mlErrorByFile` has ML data that did not decode, for
+// the reason given there.
 class MLMockClient {
     userID = USER_ID;
     collectionsQueue: CollectionsPage[] = [];
     filesByCollection = new Map<number, FilesPage[]>();
     mlByFile = new Map<number, MLData>();
+    mlErrorByFile = new Map<number, Error>();
     mlFetchCalls: number[][] = [];
 
     whoami(): { email: string; userID: number } {
@@ -298,14 +302,20 @@ class MLMockClient {
     async fetchMLData(args: {
         fileIDs: number[];
         fileKeys: Map<number, Uint8Array>;
-    }): Promise<Map<number, MLData>> {
+    }): Promise<{
+        payloads: Map<number, MLData>;
+        errors: Map<number, unknown>;
+    }> {
         this.mlFetchCalls.push([...args.fileIDs]);
-        const result = new Map<number, MLData>();
+        const payloads = new Map<number, MLData>();
+        const errors = new Map<number, unknown>();
         for (const id of args.fileIDs) {
             const p = this.mlByFile.get(id);
-            if (p) result.set(id, p);
+            if (p) payloads.set(id, p);
+            const error = this.mlErrorByFile.get(id);
+            if (error) errors.set(id, error);
         }
-        return result;
+        return { payloads, errors };
     }
 
     filesFor(collectionID: number, ...pages: FilesPage[]): void {
@@ -449,6 +459,137 @@ describe("Library ML-data fetch on refresh", () => {
         }
     });
 
+    it("reports a file whose ML data did not decode through status() and onProgress, and asks for it again", async () => {
+        const client = new MLMockClient();
+        client.collectionsQueue.push({
+            collections: [collection(1, 100)],
+            deleted: [],
+            cursor: 100,
+        });
+        client.filesFor(1, {
+            files: [file(1001, 1, 90), file(1002, 1, 95)],
+            deleted: [],
+            cursor: 95,
+        });
+        client.mlByFile.set(1001, payload([0.5, 0.25, 0.75]));
+        client.mlErrorByFile.set(
+            1002,
+            new Error("secretstream chunk authentication failed"),
+        );
+        const events: RefreshEvent[] = [];
+
+        const lib = await Library.open({
+            client,
+            cacheDirectory,
+            refreshIntervalSeconds: Infinity,
+            onProgress: (event) => events.push(event),
+        });
+        try {
+            await vi.waitFor(
+                () => expect(lib.status().lastMLFetchAt).toBeGreaterThan(0),
+                { timeout: 2000, interval: 5 },
+            );
+            const reasons = [
+                {
+                    fileID: 1002,
+                    error: "secretstream chunk authentication failed",
+                },
+            ];
+            expect(lib.status().mlDataErrors).toEqual(reasons);
+            expect(lib.status().mlStored).toBe(1);
+            expect(events.filter((e) => e.operation === "fetchMLData")).toEqual(
+                [
+                    { operation: "fetchMLData", status: "started" },
+                    {
+                        operation: "fetchMLData",
+                        status: "done",
+                        fetched: 1,
+                        mlDataErrors: reasons,
+                    },
+                ],
+            );
+
+            // Nothing was cached for 1002, so the pass a refresh starts asks
+            // for it again, and once its ML data decodes the reason is gone.
+            client.mlErrorByFile.clear();
+            client.mlByFile.set(1002, payload([1, -2, 0.5]));
+            await lib.fresh();
+            await vi.waitFor(
+                () => {
+                    expect(lib.status().mlStored).toBe(2);
+                    expect(lib.status().mlDataErrors).toEqual([]);
+                },
+                { timeout: 2000, interval: 5 },
+            );
+            expect(client.mlFetchCalls.at(-1)).toEqual([1002]);
+        } finally {
+            await lib.close();
+        }
+    });
+
+    it("still lists the last pass's reasons while the next pass runs", async () => {
+        const client = new MLMockClient();
+        client.collectionsQueue.push({
+            collections: [collection(1, 100)],
+            deleted: [],
+            cursor: 100,
+        });
+        client.filesFor(1, {
+            files: [file(1002, 1, 95)],
+            deleted: [],
+            cursor: 95,
+        });
+        client.mlErrorByFile.set(
+            1002,
+            new Error("secretstream chunk authentication failed"),
+        );
+        const reasons = [
+            { fileID: 1002, error: "secretstream chunk authentication failed" },
+        ];
+        let release!: () => void;
+        const held = new Promise<void>((r) => (release = r));
+
+        const lib = await Library.open({
+            client,
+            cacheDirectory,
+            refreshIntervalSeconds: Infinity,
+        });
+        try {
+            await vi.waitFor(
+                () => expect(lib.status().lastMLFetchAt).toBeGreaterThan(0),
+                { timeout: 2000, interval: 5 },
+            );
+            expect(lib.status().mlDataErrors).toEqual(reasons);
+
+            // Hold the next ML data fetch open until the test releases it.
+            let fetchStarted!: () => void;
+            const started = new Promise<void>((r) => (fetchStarted = r));
+            const realFetch = client.fetchMLData.bind(client);
+            client.fetchMLData = async (args) => {
+                fetchStarted();
+                await held;
+                return realFetch(args);
+            };
+            client.mlErrorByFile.clear();
+            client.mlByFile.set(1002, payload([1, -2, 0.5]));
+
+            // The pass a refresh starts asks for 1002 again. While it is
+            // held, the reason from the pass before is still listed; once it
+            // ends, its own empty list takes the place of that one.
+            await lib.fresh();
+            await started;
+            expect(lib.status().mlDataErrors).toEqual(reasons);
+            release();
+            await vi.waitFor(
+                () => expect(lib.status().mlDataErrors).toEqual([]),
+                { timeout: 2000, interval: 5 },
+            );
+        } finally {
+            release();
+            await lib.close();
+        }
+    });
+
     it("close() resolves only after a running ML data fetch has stored its payloads", async () => {
         const client = new MLMockClient();
         client.collectionsQueue.push({

test/mldata-fetch/mldata-fetch.test.ts

@@ -0,0 +1,54 @@
+// `fetchMLDataBatch`, against a server that sends one file's ML data.
+//
+// ML data that decrypts and gunzips but is not a JSON object is reported with
+// the reason, as ML data that does not parse is, and is never returned as a
+// payload: the library would fail to store it.
+
+import { gzipSync } from "node:zlib";
+import sodium from "libsodium-wrappers-sumo";
+import { beforeAll, describe, expect, it } from "vitest";
+
+import { ApiClient } from "../../src/api/client.js";
+import { encryptBlob, init, toBase64 } from "../../src/crypto/index.js";
+import { fetchMLDataBatch } from "../../src/mldata-fetch.js";
+
+beforeAll(async () => {
+    await init();
+    await sodium.ready;
+});
+
+// Fetch the ML data of file 1 from a server that sends `json`, gzipped and
+// encrypted with the file's key.
+const fetchServing = async (json: string) => {
+    const key = sodium.crypto_secretstream_xchacha20poly1305_keygen();
+    const { header, ciphertext } = encryptBlob(
+        new Uint8Array(gzipSync(json)),
+        key,
+    );
+    const data = [
+        {
+            fileID: 1,
+            encryptedData: toBase64(ciphertext),
+            decryptionHeader: toBase64(header),
+        },
+    ];
+    const api = new ApiClient({
+        fetch: async () =>
+            new Response(JSON.stringify({ data }), {
+                headers: { "content-type": "application/json" },
+            }),
+    });
+    return fetchMLDataBatch(api, [1], new Map([[1, key]]));
+};
+
+describe("fetchMLDataBatch", () => {
+    it.each(["null", "5", "[]"])(
+        "gives a reason for ML data that is %s",
+        async (json) => {
+            const { payloads, errors } = await fetchServing(json);
+
+            expect(payloads.size).toBe(0);
+            expect(errors.get(1)).toEqual(new Error("not a JSON object"));
+        },
+    );
+});