| [#1157](https://github.com/new-usemame/Calibre-Web-NextGen/pull/1157) | (fork issue [#1094](https://github.com/new-usemame/Calibre-Web-NextGen/issues/1094) @auspex) | **A failed conversion dropped the book from the library entirely.** `main()` in `scripts/ingest_processor.py` imported the incoming book only inside `if convert_successful:` and had no `else`, so any conversion failure — the reporter's 63MB PDF killed by the service safety timeout, but equally a corrupt source, unsupported internals, or a missing input plugin — returned without ever calling `add_book_to_library()`, while the `finally` block's `delete_current_file()` removed the source from the ingest folder. The service then logged `Successfully processed`. The two sibling branches that decline to convert (`auto_convert` off; format in `auto_convert_ignored_formats`) already import the original, so the fallback makes behaviour consistent rather than adding a rule. **Fix**: an `elif conversion_attempted:` branch imports the original. The guard flag matters — the ignore-list branch reports `convert_successful=False` *having already imported*, so a bare `else` would have filed every deliberately-unconverted book twice; that near-miss is pinned by `test_ignored_format_is_imported_exactly_once`. Also addressed from the same report: `backup()` now logs the absolute destination and the safety-timeout branch names `/config/processed_books/failed/<file>` (the reporter's "I've got no idea where 'failed backup' is"), and the service no longer claims the processor "should have timed out internally at N seconds" — `ingest_timeout_minutes` bounds only `is_file_in_use()`, so no internal conversion timeout exists; it now points at the setting that does govern it. **Second defect, found by the cross-family review and confirmed by reading `run:245`:** the fallback could not help the reporter's own case at all. The service runs the processor under `timeout "$safety_timeout"`, and nothing inside the processor bounded conversion — so an overrun was a SIGTERM to the processor, not a Python-visible failure, and `main()` never regained control to reach the new branch. (Their log shows `CON_ERROR ... EXIT/ERROR CODE: -15` a second before the shell's `SAFETY TIMEOUT`, which is `timeout` signalling the process group: whether Python survives long enough to print anything is a race, and whether it survives long enough to *import* is a race it usually loses.) **Fix**: `convert_book()`/`convert_to_kepub()` now run the converter with `subprocess.run(timeout=...)` from `CWA_CONVERSION_DEADLINE_SECONDS`, which the s6 script derives from the same `safety_timeout` it enforces (one source of truth; ~10% reserve, capped at a fifth of the budget; unset when `timeout 0` means no hard limit). An overrun is now an ordinary `(False, "")` and the book is imported. **Fourth defect, found by the second review leg:** that deadline was read fresh at each `subprocess.run()`, so a kepub ingest of a non-EPUB — which converts twice back to back inside one hard `timeout` — granted each stage the full deadline, letting the two outlast the watchdog and re-opening this very bug at the DEFAULT setting (at `S=2700/D=2430`, stage one can burn 2429s and stage two starts with a fresh 2430s against 271s of real time). The budget is now ONE allowance anchored to process start (the span `timeout` actually measures); each stage draws on what is left, and an exhausted budget yields a small positive value so it expires as a handled `TimeoutExpired` rather than a `ValueError` escaping the handlers. Two smaller repairs from the same leg: the shell reserve had a cliff (a flat 30s floor inverted the limits — a 31s timeout left 1s to convert while 30s left 23s), now capped so the deadline is monotonic in the timeout, with the shipped 900s/2700s endpoints unchanged and pinned; and `float("inf")` parses but fails at `int()` with `OverflowError`, which is neither `TypeError` nor `ValueError` and escaped while evaluating the `timeout=` argument, now caught. The whole-run anchor means a slow copy-in eats conversion time — the correct direction, since the watchdog starts at process launch and anchoring later lets the inner deadline outrun it; the timeout message now says so. **Third defect, same review**: `convert_to_kepub()`'s generic `except Exception` printed and fell through, returning `None` — `main()` unpacks that call, so it raised `TypeError`, skipped the fallback and dropped the book. Every non-success path now returns a pair. Also, that path backed up only the intermediate epub, so for a non-epub input the failed folder never held the file the user actually dropped in; it now keeps both. **Declined, with reason**: the review's `automerge=overwrite` finding (a rescued file could replace a good existing format) is real but pre-existing and not introduced here — the same unconditional `add_book_to_library(filepath)` runs on the auto-convert-off and ignore-list branches today, and the default is `new_record` (`cwa_schema.sql:46`). Tracked separately rather than folded into this fix. **Verified live in cwn-local mirroring the reporter's config** (`auto_convert=1`, target epub): pre-fix an unconvertible PDF logged `CON_ERROR` then `Successfully processed` with the book count unchanged at 42; post-fix the same file imports (42→43), `/book/205` and `/api/v1/books/205` both 200 with the PDF format attached. Deadline path verified live too: a 12MB txt with a 1s deadline overran, logged the actionable message, backed up to the named folder and imported as **TXT** — the original, not a converted file. Happy path re-checked (txt→epub converts, one EPUB row, fallback silent), ignore-list path re-checked (exactly one row), and an `.acsm` ticket re-checked (not filed, guidance printed, its promise about processed_books/failed now true). 27 regression tests, mutation-verified falsifiable; full unit+smoke suite 3F/4206P against a 3F baseline on `main` — the same three pre-existing failures. 49 regression tests in the module, every fix mutation-verified falsifiable; full unit+smoke 2F/4246P against the same 2F baseline on `main` (`0e423fb04`). Live re-verified in cwn-local (container Python 3.13.14): a 309MB txt with a 1s budget fires the deadline, backs up and imports as **TXT** (42→43), while a generous budget and an unset budget both still convert to **EPUB** — the anti-vacuity control that the fallback fires only on failure. `/security-review` clean (no finding above the confidence bar). Split out rather than stretched: **#1161** (converter descendants can outlive the deadline; `start_new_session` would take them out of the outer `timeout`'s group, so it needs its own signal design) and **#1163** (a failure *after* a successful conversion still unwinds without importing either file). No dependency, license, route or external URL change. Fork-original. | `a7f13064e` | v4.1.22 |
0 commit comments