fix: download every selected book, not just the last

The frontend built book_ids with `set` rather than `append`, so the archive
only ever held one book. The proxy also sent an empty Content-Length, which
is not valid HTTP/1.1, and buffered the whole zip before forwarding a byte
of it.

Entries now carry the file's own name, suffixed when two books claim the
same one, and the books are resolved before the stream starts so a miss is
a 404 instead of a truncated archive under a 200.

backend/src/chitai/controllers/book.py

@@ -509,9 +509,15 @@ class BookController(Controller):
 
         Returns:
             A stream of the compressed ZIP file.
+
+        Raises:
+            FileNotFoundError: If the selection turned up no file that is on disk,
+                which the handler renders as a 404. Awaiting here is what makes that
+                possible: the books are resolved before the response begins, so a
+                miss is still a status code rather than a truncated archive.
         """
         return Stream(
-            books_service.get_files(book_ids, library_id),
+            await books_service.get_files(book_ids, library_id),
             headers={
                 "Content-Disposition": 'attachment; filename="download.zip"',
                 "Content-Type": "application/zip",

backend/src/chitai/exceptions/handlers.py

@@ -12,4 +12,24 @@ def not_found_exception_handler(_: Request, exc: NotFoundError) -> Response:
     )
 
 
-exception_handlers = {NotFoundError: not_found_exception_handler}
+def file_not_found_exception_handler(_: Request, exc: FileNotFoundError) -> Response:
+    """
+    Handler for a row that describes a file the caller cannot be given.
+
+    The row exists, so `NotFoundError` is the wrong answer and the services raise
+    `FileNotFoundError` instead: the book is real, the file it names is not on disk,
+    or the file belongs to another book. All three are the caller asking for
+    something that is not there, which is a 404 — before this they escaped as
+    `ValueError` and reached the client as a 500.
+    """
+    return Response(
+        media_type=MediaType.TEXT,
+        content=f"Not found error: {exc}",
+        status_code=404,
+    )
+
+
+exception_handlers = {
+    NotFoundError: not_found_exception_handler,
+    FileNotFoundError: file_not_found_exception_handler,
+}

backend/src/chitai/services/book.py

@@ -243,6 +243,67 @@ def _unused_path(path: Path) -> Path:
     return candidate
 
 
+def _unused_name(name: str, taken: set[str]) -> str:
+    """
+    The given name, or the first free `name (n).ext` beside it.
+
+    The in-memory counterpart of `_unused_path`, for names within an archive rather
+    than a directory. Two books can hold a file called `Dune.epub` — nothing stops
+    them, since each lives in its own directory — and a zip happily stores the name
+    twice, which loses one of them on extraction.
+
+    Args:
+        name: The filename to place.
+        taken: Names already placed; the chosen one is added to it.
+
+    Returns:
+        A name that nothing in `taken` occupies.
+    """
+    path = Path(name)
+    candidate = name
+    suffix = 1
+
+    while candidate in taken:
+        suffix += 1
+        candidate = f"{path.stem} ({suffix}){path.suffix}"
+
+    taken.add(candidate)
+    return candidate
+
+
+async def _stream_zip(entries: list[tuple[str, Path]]) -> AsyncIterator[bytes]:
+    """
+    Compress already-resolved files into a zip, a chunk at a time.
+
+    Args:
+        entries: `(name within the archive, path on disk)` pairs, in archive order.
+
+    Yields:
+        Chunks of compressed zip file data.
+    """
+    stream = _ZipStream()
+    chunk_size = 32768  # 32 KiB
+
+    with zipfile.ZipFile(stream, "w", zipfile.ZIP_DEFLATED) as zip_file:
+        for arcname, path in entries:
+            info = zipfile.ZipInfo.from_file(path, arcname=arcname)
+            info.compress_type = zipfile.ZIP_DEFLATED
+
+            with zip_file.open(info, "w") as entry:
+                async with aiofiles.open(path, "rb") as source:
+                    while content := await source.read(chunk_size):
+                        entry.write(content)
+                        if chunk := stream.drain():
+                            yield chunk
+
+            if chunk := stream.drain():
+                yield chunk
+
+    # The central directory, written when ZipFile closes.
+    if chunk := stream.drain():
+        yield chunk
+
+
 def _title_keys(title: str, authors: set[str]) -> set[str]:
     """
     Every normalized title one book could reasonably be filed under.
@@ -1815,11 +1876,11 @@ class BookService(SQLAlchemyAsyncRepositoryService[Book]):
             A File object ready for download.
 
         Raises:
