Three new What-to-sync toggles — Ratings, Book review, Reading time —
plus a manual 'Sync rating, review & time now' menu action.
Ratings mirror summary.rating (0-5) against the server's 1-10 (exact
doubling over, nearest-star back, 0 clears). Reviews mirror
summary.note as raw markdown (the review box shows it literally, the
same accepted tradeoff as notes). Reading time pushes the statistics
plugin's cumulative book total (getStatsBookStatus) — the server
watermark-converts only the delta, so repeated pushes are harmless.
KOReader's summary carries no timestamps, so sync runs on
change-detection: the plugin remembers the last values it pushed per
book (settings.meta_sync_state); a local diff means koreader changed
it (push), a server diff with an unchanged local copy means the server
moved (pull applies, writing straight into doc_settings + flush).
Pull on book open (2s behind the progress pull), push on book close
inside the existing goOnlineToRun window, and both in the manual
action (push first so a koreader edit survives the same gesture).
Server endpoints: /api/sync/koreader/{rating,review,stats}.
KOReader's bundled LuaJSON (json.require) decodes JSON null into a
truthy sentinel function instead of nil. getMetadata responses for a
newly linked book with no server-side progress carry
"progress": null, so the `if not result.progress` guard passed,
indexing the sentinel crashed KOReader outright, and the book could
not be opened ("attempt to index local 'progress' (a function value)"
at the pull handler). "annotations": null would crash
applyServerAnnotations the same way.
Strip the sentinels recursively at the decode boundary in _request so
every call site sees plain nil and existing nil-checks behave as
written. Reproduced in the KOReader flatpak runtime:
json.decode('{"a": null}') yields type(data.a) == "function".
Found while testing a newly OPDS-downloaded book that had no server
progress yet; the crash took down the whole application, not just the
plugin.
_doGetProgress ran every pull through the same forward/backward strategy
gate as the automatic flows (pull-on-open, periodic). With the default
sync_backward = DISABLE, a manual "Pull progress from server" on a book
where the server sat BEHIND the device hit the backward branch, matched
neither SILENT nor PROMPT, and silently did nothing: no prompt, no
message, no navigation. Reported against the Bookhoard server's KOReader
integration (Sense and Sensibility: device at XXXIX, server web reader
at VIII; the server correctly served the converted xpointer
/body/DocFragment[10]/body/div[1]/h2[1] per its logs — the pull reply
was consumed and then dropped by the gate).
An explicit user pull is 'take me to the server position', so it now
bypasses the strategies entirely: syncToProgress + _showSyncedMessage.
The equal-percentage 'already synchronized' early-out is unchanged.
Automatic pulls keep their existing behavior:
- forward/backward SILENT: apply silently
- forward/backward PROMPT: ConfirmBox with the computed target page
- DISABLE: silent skip (intended for pull-on-open / auto-sync)
Verified with a stubbed-KOReader harness driving _doGetProgress through
seven scenarios (manual behind/ahead/equal/no-locator, automatic
SILENT/PROMPT/DISABLE).
Server behavior untouched.
Every push and pull runs deferred — updateProgress and getProgress hand
their work to UIManager:scheduleIn(0.5), and API replies are consumed in
UIManager:nextTick closures — so by the time the task runs, the ReaderUI
may have released its document (close/teardown race) or a slow async
load may not have attached it yet (the CRE full-render window: "cached
rendering is invalid" leaves the reader document-less for seconds on
big EPUBs). The task then dereferenced self.ui.document unguarded and
the unhandled error inside a UIManager task killed the whole app:
main.lua:846: attempt to index field 'document' (a nil value)
...in function 'collectBookData'
...in function '_doUpdateProgress'
...in function 'action' (uimanager _checkTasks)
Observed as a hard crash on the auto-push (pages_before_update = 2)
right after opening an EPUB that needed a full re-render.
getBookhoardUUID and getContextText already nil-guarded their document
access; the task entry points never did. Guard them:
- _doUpdateProgress: bail before collectBookData — the single choke
point for every push source (periodic, close, suspend, menu, the
legacy bootstrap push), so one guard covers the whole class
- _doGetProgress: bail at entry, and re-validate first thing inside the
nextTick closure (its body and applyServerAnnotations touch
document.info long after the request was issued)
- getFileSHA256: return nil instead of indexing doc_settings/document
(all callers already handle nil)
No sync-payload or endpoint changes; server behavior untouched.
John O'Keefe