Fix loading real books: stream downloads to disk #7

Merged

365DevNet merged 0 commits from refs/pull/7/head into main

2026-07-16 00:36:01 +02:00

365DevNet commented

2026-07-16 00:35:33 +02:00

(Migrated from codeberg.org)

Not a progress-sync regression — src/net/ is untouched on that branch and the timeout dates to Phase 1 (f974d2f).

What was wrong

The whole response body was buffered in RAM before a byte reached disk. A 67 MB book meant a 67 MB std::string grown by repeated reallocation, each doubling memcpy-ing tens of megabytes, on a device with little memory to spare.

CURLOPT_TIMEOUT capped the entire transfer, not the connect. A 30 second limit on an unbounded download is wrong by construction: it aborts by design once a book is big enough, no matter how healthy the connection.

The two conspired — the realloc churn dragged the transfer past the cap, and the cap killed it.

What changed

Downloads stream straight to the file. Small JSON API calls keep their 30s total timeout, where a cap is still right.
Downloads bound the connect (15s) and abort only on a stall (under 1 KB/s for 30s straight), so slow-but-progressing runs to completion.
Streaming costs what buffering gave for free: the transport needs a path before Content-Disposition reveals the real name, and a failed transfer can leave bytes behind. Downloads land on a temp .part and are renamed only after a clean 200; the caller removes the temp on every failure. A half-written book must never appear in the reader's library.
A stall abort returns CURLE_OPERATION_TIMEDOUT after a 200 was already received, so status is reported as 0 unless the transfer completed cleanly.
DoLoad shows a percentage, throttled to every 10 points because each redraw is a full e-ink refresh (a 67 MB download fires ~23,000 progress callbacks).
Connecting and downloading are now separate screens. The old code drew "Downloading..." before NetConnect, so a sleeping radio produced tens of seconds of a screen claiming to download while nothing transferred — which reads as a hang and sent this investigation down a blind alley.

Verification

Host: 55/55 tests pass, clean cross-build. New tests cover the temp-file/rename/cleanup contract, which is the part unit tests can actually reach.

The tests deliberately do not prove the fix — they use a fake transport and so replay whatever shape we believe in. That is the same trap that let two wrong endpoints ship earlier. Real evidence instead:

Host probe against real libcurl and the real 67 MB book: all 70,573,711 bytes, correct Content-Disposition name, temp cleaned, 23,230 progress callbacks.
On device: Ikigai (29.6 MB) landed at 31,047,528 bytes — exact match, valid complete epub, no .part leftovers. 7 Habits (7.9 MB) likewise.

Honest gap: the original 67 MB book has since been removed from the library, so the precise attribution between the two defects is not reproducible. Both are real and wrong independently.

Loading a real book failed: the device sat on the Downloading screen and then reported "Download failed". The only book ever loaded successfully before was a 21 KB test file, which finished instantly and hid two defects in the Phase 1 download path. The first real book (67 MB) was always going to fail. Not a progress-sync regression — src/net/ is untouched on that branch and the timeout dates to Phase 1 (f974d2f). ## What was wrong **The whole response body was buffered in RAM before a byte reached disk.** A 67 MB book meant a 67 MB std::string grown by repeated reallocation, each doubling memcpy-ing tens of megabytes, on a device with little memory to spare. **CURLOPT_TIMEOUT capped the entire transfer, not the connect.** A 30 second limit on an unbounded download is wrong by construction: it aborts by design once a book is big enough, no matter how healthy the connection. The two conspired — the realloc churn dragged the transfer past the cap, and the cap killed it. ## What changed - Downloads stream straight to the file. Small JSON API calls keep their 30s total timeout, where a cap is still right. - Downloads bound the connect (15s) and abort only on a stall (under 1 KB/s for 30s straight), so slow-but-progressing runs to completion. - Streaming costs what buffering gave for free: the transport needs a path before Content-Disposition reveals the real name, and a failed transfer can leave bytes behind. Downloads land on a temp .part and are renamed only after a clean 200; the caller removes the temp on every failure. A half-written book must never appear in the reader's library. - A stall abort returns CURLE_OPERATION_TIMEDOUT *after* a 200 was already received, so status is reported as 0 unless the transfer completed cleanly. - DoLoad shows a percentage, throttled to every 10 points because each redraw is a full e-ink refresh (a 67 MB download fires ~23,000 progress callbacks). - Connecting and downloading are now separate screens. The old code drew "Downloading..." before NetConnect, so a sleeping radio produced tens of seconds of a screen claiming to download while nothing transferred — which reads as a hang and sent this investigation down a blind alley. ## Verification Host: 55/55 tests pass, clean cross-build. New tests cover the temp-file/rename/cleanup contract, which is the part unit tests can actually reach. The tests deliberately do not prove the fix — they use a fake transport and so replay whatever shape we believe in. That is the same trap that let two wrong endpoints ship earlier. Real evidence instead: - Host probe against real libcurl and the real 67 MB book: all 70,573,711 bytes, correct Content-Disposition name, temp cleaned, 23,230 progress callbacks. - **On device:** Ikigai (29.6 MB) landed at 31,047,528 bytes — exact match, valid complete epub, no .part leftovers. 7 Habits (7.9 MB) likewise. Honest gap: the original 67 MB book has since been removed from the library, so the precise attribution between the two defects is not reproducible. Both are real and wrong independently.

No reviewers

No labels

No milestone

No project

No assignees

1 participant

Notifications

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference

365DevNet/PocketBook_Kavita_sync!7

No description provided.

Rows
Columns