-            ValueError: If the file is missing or not found for the given book.
+            FileNotFoundError: If the file is missing or not found for the given book.
         """
         book = await self.get(book_id)
         if book.path is None:
-            raise ValueError("Cannot download file: book has no path")
+            raise FileNotFoundError("Cannot download file: book has no path")
 
         for file in book.files:
             if file.id != file_id:
@@ -1827,16 +1888,23 @@ class BookService(SQLAlchemyAsyncRepositoryService[Book]):
 
             path = Path(book.path) / Path(file.path)
             if not await aios.path.isfile(path):
-                raise ValueError("The file is missing")
+                raise FileNotFoundError("The file is missing")
 
             # Rows written before `guess_content_type` existed can hold null here, and
             # so can `guess_content_type` itself. Litestar fills a null in from the
             # filename, falling back to `application/octet-stream` on the response.
+            #
+            # `filename` is what puts a name in the Content-Disposition. Without it
+            # Litestar sends `attachment; filename=""` and a client that has nothing
+            # else to go on — an OPDS reader, curl -O — names the file after the
+            # numeric URL, extension and all.
             return File(
-                path, media_type=file.content_type or guess_content_type(file.path)
+                path,
+                filename=Path(file.path).name,
+                media_type=file.content_type or guess_content_type(file.path),
             )
 
-        raise ValueError("No such file for the given book")
+        raise FileNotFoundError("No such file for the given book")
 
     async def get_files(
         self, book_ids: list[int], library_id: int
@@ -1844,55 +1912,51 @@ class BookService(SQLAlchemyAsyncRepositoryService[Book]):
         """
         Get all selected book files as a compressed zip file.
 
-        Streams the zip file in chunks to avoid loading the entire archive into memory.
-        Each book gets its own directory in the archive, so two books that happen to
-        share a filename do not overwrite one another. Files that have a row but no
-        longer exist on disk are skipped.
+        Every book and path is resolved here, up front, and only the compression is
+        left to the returned iterator. That ordering is the point: a generator that
+        queried on its first iteration would run *after* the response had already
+        started, since Litestar only drains a `Stream` once the headers are away — so
+        a missing book or a database error arrived as a truncated archive under a
+        `200` rather than as an error anyone could act on.
+
+        Entries carry the file's own name. A name claimed twice — two books can each
+        hold a `Dune.epub`, living as they do in separate directories — gets the
+        second suffixed to `Dune (2).epub`, so nothing is overwritten on extraction.
+        Files that have a row but no longer exist on disk are skipped.
 
         Args:
             book_ids: List of book IDs to include in the zip.
             library_id: The ID of the library containing the books.
 
