Make the download deadline an idle deadline and cancel failed bodies (closes #24)

downloadTimeoutMs now aborts a file or thumbnail download only when no
bytes have arrived for that long (default 60 s, was a 600 s cap on the
whole transfer), so a slow download that keeps making progress completes.
The abort reason is still a TimeoutError, so retry classification is
unchanged. A download that fails before reading the whole response body
cancels it, including when the temp file cannot be opened or the header
is malformed, so a failed file no longer holds its connection.

Model: opus-5-5

test/api/client.test.ts

@@ -38,14 +38,18 @@
  * the network. The fake records every call for assertion.
  */
 
-import { describe, expect, it } from "vitest";
+import { describe, expect, it, vi } from "vitest";
 import {
     ApiClient,
     ApiError,
     DEFAULT_DOWNLOAD_TIMEOUT_MS,
     DEFAULT_REQUEST_TIMEOUT_MS,
 } from "../../src/api/client.js";
-import type { RetryOptions } from "../../src/retry.js";
+import {
+    isRetryable,
+    isSafeToReplay,
+    type RetryOptions,
+} from "../../src/retry.js";
 
 // ---------------------------------------------------------------------------
 // Test helpers
@@ -748,13 +752,11 @@ describe("ApiClient retries", () => {
 
 describe("ApiClient timeouts", () => {
     it("ships bounded default deadlines", () => {
-        // Asserted here so the README and the code cannot drift. Two numbers
-        // rather than one, because a deadline that is sane for a JSON call is
-        // nowhere near enough for a multi-gigabyte body, and a deadline long
-        // enough for that body would let a hung API call stall a backup for
-        // ten minutes.
+        // Asserted here so the README and the code cannot drift. The request
+        // deadline bounds a whole JSON call; the download deadline is an idle
+        // one, measured from the last byte that arrived.
         expect(DEFAULT_REQUEST_TIMEOUT_MS).toBe(30_000);
-        expect(DEFAULT_DOWNLOAD_TIMEOUT_MS).toBe(600_000);
+        expect(DEFAULT_DOWNLOAD_TIMEOUT_MS).toBe(60_000);
     });
 
     it("attaches an abort signal to every request", async () => {
@@ -868,25 +870,91 @@ describe("ApiClient timeouts", () => {
         // that never produces a chunk and never observes the signal, so the
         // only thing that can unblock the read is quak's own enforcement of
         // the deadline over the stream it hands out.
-        const stalling = new Response(
-            new ReadableStream<Uint8Array>({
-                pull: () => new Promise<void>(() => {}),
-            }),
-            { status: 200 },
-        );
-        const { fetch } = scriptedFetch(stalling);
-        const client = new ApiClient({
-            fetch,
-            downloadTimeoutMs: 20,
-            retry: { ...noWait, attempts: 1 },
-        });
+        //
+        // The clock is faked, so the test runs under the real default
+        // deadline and waits for nothing.
+        vi.useFakeTimers();
+        try {
+            const stalling = new Response(
+                new ReadableStream<Uint8Array>({
+                    pull: () => new Promise<void>(() => {}),
+                }),
+                { status: 200 },
+            );
+            const { fetch } = scriptedFetch(stalling);
+            const client = new ApiClient({
+                fetch,
+                retry: { ...noWait, attempts: 1 },
+            });
 
-        const stream = await client.getFileStream(42);
-        const err: unknown = await readAll(stream).catch((e: unknown) => e);
+            const stream = await client.getFileStream(42);
+            let settled = false;
+            const result = readAll(stream).then(
+                (n) => n,
+                (e: unknown) => e,
+            );
+            void result.finally(() => {
+                settled = true;
+            });
 
-        expect(err).toBeInstanceOf(Error);
-        expect((err as Error).name).toBe("TimeoutError");
-    }, 5000);
+            await vi.advanceTimersByTimeAsync(DEFAULT_DOWNLOAD_TIMEOUT_MS - 1);
+            expect(settled).toBe(false);
+            await vi.advanceTimersByTimeAsync(1);
+
+            const err = await result;
+            expect(err).toBeInstanceOf(Error);
+            expect((err as Error).name).toBe("TimeoutError");
+            // Classified as every deadline is: retried by the idempotent
+            // downloads, never replayed for a POST or PUT.
+            expect(isRetryable(err)).toBe(true);
+            expect(isSafeToReplay(err)).toBe(false);
+        } finally {
+            vi.useRealTimers();
+        }
+    });
+
+    it("does not abort a slow body that keeps making progress", async () => {
+        // A deadline over the whole transfer would cut off a large video on a
+        // slow link however steadily it was arriving. The deadline restarts
+        // with every chunk, so a body that sends one byte every 600 ms for
+        // well over the 1000 ms deadline completes.
+        vi.useFakeTimers();
+        try {
+            let sent = 0;
+            const trickling = new Response(
+                new ReadableStream<Uint8Array>({
+                    async pull(controller) {
+                        await new Promise((resolve) =>
+                            setTimeout(resolve, 600),
+                        );
+                        if (sent === 10) {
+                            controller.close();
+                            return;
+                        }
+                        controller.enqueue(new Uint8Array([sent++]));
+                    },
+                }),
+                { status: 200 },
+            );
+            const { fetch } = scriptedFetch(trickling);
+            const client = new ApiClient({
+                fetch,
+                downloadTimeoutMs: 1000,
+                retry: { ...noWait, attempts: 1 },
+            });
+
+            const stream = await client.getFileStream(42);
+            const result = readAll(stream).then(
+                (n) => n,
+                (e: unknown) => e,
+            );
+            await vi.advanceTimersByTimeAsync(11 * 600);
+
+            expect(await result).toBe(10);
+        } finally {
+            vi.useRealTimers();
+        }
+    });
 
     it("lets a body that arrives in time through untouched", async () => {
         // The counterpart to the previous test: enforcing the deadline over

test/download/download.test.ts

@@ -1337,6 +1337,102 @@ describe("download retries: corruption is not retried", () => {
 
         expect(requests()).toBe(1);
     });
+
+    it("cancels the response body when decryption fails", async () => {
+        // A backup run carries on past a failed file, so a body left open on
+        // failure would hold its connection until garbage collection, once
+        // per failed file. This body delivers a corrupt chunk and then stays
+        // open, so only a cancel from the downloader can close it.
+        const corrupted = Uint8Array.from(multiChunk.body);
+        corrupted[10] ^= 0xff;
+        let cancelled = false;
+        const fetch = (async () =>
+            new Response(
+                new ReadableStream<Uint8Array>({
+                    start(controller) {
+                        controller.enqueue(corrupted);
+                    },
+                    cancel() {
+                        cancelled = true;
+                    },
+                }),
+                { status: 200 },
+            )) as typeof globalThis.fetch;
+        const api = new ApiClient({ fetch, retry: { ...noWait, attempts: 1 } });
+        const file = buildMockEnteFile(
+            multiChunkKey,
+            multiChunk.header,
+            multiChunk.header,
+        );
+        const outPath = join(mkdtempSync(join(testDir, "cancel-")), "c.bin");
+
+        await expect(downloadFile(api, file, outPath)).rejects.toThrow(
+            /authentication failed/i,
+        );
+
+        expect(cancelled).toBe(true);
+    });
+
+    it("cancels the response body when the temp file cannot be opened", async () => {
+        // The download fails before a byte of the body is read, so the body
+        // is still open and only a cancel from the downloader can close it.
+        let cancelled = false;
+        const fetch = (async () =>
+            new Response(
+                new ReadableStream<Uint8Array>({
+                    start(controller) {
+                        controller.enqueue(multiChunk.body);
+                    },
+                    cancel() {
+                        cancelled = true;
+                    },
+                }),
+                { status: 200 },
+            )) as typeof globalThis.fetch;
+        const api = new ApiClient({ fetch, retry: { ...noWait, attempts: 1 } });
+        const file = buildMockEnteFile(
+            multiChunkKey,
+            multiChunk.header,
+            multiChunk.header,
+        );
+        const outPath = join(
+            mkdtempSync(join(testDir, "cancel-")),
+            "missing",
+            "c.bin",
+        );
+
+        await expect(downloadFile(api, file, outPath)).rejects.toMatchObject({
+            code: "ENOENT",
+        });
+
+        expect(cancelled).toBe(true);
+    });
+
+    it("cancels the response body when the header is malformed", async () => {
+        // The header is rejected before the body is read, so the body is
+        // still open and only a cancel from the downloader can close it.
+        let cancelled = false;
+        const fetch = (async () =>
+            new Response(
+                new ReadableStream<Uint8Array>({
+                    start(controller) {
+                        controller.enqueue(multiChunk.body);
+                    },
+                    cancel() {
+                        cancelled = true;
+                    },
+                }),
+                { status: 200 },
+            )) as typeof globalThis.fetch;
+        const api = new ApiClient({ fetch, retry: { ...noWait, attempts: 1 } });
+        const shortHeader = multiChunk.header.subarray(0, 5);
+        const file = buildMockEnteFile(multiChunkKey, shortHeader, shortHeader);
+        const outPath = join(mkdtempSync(join(testDir, "cancel-")), "c.bin");
+
+        await expect(downloadFile(api, file, outPath)).rejects.toThrow();
+
+        expect(cancelled).toBe(true);
+    });
 });
 
 // ---------------------------------------------------------------------------