Skip to content

Commit 238a12e

Browse files
authored
Keep the library working when its custom-column definitions can't be read (#1154)
get_cc_columns reads a library's custom-column definitions for five surfaces: the book detail page, the detail API, the books table and both search screens. Only the API had error handling, and it caught the wrong class -- an unreadable schema raises SQLAlchemy's OperationalError, so a missing custom_columns table took whole pages down with a 500 while the rest of the library read fine. Degrade inside get_cc_columns instead of copying a guard to five callsites, and read under no_autoflush so the lookup can never flush a caller's pending write and swallow their error as "no custom columns". Verified on a live server with custom_columns renamed out from under it: /book/2 and /api/v1/books/2 went 500 -> 200, /table and /advsearch stayed 200, and subsequent queries on the same session kept working. Suite 9F/4189P -> 3F/4199P, the 3 remaining pre-existing and unrelated. Addresses #1153.
1 parent 5e32e64 commit 238a12e

5 files changed

Lines changed: 232 additions & 5 deletions

File tree

CHANGELOG.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,8 @@ is for things you can see or feel when running the app.
2121
- **Two people using the library at once no longer break each other's pages.** The server kept one working copy of your library data and shared it across every request it was handling. As soon as any one page finished, it closed that copy, so any other page still being built lost the books it had already read and failed. In the log it showed up as `Instance ... is not persistent within this Session`, pointing at whatever the unlucky page happened to be doing at the time, which made it look like a different bug on every occurrence. It got likelier the busier the server was, so it hit big libraries, shared instances and anything running during an import hardest. Each request now keeps its own working copy ([#1150](https://github.com/new-usemame/Calibre-Web-NextGen/issues/1150)).
2222
- **A page you are reading no longer breaks because a background job finished.** Duplicate scans, thumbnail generation, metadata backups and Hardcover syncs all share the server's connection to your library, and when one of them finished it could close that connection while a page you had open was still reading from it. The page then failed, usually with `Cannot operate on a closed database` in the log. It was intermittent by nature — it only bit when the timing overlapped, which is why a manual scan could break browsing once and then work fine on the next try. Background jobs and page loads now keep their own handle on the library, so one finishing can no longer interrupt the other ([#1121](https://github.com/new-usemame/Calibre-Web-NextGen/issues/1121), reported downstream of [#1048](https://github.com/new-usemame/Calibre-Web-NextGen/issues/1048) by [@auspex](https://github.com/auspex)).
2323