-        Yields:
-            Chunks of compressed zip file data.
+        Returns:
+            An async iterator over chunks of compressed zip file data.
+
+        Raises:
+            FileNotFoundError: If the selection turned up no file that is on disk.
         """
         books = await self.list(Book.id.in_(book_ids), Book.library_id == library_id)
 
-        stream = _ZipStream()
-        chunk_size = 32768  # 32 KiB
+        entries: list[tuple[str, Path]] = []
+        taken: set[str] = set()
 
-        with zipfile.ZipFile(stream, "w", zipfile.ZIP_DEFLATED) as zip_file:
-            for book in books:
-                # Books without files have no path, and `file.path` is stored relative
-                # to it, so there is nothing to resolve against.
-                if book.path is None:
+        for book in books:
+            # Books without files have no path, and `file.path` is stored relative
+            # to it, so there is nothing to resolve against.
+            if book.path is None:
+                continue
+
+            book_path = Path(book.path)
+            for file in book.files:
+                path = book_path / file.path
+                if not await aios.path.isfile(path):
                     continue
 
-                book_path = Path(book.path)
-                for file in book.files:
-                    path = book_path / file.path
-                    if not await aios.path.isfile(path):
-                        continue
+                entries.append((_unused_name(Path(file.path).name, taken), path))
 
-                    # Namespaced by the book's own directory to avoid collisions.
-                    info = zipfile.ZipInfo.from_file(
-                        path, arcname=f"{book_path.name}/{Path(file.path).name}"
-                    )
-                    info.compress_type = zipfile.ZIP_DEFLATED
+        if not entries:
+            raise FileNotFoundError("No files to download for the given books")
 
-                    with zip_file.open(info, "w") as entry:
-                        async with aiofiles.open(path, "rb") as source:
-                            while content := await source.read(chunk_size):
-                                entry.write(content)
-                                if chunk := stream.drain():
-                                    yield chunk
-
-                    if chunk := stream.drain():
-                        yield chunk
-
-        # The central directory, written when ZipFile closes.
-        if chunk := stream.drain():
-            yield chunk
+        return _stream_zip(entries)
 
     async def update_book(
         self, book_id: int, update_data: ModelDictT[Book], library: Library

backend/tests/unit/test_services/test_book_service.py

@@ -221,7 +221,9 @@ class TestBookServiceCRUD:
         archive_bytes = b"".join(
             [
                 chunk
-                async for chunk in books_service.get_files([book.id], test_library.id)
+                async for chunk in await books_service.get_files(
+                    [book.id], test_library.id
+                )
             ]
         )
 
@@ -231,13 +233,79 @@ class TestBookServiceCRUD:
             names = archive.namelist()
             assert len(names) == len(contents)
 
-            # Entries are namespaced by the book's directory, so two books sharing a
-            # filename cannot overwrite one another.
-            assert {Path(name).parent.name for name in names} == {"Dracula"}
+            # Entries carry the file's own name, with no directory in front of it.
+            assert set(names) == set(contents)
 
             for name, payload in contents.items():
-                entry = next(n for n in names if Path(n).name == name)
-                assert archive.read(entry) == payload
+                assert archive.read(name) == payload
+
+    async def test_get_files_suffixes_a_name_claimed_twice(
+        self, books_service: BookService, test_library: m.Library
+    ) -> None:
+        """Two books can each hold a `Dune.epub`; the archive must keep both.
+
+        Entries are named after the file, and a zip stores the same name twice
+        happily — leaving whichever copy was written last as the one that survives
+        extraction. The second is suffixed instead.
+        """
+        payloads = {
+            "Herbert": b"herbert-payload-" * 512,
+            "Other": b"other-payload-" * 512,
+        }
+        book_ids = []
+
+        for author, payload in payloads.items():
+            book_data = BookCreate(
+                library_id=test_library.id,
+                title="Dune",
+                authors=[author],
+                pages=412,
+            )
+            book = await books_service.to_model_on_create(book_data.model_dump())
+            assert isinstance(book, m.Book)
+
+            book.path = f"{test_library.root_path}/{author}/Dune"
+            await aios.makedirs(book.path)  # type: ignore[arg-type]
+            Path(book.path, "Dune.epub").write_bytes(payload)
+            book.files.append(
+                m.FileMetadata(
+                    path="Dune.epub",
+                    size=len(payload),
+                    hash=f"hash-{author}",
+                    content_type=None,
+                )
+            )
+
+            books_service.repository.session.add(book)
+            await books_service.repository.session.commit()
+            book_ids.append(book.id)
+
+        archive_bytes = b"".join(
+            [
+                chunk
+                async for chunk in await books_service.get_files(
+                    book_ids, test_library.id
+                )
+            ]
+        )
+
+        with zipfile.ZipFile(BytesIO(archive_bytes)) as archive:
+            assert archive.testzip() is None
+            assert sorted(archive.namelist()) == ["Dune (2).epub", "Dune.epub"]
+
+            stored = {archive.read(name) for name in archive.namelist()}
+            assert stored == set(payloads.values())
+
+    async def test_get_files_refuses_a_selection_with_nothing_in_it(
+        self, books_service: BookService, test_library: m.Library
+    ) -> None:
+        """A miss is a 404, raised before the response starts.
+
+        Resolving lazily inside the stream meant an empty selection produced a valid
+        22-byte archive under a 200, which tells the caller nothing went wrong.
+        """
+        with pytest.raises(FileNotFoundError):
+            await books_service.get_files([424242], test_library.id)
 
 
 @pytest.mark.asyncio

frontend/src/lib/state/bookOperations.svelte.ts

@@ -134,8 +134,11 @@ export class BookOperationsState {
 		// eslint-disable-next-line svelte/prefer-svelte-reactivity -- local temporary, not reactive state
 		const downloadUrl = new URL(`/api/books/download`, page.url.origin);
 		downloadUrl.searchParams.set('library_id', this.libraryId);
+		// `append`, not `set` — `set` replaces the existing value, so every book but
+		// the last was dropped from the query string and the archive came back holding
+		// one book however many were selected.
 		bookIds.forEach((id) => {
-			downloadUrl.searchParams.set('book_ids', id.toString());
+			downloadUrl.searchParams.append('book_ids', id.toString());
 		});
 
 		this.download(downloadUrl, filename || 'download.zip');

frontend/src/routes/api/[...path]/+server.ts

@@ -16,16 +16,27 @@ async function handleResponse(response: Response) {
 		const data = await response.json();
 		return json(data);
 	} else {
-		// Handle file or other non-JSON response
-		const responseBody = await response.arrayBuffer();
+		// Only headers the backend actually sent are forwarded. Defaulting a missing
+		// one to '' put `Content-Length: ` on the wire — SvelteKit's `setResponse`
+		// hands every header to `res.setHeader` as-is, empty or not — and an empty
+		// Content-Length is not valid HTTP/1.1, so strict clients reject the whole
+		// response. The multi-book ZIP is streamed and carries no Content-Length,
+		// which made it the one download that always tripped over this.
+		const headers = new Headers({ 'Content-Type': contentType });
+		for (const name of ['Content-Disposition', 'Content-Length']) {
+			const value = response.headers.get(name);
+			if (value) headers.set(name, value);
+		}
 
-		return new Response(responseBody, {
+		// The body is passed through as a stream, matching what `bodyOf` does for
+		// requests. `arrayBuffer()` held the whole response here first, so nothing
+		// reached the browser until the last byte had been read and this process
+		// carried a copy of it — survivable for one book, not for a ZIP of a whole
+		// shelf, which only starts arriving once the backend has finished
+		// compressing all of it.
+		return new Response(response.body, {
 			status: response.status,
-			headers: {
-				'Content-Type': contentType,
-				'Content-Disposition': response.headers.get('Content-Disposition') || '',
-				'Content-Length': response.headers.get('Content-Length') || ''
-			}
+			headers
 		});
 	}
 }