Repository navigation
fix(server): handle missing uhubctl + missing DYMO without traceback spam - #36
Merged
Merged
Conversation
Local dev outside Docker doesn't have uhubctl on PATH. Without it, every /api/power/status poll raised FileNotFoundError out of find_or_recall_printer_port() — *before* the endpoint's try/except caught it — and the frontend retried, producing a wall of tracebacks on every page open. Now: - _run() logs a single WARNING the first time uhubctl isn't found, then re-raises FileNotFoundError unchanged - find_printer_port() catches FileNotFoundError and returns None, so _printer_port_or_404 returns a clean 404 "Printer not detected" The frontend already treats 404 as "hide the power-toggle UI" (see PowerToggle.tsx:6-10), so this works without any client change. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
szrudi
force-pushed
the
fix/uhubctl-missing-graceful
branch
from
May 22, 2026 12:15
17dcf99 to
605cb09
Compare
There was a problem hiding this comment.
Pull request overview
This PR aims to prevent repeated 500 tracebacks when uhubctl is missing by turning that condition into the existing “no controllable printer” 404 behavior and logging a single warning instead of spamming logs.
Changes:
- Add “log warning once” behavior when
uhubctlis missing and translateFileNotFoundErrorintoNonefromfind_printer_port(). - Add tests asserting
find_printer_port()returnsNoneand that the warning is only logged once. - Also includes unrelated changes: a client footer migration banner and a
package.jsonversion bump (both appear out of scope vs PR description/title).
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
server/usb_power.py |
Catch missing uhubctl, log once, and return None so API can return 404 instead of 500 spam. |
server/tests/test_usb_power.py |
Adds coverage for “uhubctl missing” return-None and log-once behavior. |
package.json |
Bumps version to 1.6.5 (contradicts PR description stating no version bump). |
client/src/components/Footer.tsx |
Adds a repository-migration banner (unrelated to usb-power fix). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Address Copilot review on #36: If state.json from a prior run on a uhubctl-equipped host has saved _last_known_port, the previous version of find_or_recall_printer_port would fall back to the cached port even when uhubctl had been removed. The endpoint would then proceed to get_port_status()/power_*, which also shell out via _run() → another FileNotFoundError → 500 with traceback. Same symptom this PR aims to eliminate. - find_printer_port now propagates FileNotFoundError (no longer hides it as None). Cleaner contract: the function does what it says. - find_or_recall_printer_port catches FileNotFoundError → returns None, explicitly bypassing _last_known_port. /api/power/* then renders the same clean 404 as the no-cache path. - Regression test test_find_or_recall_bypasses_cache_when_uhubctl_missing exercises the previously-broken scenario. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Address Copilot review: the previous message said "uhubctl not found on PATH", which is misleading when UHUBCTL_BIN is set to an explicit path that doesn't exist (no PATH lookup happens in that case). The new wording is true for both scenarios and points at the env var as an alternative remedy. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Copilot flagged the missing lock on the log-once latch. The race is real but benign (microsecond window, self-limiting, max N duplicate log lines per process lifetime) — same character of trade-off as the existing unlocked _last_known_port. Document the choice in-place so future readers don't have to re-derive it. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…t_printers list_printers() catches all scan exceptions and dumps a full traceback, but labelle raises DeviceManagerNoDevices as its normal "no DYMO plugged in" signal — so every /api/printers call on a host without a printer was logging a 5-line stack trace. Catch DeviceManagerNoDevices specifically and surface it as a single INFO line per scan. Other unexpected exceptions still print a traceback so genuine USB-stack failures don't get swallowed silently. No latch: scans are user-triggered (page load / manual refresh), not polled, and plug/unplug should be observable in the log. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…isible Adding logger.info() in printer_service was effectively dead code in production: Python's default root logger threshold is WARNING, so INFO was silently dropped. Tests passed because pytest's caplog captures at any level via propagation, regardless of root level — a misleading green light. Set logging.basicConfig in app.py before any module-level logger fires. Default level INFO so service diagnostics show up; honor LOG_LEVEL env var so production can dial it up or down. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…ple in dev Earlier commit defaulted to INFO; the user point is that production should be quieter and dev should be even chattier. Net result: - app.py default LOG_LEVEL = WARNING (production-safe — operators only see things worth looking at, no noise from "no DYMO detected" INFO on every page load). - .env.example sets LOG_LEVEL=DEBUG so `npm run dev` is verbose via the template that fresh contributors copy. Also picks up Copilot's docstring nit: find_printer_port now says "the uhubctl executable can't be found", matching the more general warning wording instead of the over-narrow "isn't on PATH". Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…tartup Copilot review: passing an unrecognized string (typo like INFOO, or numeric like "20") to logging.basicConfig(level=...) raises ValueError at import time, preventing the server from starting. Easy to introduce in production via a misedit of .env. Resolve LOG_LEVEL via getattr with an isinstance(int) check; fall back to WARNING when the value doesn't resolve to a logging level constant. After logging is configured, surface the bad input as a single WARNING so operators see it without the server failing. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
PATCH bump: server-side dev-console cleanups (missing-uhubctl 500-spam gone, DeviceManagerNoDevices traceback gone, defensive LOG_LEVEL parsing). No user-visible behavior change for the happy path. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This was referenced May 22, 2026
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.
Summary
Local dev outside Docker, or a Pi without a printer plugged in, would spam the
npm run devconsole with full Python tracebacks on two different code paths — both of which are expected states, not errors. This PR converts both into clean log lines and 404 responses without changing API contracts.The two issues
1.
uhubctlnot on PATH (/api/power/status)Every poll raised
FileNotFoundErrorout offind_or_recall_printer_port()— before the endpoint'stry/exceptcaught it — and the frontend's auto-retry produced a wall of tracebacks. The frontend already treats404from/api/power/statusas "hide the power-toggle UI" (seeclient/src/components/PowerToggle.tsx:6-10), so the cleanest fix is to convert "uhubctl missing" into that same 404 path.Changes in
server/usb_power.py:_run()logs a single WARNING the first time uhubctl can't be executed (latched via_uhubctl_missing_logged), then re-raisesFileNotFoundErrorunchanged. Wording is general enough for both "not on PATH" and "UHUBCTL_BINpoints at a missing path".find_printer_port()propagatesFileNotFoundErrorunchanged — cleaner contract (the function does what it says, no hidden masking).find_or_recall_printer_port()catchesFileNotFoundErrorand returnsNone, explicitly bypassing_last_known_port. Without that bypass, a cached port from a prior run on a host that had uhubctl would still get returned, and the endpoint would proceed toget_port_status()/power_*and 500-traceback there.2. No DYMO printer connected (
/api/printers)list_printers()caught all scan exceptions and calledtraceback.print_exc(), but labelle raisesDeviceManagerNoDevicesas its normal "no DYMO plugged in" signal. Every page-load scan logged a 5-line stack trace.Changes in
server/printer_service.py:DeviceManagerNoDevicesspecifically and surface it as a single INFO line per scan. Other unexpected exceptions still print a traceback so genuine USB-stack failures aren't swallowed silently.Tests
server/tests/test_usb_power.py— newTestUhubctlMissingclass:find_printer_portraises;find_or_recallreturns None with no cache;find_or_recallbypasses cache when uhubctl missing (regression); WARNING is logged exactly once.server/tests/test_printer_service.py— newtest_no_devices_logs_info_without_traceback: confirmsDeviceManagerNoDevicesproduces an INFO log + empty result, no traceback.Before / after on
npm run devBefore: every poll prints either the Flask + subprocess traceback (uhubctl path) or the labelle scan traceback (printers path).
After:
The WARNING is logged once per process; the INFO appears per scan (cheap, useful diagnostic across plug/unplug).
Sequencing note
No version bump in this PR — leaving the call to whoever merges first. If this lands before #34, bump #34 from
1.6.5→1.6.6(and #35 from1.6.6→1.6.7).Test plan
pytest server/tests/— 218 passed (29 usb_power + 9 printer_service)npm run devno longer spams tracebacks; one WARNING + INFOs on first poll🤖 Generated with Claude Code