fix(viewer): quieten two Sentry/journal noise sources and fail fast on a lost display - #3271
Merged
vpetersson merged 2 commits intoAug 7, 2026
Merged
Conversation
…n a lost display Three independent viewer fixes, all found in the 2026-08-07 Sentry sweep and validated on the physical testbeds. Screenly#3265 — the Wayland headless-boot recovery logged its own SUCCESS at ERROR, and the Sentry logging integration promotes ERROR records to events, so every self-heal filed an issue (ANTHIAS-4M, the only issue seen on the current release). Demoted to warning; the give-up-after-cap branch stays at ERROR, since that is where a human is actually needed. Induced for real on the Pi 5: recovery WARNING(30), give-up ERROR(40), and a capturing-transport A/B confirmed ERROR -> 1 event, WARNING -> 0. Screenly#3266 — short-circuit the launch retry loop when the display device has vanished. The container's /dev is a start-time snapshot and wait_for_framebuffer only runs at container start, so retrying cannot re-enumerate it; it only delays the restart that actually recovers. Reproduced deterministically on the Pi 2: 3 attempts / 6.02s before, 1 attempt / 2.94s after. Conservative by construction — needs a Qt no-screen signature AND the framebuffer node genuinely absent, and never fires on wayland/eglfs. The false-positive risk was attacked directly: with /dev/fb0 replaced by an unopenable node Qt emitted two of the three signatures and the guard correctly declined, spending its full budget. Screenly#3268 — log the empty-playlist notice on transition rather than every 5s poll, drop the per-tick URL line to debug, and throttle the not-displayable arm to once per offending asset. Measured idle: 1437-1824 lines/hour -> 0 across boards. That arm was the worse offender at 7155-7200 lines/hour on a 0.5s wait, and it also never cleared the empty-playlist latch, which made the notice go permanently silent after "empty -> unavailable -> empty". Honest scope note: Screenly#3266 does NOT reduce Sentry event volume (2 events before and after) — it improves grouping via the distinct message and cuts the black-screen delay. It also cannot fire on the handshake-timeout flavour, whose message carries no Qt output to match.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #3271 +/- ##
=========================================
Coverage ? 90.71%
=========================================
Files ? 76
Lines ? 8440
Branches ? 896
=========================================
Hits ? 7656
Misses ? 563
Partials ? 221 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
5 tasks
There was a problem hiding this comment.
Pull request overview
This PR targets the Anthias viewer’s startup/asset loop diagnostics by reducing high-volume log noise (to preserve journald crash context), demoting a benign Wayland recovery log level to avoid Sentry event spam, and adding a linuxfb-specific “fail fast” path when the display device is no longer usable inside the container.
Changes:
- Demote the Wayland headless-boot recovery log from
ERRORtoWARNINGwhile preserving the give-up branch atERROR. - Add a linuxfb-only guard to abandon the
load_browserretry budget immediately when Qt reports “no screen” and the framebuffer device is absent. - Throttle high-frequency viewer loop logs via transition-based latches (empty playlist + per-offender unavailable asset), and move per-tick “Current url” logging to
debug.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
src/anthias_viewer/__init__.py |
Implements the logging changes, linuxfb fail-fast guard, and new latch-based throttling in asset_loop / load_browser. |
tests/test_viewer.py |
Adds unit/integration-style coverage for the new guard behavior, linuxfb fb-device parsing, and the new log-latch semantics. |
Suppressed comments (1)
src/anthias_viewer/init.py:1191
- This fail-fast WebviewLaunchError message hardcodes "/dev/fb0 absent", but the guard explicitly supports linuxfb's fb=/dev/fbN option via _linuxfb_device(). If fb is set (or becomes set via existing rotation-option preservation), the message can be misleading even when the guard is correct.
raise WebviewLaunchError(
'AnthiasViewer cannot start: the display device is gone '
'(/dev/fb0 absent inside the container, and Qt reports '
'no usable screen). The container needs to restart to '
're-enumerate /dev; retrying in-process cannot help. '
f'Last error: {exc}'
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Copilot review of Screenly#3266. Qt's QPA syntax is `<plugin>[:opt1=val1,opt2=val2,...]` — a single colon, then COMMA-separated options, as _set_qpa_rotation already documents and parses. _linuxfb_device() split the options on ':' instead, so `linuxfb:fb=/dev/fb1,rotation=90` yielded '/dev/fb1,rotation=90' as the device path; os.path.exists then said False and the guard short-circuited the retry budget on a board whose framebuffer was perfectly fine. That is a false positive in exactly the guard the hardware run was checking for false positives — it just needed a board with more than one QPA option set, which none of the testbeds has. - parse the same way _set_qpa_rotation does - name the resolved device in the error message instead of hardcoding /dev/fb0, which would have misreported on an fb1 board - test the comma-separated forms, including options either side of fb=
|
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.
Suppressed comments (2)
tests/test_viewer.py:3796
- This test’s name/docstring claims it verifies the empty-playlist latch “resets on both edges”, but the body never transitions out of the empty state, so it only tests the log-once behavior while continuously empty. Either add a transition in the test, or rename the test/docstring to match what it actually asserts.
def test_empty_playlist_logs_once_then_resets_on_both_edges(
caplog: Any,
) -> None:
"""The idle notice must fire on the transition, not on every 5s poll —
2 lines/tick was ~1440 lines/hour into a ~15 MB volatile journal,
tests/test_viewer.py:3868
caplogis listed as a fixture argument here but never used in the test body, which makes the intent unclear and adds unnecessary fixture setup. Remove it (or add assertions that actually use captured logs).
def test_load_browser_short_circuits_a_vanished_display(caplog: Any) -> None:
"""Integration for the guard: the retry loop must abandon its budget
vpetersson
approved these changes
Aug 7, 2026
Merged
5 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Issues Fixed
Fixes #3265, #3266, #3268 — found in the 2026-08-07 Sentry triage sweep and validated on six physical testbeds.
Description
Three independent viewer fixes. Grouped because they are all in
asset_loop/load_browserand all about the same underlying problem — the viewer drowning its own diagnostics — but they are otherwise unrelated and each stands alone.#3265 — the recovery logged its own success at ERROR
The Wayland headless-boot watchdog exits so the container restarts and re-enumerates the display. It logged that at
ERROR, and the Sentry logging integration promotes ERROR records to events, so every successful self-heal filed a Sentry issue — that is ANTHIAS-4M, the only issue ever seen on the current release. Demoted towarning. The give-up-after-cap branch stays atERROR, because that is the point where recovery has failed and a human is needed.#3266 — fail fast when the display device is gone
The container's
/devis a start-time snapshot andwait_for_framebufferonly runs at container start, so a linuxfb board whose display disappears afterwards can never spawn a webview again. Retrying cannot re-enumerate/dev; it only delays the container restart that actually recovers, by ~6.5 min on the 30-attempt startup path.Conservative by construction: requires both one of Qt's own no-screen messages and the framebuffer node genuinely absent, and returns False for wayland/eglfs. Also honours linuxfb's
fb=/dev/fbNrather than hardcoding/dev/fb0.#3268 — stop the viewer evicting its own crash diagnostics
In production the docker
journalddriver feeds a volatile journal capped at ~10% of/run— about 15 MB on an 800 MB board. The idle viewer was emitting ~1440 lines/hour into it, and the not-displayable arm (a 0.5s wait) up to ~7200 lines/hour, so a single unreachable asset could evict the crash context around any failure within hours. Now: empty-playlist logged on transition, per-tick URL line atdebug, and the not-displayable arm throttled to once per offending asset.Hardware validation
#3266 — Pi 2, before: 3 attempts / 6.02s / generic message. After: 1 attempt / 2.94s / distinct message, no
Retryinglines at all.The false-positive risk does not exist, and this was the main risk in the change. It was attacked three ways on hardware: 5 healthy restarts with
/dev/fb0present (0 firings); an adversarial/dev/fb0replaced by an unopenable node so that Qt emitted two of the three guard signatures while the device still existed — the guard correctly declined and spent its full budget; and an 8/8 on-board truth table. The device check is the load-bearing condition, so a signature match alone cannot fire it.#3265 — induced for real on the Pi 5 by running a second live cage with no outputs and driving the real watchdog (nothing mocked): recovery at WARNING(30), give-up still ERROR(40), restart counter and cap unchanged. A capturing-transport A/B confirmed the mechanism: ERROR → 1 Sentry event, WARNING → 0.
#3268 — idle, 0 enabled assets: 1437–1824 lines/hour → 0, on four boards. Pi 2 measured 17.9 minutes of complete silence. The latch resets in both directions, verified across multiple enable/disable cycles.
Scope notes, so this is not oversold
WebviewLaunchError, whose message carries no Qt output to match. An absent framebuffer makes Qt exit in ~1s, so in practice the crash flavour is what occurs._NO_SCREEN_SIGNATURESis a sample of Qt's phrasings, not an exhaustive set — which is precisely why the device check, not the signature, decides.wait_for_framebuffergate. That is pre-existing and out of scope.Testing
190 passedintests/test_viewer.py, mypy clean across 169 files, ruff check + format clean.New tests cover the gaps the boards flagged as uncovered: the
load_browsershort-circuit and its false-positive counterpart (full budget spent when the device is present), thefb=/dev/fbNlookup, both #3268 latches including that they reset rather than going silent, and the #3265 ERROR→WARNING demotion.Checklist