24+
- **Your library keeps working when its extra fields can't be read.** Book pages, the books table and both search screens all ask the library for its custom-column definitions — the extra fields you can add to a book in Calibre. If that lookup failed because the library was mid-write, its folder had moved, or the definitions table wasn't there yet, those pages returned an error instead of simply leaving the extra fields out, even though the rest of the library was perfectly readable. Custom columns are supplementary, so these pages now load without them and note the reason in the log ([#1153](https://github.com/new-usemame/Calibre-Web-NextGen/issues/1153)).
25+
2426
- **Opening your library no longer runs the same search twice.** Every visit to the library asked the server for the first page of books, then immediately asked for it again at a different size and threw the first answer away. Nothing looked wrong — the wasted answer never reached the screen — but on a large library that first page is the slowest query on it, and it was being run twice on every load, for every reader. The grid now waits until it knows how many columns it has before asking, so the page is fetched once ([#1144](https://github.com/new-usemame/Calibre-Web-NextGen/issues/1144)).
2527

2628
## [v4.1.21] - 2026-07-25

CHANGES-vs-upstream.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -407,6 +407,8 @@ Format: each row is one fork-PR, mapped to its upstream PR or issue (if any), wi
407407
| #340 | **A new cover now shows up everywhere immediately — web UI and Kobo.** Applying a cover via the focused cover-picker (`POST /book/<id>/cover/apply`) saved `has_cover` and busted the picker's own `?ts=` preview but never bumped `book.last_modified`. Every cover URL in the app cache-busts on `c=last_modified` (the `jinjia` filter feeding detail page, home-grid srcset, edit page, and og:image), so an unchanged timestamp meant the browser kept serving the cached cover until a manual refresh; Kobo native sync selects changed books with `Books.last_modified > sync_token.books_last_modified`, so the device never re-pulled the new cover. **Root cause confirmed on teenyverse**: *Carmilla*'s `cover.jpg` was rewritten 2026-05-28 17:06Z while its `last_modified` was stuck at 2026-05-27 03:01Z. **Fix** mirrors the edit-book path in `_apply_response` — bump `last_modified`, `set_metadata_dirty`, commit, then `remove_synced_book(all=True)`; the `CoverImageId` already embeds the cover mtime, so devices re-fetch once the book is re-selected. The post-commit best-effort steps were split from the critical DB write so a commit failure now returns 500 instead of a false `ok:True`. **Discoverability**: the cover image itself is now the entry to the picker on the book detail page — an always-visible corner camera badge (discoverable on touch) plus a hover/focus "Change cover" overlay; the edit-page button was relabeled from "Open focused cover picker" to "Change cover" (no overlay on the edit page, to avoid discarding unsaved form edits on an accidental click). Shared overlay CSS in `cwa.css` scoped to a new `.cover-editable` class; reuses the existing "Change cover" message id (no new translation strings). **4 regression tests** pin the three sync/cache invariants (last_modified bump, `remove_synced_book(all=True)`, `set_metadata_dirty`) for both the success and failure paths plus a source-pin refactor guard. **Live-verified on cwn-local**: real authenticated multipart upload → `last_modified` bumped, seeded `kobo_synced_books` row cleared, `metadata_dirtied` set, detail page served the new cover (new `c=` param, new image bytes), `CoverImageId` recomputed to `<uuid>-<mtime>`; both entry points exercised via Playwright with 0 console errors. Greptile reviewed (4/5); both P2 findings addressed. | `150e83582` | v4.0.141 |
408408
| #349 | **Edit highlights in the web reader + a KOReader↔server annotation bridge (Phase 1 + 2 of the Kobo highlight feature).** Phase 1: select text in the epub reader → popup (4 colour swatches + optional note) → Save creates a `source='webreader'` highlight that paints as an overlay and persists; click a highlight to recolor / edit the note / soft-delete. Routes on the `annotations` blueprint, UI in `annotations.js`, Hardcover fan-out when that sync target is enabled. Phase 2: `GET/PUT /kosync/syncs/annotations` on the `kosync` blueprint (reuses its Basic auth + checksum→book resolution, no new credentials), a `device_origin_id` column + idempotent migration to suppress feedback loops, a portable-annotation projection as the wire shape, and a `cwasync.koplugin` KoboReader.sqlite provider that writes highlights into a Kobo so the stock reader renders them. The plugin's device-write toggle ships **default-off** until the on-device round-trip (`notes/feat-annotation-koreader-bridge-device-verification.md` Tests 1–4) passes — the SQL→Nickel render was already proven on real hardware (2026-05-26); only the plugin's runtime sqlite-FFI/wifi glue is ungated. Phase 3 (server→stock-Nickel push) excluded — disproven on hardware. **i18n**: the popup strings move from hardcoded English to the reader's `window.calibre.annotationsI18n` bridge (each `_()`-wrapped); Save/Cancel/Delete + the empty-state reuse existing msgids, verified live under a Hungarian session (`Mentés`/`Mégsem`/`Törlés`). **Verified**: 38 unit + a provider schema-fit guard replaying the exact INSERT/SELECT against the real `Bookmark` schema (and a real 1,940-bookmark device-DB copy) + busted Lua + live HTTP CRUD and pull/push (503/401/200) + full Playwright create→paint→reload→edit→delete. Also de-stales two plugin tests already red on `main` (plugin version `4.0.54`→`4.0.136`; CWA→NextGen prompt rebrand). | `(pending squash)` | `(pending release)` |
409409

410+
| #TBD | (fork issue [#1153](https://github.com/new-usemame/Calibre-Web-NextGen/issues/1153), self-reported) | **A book's page no longer 500s when the library's custom-column definitions can't be read.** `_detail_custom_columns()` in `cps/api/books.py` documents itself as "degrading safely if DB metadata is unavailable" but caught only `AttributeError`/`KeyError`/`TypeError`. The error an actually-unreadable calibre schema raises is a SQLAlchemy `OperationalError` (`no such table: custom_columns`) — not in that list — so it escaped and took the whole detail payload down over a supplementary field. Reachable whenever the metadata DB opens but its schema isn't there: library mid-write, moved/renamed library path, `custom_columns` mid-migration, partially-restored library. **Pre-existing in production**; it was masked in tests only because `calibre_db.session` could be `None` and `None.query(...)` raises `AttributeError`, so the *no-session* case was caught while the *unreadable-schema* case was never exercised. #1149 made `session` a property that always materialises, which removed that accidental cushion and turned the latent gap into two red tests on `main` (`test_587_koreader_progress`, plus the same class in `test_kobo_bookmark_created_at` / `test_api_v1_books` under `--dist=loadfile` worker grouping). **Fix**: degrade inside `CalibreDB.get_cc_columns` itself (catch `SQLAlchemyError`/`AttributeError` → log + return `[]`), not at one callsite. Driving the real routes on `cwn-local` showed the API-only fix was incomplete: `/book/<id>` (`cps/web.py` `show_book`) still 500'd, because four of the five callers — book detail page, books table, and both search surfaces — had no guard at all. All five want "the custom columns, if any" and none can act on a DB error, so one source of truth beats five copies of the same try/except. `_detail_custom_columns` keeps its own guard as defense-in-depth. **4 regression tests** in `tests/unit/test_book_detail_custom_columns_degrade.py` — the contract pin (unreadable schema degrades to `[]`) is red without the fix and green with it; two supporting pins assert the fixture reproduces the real raise and that `session` materialises rather than returning `None`, so the contract test can't pass vacuously. Fixture saves/restores the class-level `session_factory` — the bug class here is cross-test pollution. | `TBD` | `TBD` |
411+
410412
## Backports from janeczku/calibre-web (2026-04 wave)
411413

412414
CWA detached from the original Calibre-Web upstream (`janeczku/calibre-web`) at some point before this fork started. The original upstream is alive and pushing fixes — most notably a security wave in April 2026 by @jvoisin and @haraldpdl that CWA never picked up. We sync these fixes directly into our `main` so users on `ghcr.io/new-usemame/calibre-web-nextgen` get them without waiting for upstream's next release tag (releases on calibre-web tend to be 5–6 months apart).

cps/api/books.py

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
from flask import jsonify, request
77
from flask_babel import get_locale
88
from sqlalchemy import and_, func, or_
9+
from sqlalchemy.exc import SQLAlchemyError
910
from sqlalchemy.sql.functions import coalesce
1011

1112
from . import api_v1
@@ -39,10 +40,21 @@ def _visible_hot_book_ids(book_ids):
3940

4041

4142
def _detail_custom_columns():
42-
"""Classic-parity display definitions, degrading safely if DB metadata is unavailable."""
43+
"""Classic-parity display definitions, degrading safely if DB metadata is unavailable.
44+
45+
``SQLAlchemyError`` is the case that actually happens in production: the
46+
calibre metadata DB is reachable but its schema is not (a library that is
47+
still being written, a wrong/renamed library path, a mid-migration
48+
``custom_columns``), and the query raises ``OperationalError``. The
49+
non-DB errors below cover the reconnect window, where ``calibre_db.session``
50+
can be absent rather than merely unreadable.
51+
52+
Custom columns are supplementary to a book's detail payload, so an
53+
unreadable definition table must not take the whole page down with a 500.
54+
"""
4355
try:
4456
return calibre_db.get_cc_columns(config, filter_config_custom_read=True)
45-
except (AttributeError, KeyError, TypeError):
57+
except (SQLAlchemyError, AttributeError, KeyError, TypeError):
4658
log.warning("Custom-column definitions unavailable for book detail", exc_info=True)
4759
return []
4860

cps/db.py

Lines changed: 38 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@
2323
from sqlalchemy.orm import relationship, sessionmaker, scoped_session, joinedload, object_session
2424
from sqlalchemy.orm.collections import InstrumentedList
2525
from sqlalchemy.ext.declarative import DeclarativeMeta
26-
from sqlalchemy.exc import OperationalError
26+
from sqlalchemy.exc import OperationalError, SQLAlchemyError
2727
try:
2828
# Compatibility with sqlalchemy 2.0
2929
from sqlalchemy.orm import declarative_base
@@ -1887,8 +1887,43 @@ def search_query(self, term, config, *join, allow_show_hidden=False):
18871887
.filter(or_(*filter_expression))
18881888

18891889
def get_cc_columns(self, config, filter_config_custom_read=False):
1890-
self.ensure_session()
1891-
tmp_cc = self.session.query(CustomColumns).filter(CustomColumns.datatype.notin_(cc_exceptions)).all()
1890+
"""Custom-column display definitions, or ``[]`` if they can't be read.
1891+
1892+
Degrading here rather than at each callsite is deliberate: five callers
1893+
(book detail page, book detail API, books table, and both search
1894+
surfaces) all want "the custom columns, if there are any", and none of
1895+
them can do anything useful with a DB error. Letting it propagate meant
1896+
an unreadable ``custom_columns`` table took whole pages down with a 500
1897+
over supplementary fields -- while the rest of the library read fine.
1898+
1899+
That happens whenever the metadata DB opens but its schema does not:
1900+
a library mid-write, a moved or renamed library path, ``custom_columns``
1901+
mid-migration, a partly restored library. An empty list is the same
1902+
thing these callers already handle for a library with no custom columns
1903+
at all, so the degraded page is a shape they all render correctly.
1904+
"""
1905+
try:
1906+
self.ensure_session()
1907+
session = self.session
1908+
# no_autoflush because this is a read of definitions, and the
1909+
# session may be carrying a caller's pending mutation. With
1910+
# autoflush on, our SELECT would flush *their* write first, so an
1911+
# unrelated IntegrityError would surface here, get swallowed as
1912+
# "no custom columns", and leave the session needing a rollback for
1913+
# the rest of the request. Not reachable from today's five callers
1914+
# (all read paths), but the failure would be silent and remote from
1915+
# its cause, so don't leave it available to the sixth.
1916+
with session.no_autoflush:
1917+
tmp_cc = (session.query(CustomColumns)
1918+
.filter(CustomColumns.datatype.notin_(cc_exceptions)).all())
1919+
except (SQLAlchemyError, AttributeError):
1920+
# AttributeError: the session is absent rather than unreadable --
1921+
# `session` is None whenever session_factory is (before init_db, or
1922+
# after an explicit `session = None`), and None.query() is an
1923+
# AttributeError rather than a SQLAlchemyError.
1924+
log.warning("Custom-column definitions unavailable; continuing without them",
1925+
exc_info=True)
1926+
return []
18921927
cc = []
18931928
r = None
18941929
if config.config_columns_to_ignore:

0 commit comments

Comments
 (0)