š Stop a recovered chapter jump reporting as a reader error - #1600
š Stop a recovered chapter jump reporting as a reader error#1600williamchong wants to merge 1 commit into
Conversation
A ToC href and the spine's can name one file by different paths, so the first display attempt missed and `setActiveNavItem` recovered on the basename map. That miss still logged at error level, which Sentry's captureConsoleIntegration and PostHog's capture_console_errors both promote into an exception: ~100/day across 160 users, none of whom saw anything go wrong, escalating over the real reader errors. Resolve the spine href before the first attempt so the jump lands outright, and warn rather than error whenever the caller holds a fallback. `reader_epub_display_failed` stays the measurement, now with its rung passed explicitly instead of always reporting attempt zero.
There was a problem hiding this comment.
š” Changes recommended
The new āwarn vs errorā behavior is keyed to isSilentError, which is also used for UI suppression, and can unintentionally hide genuine user-visible navigation failures from Sentry/PostHog.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
This PR reduces false-positive āreader errorā exceptions caused by EPUB ToC targets that donāt match the spineās canonical href path. It makes chapter jumps land on the correct spine href on the first attempt (instead of failing then recovering), and adjusts failure logging so recovered attempts donāt get promoted into Sentry/PostHog exceptions while still emitting the reader_epub_display_failed metric with an explicit fallback rung.
Changes:
- Downgrade console logging to
warnfor āsilentā display failures to avoid recovered failures being captured as exceptions. - Resolve the spine href up-front in
setActiveNavItem(usingisEPUBTargetInSpine) so mismatched ToC/spine paths display correctly on the first attempt. - Pass explicit
attemptvalues intodisplayRenditionsoreader_epub_display_failedcan distinguish which fallback rung failed.
File summaries
| File | Description |
|---|---|
| app/pages/reader/epub.vue | Improves EPUB chapter jump targeting and adjusts logging/analytics around display failures to reduce noisy exceptions. |
Review details
Suppressed comments (1)
app/pages/reader/epub.vue:1776
- The first
displayRendition(primaryHref, { isSilentError: true, ... })attempt is always marked silent. After the log-level change inhandleDisplayFailure, any failure on this first attempt will be logged aswarneven when there is no subsequent fallback display attempt (e.g.spineHrefis missing orprimaryHrefalready equalsspineHref), which can hide genuine user-facing navigation failures from Sentry/PostHog.
It would be safer if the ādowngrade to warnā decision was based on whether a second display attempt exists (or another concrete recovery path), rather than on isSilentError.
const isNavHrefInSpine = isEPUBTargetInSpine(rendition.value?.book?.spine, item.href)
const primaryHref = isNavHrefInSpine || !spineHref ? item.href : spineHref
let hasDisplayed = await displayRendition(primaryHref, { isSilentError: true, attempt: 0 })
if (hasDisplayed) return true
- Files reviewed: 1/1 changed files
- Comments generated: 1
- Review effort level: Lite
š” Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| // A silent failure means the caller has a fallback, so warn: Sentry and | ||
| // PostHog both promote console.error into an exception, and a recovered | ||
| // rung would report as a user-facing break. | ||
| console[isSilentError ? 'warn' : 'error'](`Error occurred when displaying${target ? ` ${target}` : ''} in rendition of ${nftClassId.value}`, error) |
A ToC href and the spine's can name one file by different paths, so the first display attempt missed and
setActiveNavItemrecovered on the basename map. That miss still logged at error level, which Sentry's captureConsoleIntegration and PostHog's capture_console_errors both promote into an exception: ~100/day across 160 users, none of whom saw anything go wrong, escalating over the real reader errors.Resolve the spine href before the first attempt so the jump lands outright, and warn rather than error whenever the caller holds a fallback.
reader_epub_display_failedstays the measurement, now with its rung passed explicitly instead of always reporting attempt zero.