Compare commits

..
Author SHA1 Message Date
Chuck 4521acdf57 Merge main into fix/live-display-modes-668 2026-10-05 17:42:33 -04:00
ChuckandClaude Opus 5.5 e4f5e49ff7 test(sports): treat an adopted sports_helpers copy as parity, not missing (#777)
* test(sports): treat an adopted sports_helpers copy as parity, not missing

The scoreboards deleted their copies of the sports_helpers bodies and
constants when they adopted SportsHelpersMixin (ledmatrix-plugins
#563/#564), so the 19 parity tests in test/test_sports_helpers.py failed
whenever LEDMATRIX_PLUGINS pointed at a plugins checkout. A copy that is
gone now counts as adopted when the plugin imports
src.common.sports_helpers, as the stage 3/4 and game-over parity tests
already do; a copy that remains must still match.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* test(sports): _adopted checks for a real import via the AST, not a text match

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
2026-10-05 17:42:27 -04:00
ChuckandClaude Sonnet 5.5 b440a27cdb fix(plugins): address review -- no manifest fallback for live plugins, keep mode names whole, send registered spelling
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
2026-10-05 17:30:07 -04:00
Chuck 6dbec2e0b1 Merge main into fix/live-display-modes-668 2026-10-05 17:28:51 -04:00
ChuckandClaude Opus 5.5 e74447c65e fix(plugins): call the runtime view's display_modes directly
Codacy flagged the getattr/callable indirection as 'lookup is not callable'.
The view is a PluginRuntimeView or None; anything else raises inside the
existing try and falls back to the manifest.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
2026-10-05 09:54:38 -04:00
ChuckandClaude Opus 5.5 554426e023 fix(plugins): web mode lookups use the modes the display registered (#668)
A plugin may compute its display modes from its config: soccer-scoreboard
registers soccer_<league>_live/recent/upcoming for every custom_leagues
entry, which no manifest can list ahead of time. The display always rotated
them (_register_loaded_plugin prefers plugin.modes), but the web process
reads plugins as files, so /display/modes, the on-demand dialog and
on-demand/start with a mode and no plugin_id saw only manifests -- a custom
league's mode was missing from every list and 404'd on lookup.

- PluginStateManager.record_modes(): the controller records what it
  registered, on the loaded record (an unload or reload forgets it)
- the runtime snapshot carries it per plugin as "modes" (bounded), and
  PluginRuntimeView.display_modes() reports it only while live
- PluginCatalog takes a runtime_source; get_plugin_display_modes and
  find_plugin_for_mode prefer the live modes, falling back to the manifest
  when the display is stopped or has not loaded the plugin. The view is read
  at most once a second, so a listing is one read, not one per plugin.

No manifest or plugin change needed.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
2026-10-05 08:33:47 -04:00
26 changed files with 586 additions and 303 deletions
+9 -9
View File
@@ -19,15 +19,15 @@ accepts both, but the store flags the old spelling as deprecated
## Unreleased ## Unreleased
- Web API error responses no longer carry an exception's message (CodeQL ### Tooling
`py/stack-trace-exposure`). `describe_exception()` now returns a reason
code -- the exception type, plus the errno for an `OSError` - `test/test_sports_helpers.py`'s parity tests pass again with
(`OSError:EIO`, `PermissionError:EACCES`) -- and logs the message `LEDMATRIX_PLUGINS` set. The scoreboards deleted their copies of the
instead, so `details` still names the fault without quoting paths, URLs `sports_helpers` bodies and constants when they adopted `SportsHelpersMixin`
or library internals. The display service status and the on-demand (ledmatrix-plugins #563/#564), and the 19 tests still expected them. A copy
start/stop `service` results keep `active`, `returncode` and `started` that is gone now counts as adopted when the plugin imports
but drop systemctl's `stdout`/`stderr`; WiFi, unit-refresh and `src.common.sports_helpers`, as the stage 3/4 and game-over parity tests
config-save failures say what failed and point at the log. already do; a copy that remains must still match.
## 3.8.2 ## 3.8.2
+6 -1
View File
@@ -143,7 +143,9 @@ loaded and when. Nothing else keeps plugin state:
`DisplayController` right after it creates the `PluginManager`, writes the `DisplayController` right after it creates the `PluginManager`, writes the
cache key `plugin_runtime_snapshot`: per plugin `loaded`, `state`, `error` cache key `plugin_runtime_snapshot`: per plugin `loaded`, `state`, `error`
(type, a redacted message of at most 200 characters, when, recoverable), (type, a redacted message of at most 200 characters, when, recoverable),
`version` and `loaded_at`, plus `published_at`, `stale_after` and `running`. `version`, `loaded_at` and `modes` (the display modes `DisplayController`
registered -- `plugin.modes` when the plugin computes them, else the
manifest's), plus `published_at`, `stale_after` and `running`.
The cache is on disk, usually the SD card, so it writes when something a The cache is on disk, usually the SD card, so it writes when something a
reader sees changes -- throttled to once per 10 s -- and otherwise once a reader sees changes -- throttled to once per 10 s -- and otherwise once a
minute as a heartbeat. RUNNING, which every `update()` passes through, is minute as a heartbeat. RUNNING, which every `update()` passes through, is
@@ -159,6 +161,9 @@ truth cannot leak into a response. `/api/v3/plugins/installed` returns
`loaded`, `state`, `error_info`, `loaded_version` and `loaded_at` per `loaded`, `state`, `error_info`, `loaded_version` and `loaded_at` per
plugin and `data.runtime` (`status`, `published_at`, `age_seconds`); plugin and `data.runtime` (`status`, `published_at`, `age_seconds`);
`/api/v3/plugins/state` returns the same beside the desired state. `/api/v3/plugins/state` returns the same beside the desired state.
`PluginCatalog.get_plugin_display_modes` and `find_plugin_for_mode` prefer a
live view's `modes` to the manifest's `display_modes`, so `/display/modes`
and on-demand see modes a plugin generates from its config (#668).
**Reconciliation** **Reconciliation**
([`state_reconciliation.py`](../src/plugin_system/state_reconciliation.py)) ([`state_reconciliation.py`](../src/plugin_system/state_reconciliation.py))
+5 -3
View File
@@ -363,9 +363,11 @@ it. This is the list the force-display dialog offers.
Send the reported `plugin_id` alongside `mode` when starting an on-demand Send the reported `plugin_id` alongside `mode` when starting an on-demand
display: `/display/on-demand/start` falls back to `find_plugin_for_mode` when display: `/display/on-demand/start` falls back to `find_plugin_for_mode` when
`plugin_id` is omitted, and that lookup only sees modes declared in a static `plugin_id` is omitted. While the display is running, this list and that
manifest — a plugin whose modes are generated (each installed Starlark app is lookup use the modes the display registered, including ones a plugin generates
one) returns 404 there. from its config (each installed Starlark app, each soccer `custom_leagues`
entry). With the display stopped, or for a plugin it has not loaded, both see
only the modes its manifest declares.
Triggers plugin discovery, which is otherwise lazy — so a caller that never Triggers plugin discovery, which is otherwise lazy — so a caller that never
opens the dashboard still gets the full list. opens the dashboard still gets the full list.
+9
View File
@@ -4624,6 +4624,15 @@ class DisplayController:
display_modes = [plugin_id] display_modes = [plugin_id]
with self._plugin_modes_lock: with self._plugin_modes_lock:
self.plugin_display_modes[plugin_id] = list(display_modes) self.plugin_display_modes[plugin_id] = list(display_modes)
# Into the runtime snapshot the web interface reads, so its mode
# lists and on-demand lookups see computed modes too (#668).
state_manager = getattr(self.plugin_manager, 'state_manager', None)
record_modes = getattr(state_manager, 'record_modes', None)
if callable(record_modes):
try:
record_modes(plugin_id, list(display_modes))
except Exception as e: # reporting must never break registration
logger.debug("Could not record display modes for %s: %s", plugin_id, e)
# Subscribe to config changes for per-plugin hot-reload. Bind plugin_id # Subscribe to config changes for per-plugin hot-reload. Bind plugin_id
# and instance as defaults so each plugin's callback targets its own # and instance as defaults so each plugin's callback targets its own
+65 -8
View File
@@ -15,7 +15,8 @@ reads through a catalog unchanged. It has nothing that runs a plugin: no
``load_plugin``, ``get_plugin`` or ``plugins``. ``load_plugin``, ``get_plugin`` or ``plugins``.
Runtime state -- whether the display has a plugin loaded, its health, its Runtime state -- whether the display has a plugin loaded, its health, its
errors -- is not here either. The display process publishes what it knows to errors -- is not here either, with one exception: given a ``runtime_source``,
the mode lookups prefer the modes the running display registered. The display process publishes what it knows to
the shared cache (health and resource metrics, the current mode, the error the shared cache (health and resource metrics, the current mode, the error
aggregator snapshot), and the web routes read those publications. What the aggregator snapshot), and the web routes read those publications. What the
display does not publish (which plugins it has loaded, its plugin state display does not publish (which plugins it has loaded, its plugin state
@@ -26,8 +27,9 @@ See docs/ARCHITECTURE.md ("Web and display processes").
import json import json
import threading import threading
import time
from pathlib import Path from pathlib import Path
from typing import Any, Dict, List, Optional, Union, cast from typing import Any, Callable, Dict, List, Optional, Union, cast
from src.common.permission_utils import ( from src.common.permission_utils import (
ensure_directory_permissions, get_plugin_dir_mode, ensure_directory_permissions, get_plugin_dir_mode,
@@ -39,6 +41,10 @@ from src.plugin_system.plugin_dirs import (
PathLike = Union[str, Path] PathLike = Union[str, Path]
#: How long one read of the display's runtime view answers mode lookups. A
#: listing asks once per plugin; the cache copy is a file read each time.
_RUNTIME_VIEW_TTL_SECONDS = 1.0
class PluginCatalog: class PluginCatalog:
"""Manifests, schemas, config and versions of the installed plugins. """Manifests, schemas, config and versions of the installed plugins.
@@ -49,10 +55,17 @@ class PluginCatalog:
""" """
def __init__(self, plugins_dir: PathLike, config_manager: Optional[Any] = None, def __init__(self, plugins_dir: PathLike, config_manager: Optional[Any] = None,
schema_manager: Optional[Any] = None) -> None: schema_manager: Optional[Any] = None,
runtime_source: Optional[Callable[[], Any]] = None) -> None:
self.plugins_dir: Path = Path(plugins_dir) self.plugins_dir: Path = Path(plugins_dir)
self.config_manager = config_manager self.config_manager = config_manager
self.schema_manager = schema_manager self.schema_manager = schema_manager
# Returns the display's PluginRuntimeView
# (src/plugin_system/plugin_runtime.py). Its live view carries the
# modes the display registered, which the mode lookups below prefer
# to the manifest's. None: manifests only.
self.runtime_source = runtime_source
self._runtime_view_memo: Optional[tuple] = None
self.logger = get_logger(__name__) self.logger = get_logger(__name__)
# Guards plugin_manifests/plugin_directories: request threads read # Guards plugin_manifests/plugin_directories: request threads read
@@ -172,23 +185,67 @@ class PluginCatalog:
by_manifest=False) by_manifest=False)
return str(plugin_dir) if plugin_dir is not None else None return str(plugin_dir) if plugin_dir is not None else None
def get_plugin_display_modes(self, plugin_id: str) -> List[str]: def _runtime_view(self) -> Any:
"""The manifest's ``display_modes``, or []. """The display's runtime view, read at most once a second; None
without a source or when reading it fails."""
if self.runtime_source is None:
return None
now = time.monotonic()
memo = self._runtime_view_memo
if memo is not None and now - memo[0] < _RUNTIME_VIEW_TTL_SECONDS:
return memo[1]
try:
view = self.runtime_source()
except Exception as exc: # a lookup must still answer from manifests
self.logger.debug("Could not read the display's runtime view: %s", exc)
view = None
self._runtime_view_memo = (now, view)
return view
What the display actually rotates can differ: a plugin may compute def _live_display_modes(self, plugin_id: str) -> Optional[List[str]]:
its modes at run time (``plugin.modes``). This is the declared list. """The modes the running display registered for ``plugin_id``, or None."""
view = self._runtime_view()
if view is None:
return None
try:
modes = view.display_modes(plugin_id)
except Exception as exc: # includes a source returning something else
self.logger.debug("Could not read display modes for %s: %s", plugin_id, exc)
return None
return list(modes) if isinstance(modes, list) and modes else None
def get_plugin_display_modes(self, plugin_id: str) -> List[str]:
"""The modes the display registered for the plugin, else the
manifest's ``display_modes``, else [].
A plugin may compute its modes at run time (``plugin.modes``): each
league soccer-scoreboard's ``custom_leagues`` adds is a mode no
manifest can list ahead of time (#668). The running display
publishes what it registered, and that wins while the display is
live and has the plugin loaded. Otherwise -- display stopped, plugin
disabled -- the declared list is the best answer there is.
""" """
live = self._live_display_modes(plugin_id)
if live is not None:
return live
with self._lock: with self._lock:
manifest = self.plugin_manifests.get(plugin_id) manifest = self.plugin_manifests.get(plugin_id)
modes = (manifest or {}).get('display_modes', []) modes = (manifest or {}).get('display_modes', [])
return list(modes) if isinstance(modes, list) else [] return list(modes) if isinstance(modes, list) else []
def find_plugin_for_mode(self, mode: str) -> Optional[str]: def find_plugin_for_mode(self, mode: str) -> Optional[str]:
"""The plugin whose manifest declares ``mode`` (case-insensitive).""" """The plugin that registered ``mode`` on the running display, else
the one whose manifest declares it (case-insensitive both ways)."""
wanted = mode.strip().lower() wanted = mode.strip().lower()
with self._lock: with self._lock:
manifests = dict(self.plugin_manifests) manifests = dict(self.plugin_manifests)
for plugin_id in manifests:
live = self._live_display_modes(plugin_id)
if live and any(m.lower() == wanted for m in live):
return plugin_id
for plugin_id, manifest in manifests.items(): for plugin_id, manifest in manifests.items():
if self._live_display_modes(plugin_id):
continue # the display's list is the truth for this plugin
modes = manifest.get('display_modes') modes = manifest.get('display_modes')
if isinstance(modes, list) and any( if isinstance(modes, list) and any(
isinstance(m, str) and m.lower() == wanted for m in modes): isinstance(m, str) and m.lower() == wanted for m in modes):
+29 -1
View File
@@ -56,7 +56,7 @@ import os
import threading import threading
import time import time
from dataclasses import dataclass, field, replace from dataclasses import dataclass, field, replace
from typing import Any, Callable, Dict, Optional from typing import Any, Callable, Dict, List, Optional
from src import display_watchdog from src import display_watchdog
from src.logging_config import get_logger from src.logging_config import get_logger
@@ -100,6 +100,9 @@ _ERROR_MESSAGE_CHARS = 200
_ERROR_TYPE_CHARS = 80 _ERROR_TYPE_CHARS = 80
_ID_CHARS = 100 _ID_CHARS = 100
_VERSION_CHARS = 40 _VERSION_CHARS = 40
#: Bounds on a plugin's published ``modes``: a plugin computes them, so a
#: runaway list must not bloat a file written to the SD card.
_MAX_MODES = 200
#: Reader statuses. Only LIVE carries runtime facts. #: Reader statuses. Only LIVE carries runtime facts.
LIVE = "live" LIVE = "live"
@@ -154,6 +157,15 @@ def summarize_error(error_info: Optional[Dict[str, Any]]) -> Optional[Dict[str,
} }
def _published_modes(modes: Any) -> Optional[List[str]]:
"""The registered display modes as a snapshot carries them, or None."""
if not isinstance(modes, list):
return None
# A name is a key the display matches exactly: drop one too long to
# carry whole rather than clip it into a different name.
return [m for m in modes if isinstance(m, str) and len(m) <= _ID_CHARS][:_MAX_MODES]
def build_runtime_snapshot(state_manager: Any, *, started_at: float, def build_runtime_snapshot(state_manager: Any, *, started_at: float,
now: Optional[float] = None, now: Optional[float] = None,
running: bool = True, running: bool = True,
@@ -173,6 +185,7 @@ def build_runtime_snapshot(state_manager: Any, *, started_at: float,
"error": summarize_error(record.get("error_info")), "error": summarize_error(record.get("error_info")),
"version": _clip(version, _VERSION_CHARS) if version else None, "version": _clip(version, _VERSION_CHARS) if version else None,
"loaded_at": _epoch(record.get("loaded_at")), "loaded_at": _epoch(record.get("loaded_at")),
"modes": _published_modes(record.get("modes")),
} }
return { return {
"schema": SNAPSHOT_SCHEMA, "schema": SNAPSHOT_SCHEMA,
@@ -416,6 +429,21 @@ class PluginRuntimeView:
"loaded_at": record.get("loaded_at"), "loaded_at": record.get("loaded_at"),
} }
def display_modes(self, plugin_id: str) -> Optional[List[str]]:
"""The display modes the display registered for ``plugin_id``: what
it rotates and accepts on-demand, including modes a plugin computes
from its config. None unless the view is live and the plugin is
loaded with its modes registered -- the caller then falls back to
the manifest's ``display_modes``."""
if not self.live:
return None
record = self.plugins.get(plugin_id)
modes = record.get("modes") if isinstance(record, dict) else None
if not isinstance(modes, list):
return None
modes = [m for m in modes if isinstance(m, str)]
return modes or None
def describe(self) -> Dict[str, Any]: def describe(self) -> Dict[str, Any]:
"""The view's own status, for a response to carry beside the facts.""" """The view's own status, for a response to carry beside the facts."""
return { return {
+24 -2
View File
@@ -10,7 +10,7 @@ snapshot ``plugin_runtime.PluginRuntimePublisher`` publishes from it.
import threading import threading
import time import time
from enum import Enum from enum import Enum
from typing import Optional, Dict, Any from typing import Any, Dict, List, Optional
from datetime import datetime from datetime import datetime
import logging import logging
@@ -231,6 +231,26 @@ class PluginStateManager:
} }
self._note_change() self._note_change()
def record_modes(self, plugin_id: str, modes: List[str]) -> None:
"""Record the display modes the display registered for ``plugin_id``.
Called by the DisplayController each time it registers the plugin.
These are the modes it actually rotates and accepts on-demand --
``plugin.modes`` when the plugin computes them (a soccer league the
user added under ``custom_leagues``), else the manifest's list -- and
the web interface has no other way to learn them (#668). Kept on the
loaded record, so an unload or a reload's fresh record_loaded()
forgets them until the plugin is registered again.
"""
with self._lock:
loaded = self._loaded.get(plugin_id)
if loaded is None:
return
modes = [str(m) for m in modes]
if loaded.get('modes') != modes:
loaded['modes'] = modes
self._note_change()
def record_unloaded(self, plugin_id: str) -> None: def record_unloaded(self, plugin_id: str) -> None:
"""Forget the loaded record alone, keeping state and error info: for """Forget the loaded record alone, keeping state and error info: for
an unload that failed after the instance was already dropped.""" an unload that failed after the instance was already dropped."""
@@ -243,7 +263,8 @@ class PluginStateManager:
section so a concurrent load or unload is seen whole or not at all. section so a concurrent load or unload is seen whole or not at all.
Per plugin: ``state`` (published_state()'s value), ``loaded``, Per plugin: ``state`` (published_state()'s value), ``loaded``,
``version`` and ``loaded_at`` (None unless loaded) and ``error_info`` ``version``, ``loaded_at`` and ``modes`` (None unless loaded; ``modes``
also None until the display registers it) and ``error_info``
(a copy, or None). (a copy, or None).
""" """
with self._lock: with self._lock:
@@ -257,6 +278,7 @@ class PluginStateManager:
'loaded': loaded is not None, 'loaded': loaded is not None,
'version': loaded['version'] if loaded else None, 'version': loaded['version'] if loaded else None,
'loaded_at': loaded['loaded_at'] if loaded else None, 'loaded_at': loaded['loaded_at'] if loaded else None,
'modes': list(loaded['modes']) if loaded and 'modes' in loaded else None,
'error_info': dict(info) if info is not None else None, 'error_info': dict(info) if info is not None else None,
} }
return records return records
+11 -15
View File
@@ -4,7 +4,6 @@ Centralized error handling for web interface.
Provides helpers for consistent error responses across API endpoints. Provides helpers for consistent error responses across API endpoints.
""" """
import errno
from typing import Any, Optional from typing import Any, Optional
from flask import jsonify from flask import jsonify
@@ -21,9 +20,10 @@ logger = get_logger(__name__)
_MAX_DETAIL_LENGTH = 400 _MAX_DETAIL_LENGTH = 400
def describe_exception(exc: BaseException) -> str: def describe_exception(exc: BaseException,
max_length: int = _MAX_DETAIL_LENGTH) -> str:
""" """
Machine-readable reason code for an exception, safe to return over HTTP. One-line, safe-to-return description of an exception.
The generic "an error occurred; see logs for details" tells a user nothing The generic "an error occurred; see logs for details" tells a user nothing
and, when the failure is bad enough, the logs are unreachable too: a device and, when the failure is bad enough, the logs are unreachable too: a device
@@ -31,24 +31,20 @@ def describe_exception(exc: BaseException) -> str:
*including* the log viewer, because journalctl could not be executed. The *including* the log viewer, because journalctl could not be executed. The
underlying `[Errno 5] Input/output error` named the fault immediately. underlying `[Errno 5] Input/output error` named the fault immediately.
So the type and errno still go back -- "OSError:EIO", "PermissionError: Returns "TypeName: message", credentials redacted and length capped. The
EACCES", "TimeoutExpired" -- but never the exception's message, which can type alone is worth carrying -- a bare PermissionError says more than any
quote paths, URLs, credentials or a library's internals (CodeQL generic sentence.
py/stack-trace-exposure). The message is logged here instead, so every
reason code a client sees has its full text in the log.
Args: Args:
exc: The exception to describe exc: The exception to describe
max_length: Truncate beyond this many characters
Returns: Returns:
"TypeName" or "TypeName:ERRNO", never empty A single-line description, never empty
""" """
code = type(exc).__name__ message = str(exc).strip()
exc_errno = getattr(exc, 'errno', None) text = f"{type(exc).__name__}: {message}" if message else type(exc).__name__
if isinstance(exc_errno, int) and exc_errno in errno.errorcode: return redact_text(text, max_length)
code = f"{code}:{errno.errorcode[exc_errno]}"
logger.warning("Error reported to the client as %s: %s", code, redact_text(str(exc)))
return code
def redact_text(text: str, max_length: int = _MAX_DETAIL_LENGTH) -> str: def redact_text(text: str, max_length: int = _MAX_DETAIL_LENGTH) -> str:
+9 -9
View File
@@ -1369,7 +1369,7 @@ class WiFiManager:
self.enable_ap_mode(force=True) self.enable_ap_mode(force=True)
except Exception as ap_error: # nosec B110 - last-resort; do not re-raise, but log for debugging except Exception as ap_error: # nosec B110 - last-resort; do not re-raise, but log for debugging
logger.error("Last-resort AP mode enable failed in recovery path: %s", ap_error, exc_info=True) logger.error("Last-resort AP mode enable failed in recovery path: %s", ap_error, exc_info=True)
return False, f"Connection failed ({type(e).__name__}); see logs for details" return False, str(e)
def _failsafe_ap(self, enabled_msg: str, failed_msg: str) -> Tuple[bool, str]: def _failsafe_ap(self, enabled_msg: str, failed_msg: str) -> Tuple[bool, str]:
"""Force the setup AP up after a connect that left no working network, """Force the setup AP up after a connect that left no working network,
@@ -1585,7 +1585,7 @@ class WiFiManager:
except Exception as e: except Exception as e:
logger.error(f"Error connecting with nmcli: {e}") logger.error(f"Error connecting with nmcli: {e}")
self._show_led_message("Connection error", duration=5) self._show_led_message("Connection error", duration=5)
return False, f"Connection failed ({type(e).__name__}); see logs for details" return False, str(e)
# 802.11 caps an SSID at 32 octets. Control characters cannot appear in a # 802.11 caps an SSID at 32 octets. Control characters cannot appear in a
# real one, and a leading "-" would be read by nmcli as an option rather # real one, and a leading "-" would be read by nmcli as an option rather
@@ -1725,7 +1725,7 @@ class WiFiManager:
return False, "nmcli is required to disconnect from WiFi" return False, "nmcli is required to disconnect from WiFi"
except Exception as e: except Exception as e:
logger.error(f"Error disconnecting from WiFi: {e}") logger.error(f"Error disconnecting from WiFi: {e}")
return False, f"Disconnect failed ({type(e).__name__}); see logs for details" return False, str(e)
def _ensure_wifi_radio_enabled(self, max_retries: int = 3) -> bool: def _ensure_wifi_radio_enabled(self, max_retries: int = 3) -> bool:
""" """
@@ -2004,7 +2004,7 @@ class WiFiManager:
return False, "No WiFi tools available (nmcli, hostapd, or dnsmasq required)" return False, "No WiFi tools available (nmcli, hostapd, or dnsmasq required)"
except Exception as e: except Exception as e:
logger.error(f"Error in enable_ap_mode: {e}") logger.error(f"Error in enable_ap_mode: {e}")
return False, f"Could not enable AP mode ({type(e).__name__}); see logs for details" return False, str(e)
def _mark_forced(self) -> None: def _mark_forced(self) -> None:
"""Record that AP mode was forced on, so the periodic check leaves it """Record that AP mode was forced on, so the periodic check leaves it
@@ -2099,10 +2099,10 @@ class WiFiManager:
return True, "AP mode enabled" return True, "AP mode enabled"
except Exception as e: except Exception as e:
logger.error(f"Error starting AP services: {e}") logger.error(f"Error starting AP services: {e}")
return False, f"Could not enable AP mode ({type(e).__name__}); see logs for details" return False, str(e)
except Exception as e: except Exception as e:
logger.error(f"Error enabling AP mode: {e}") logger.error(f"Error enabling AP mode: {e}")
return False, f"Could not enable AP mode ({type(e).__name__}); see logs for details" return False, str(e)
def _enable_ap_mode_nmcli_hotspot(self) -> Tuple[bool, str]: def _enable_ap_mode_nmcli_hotspot(self) -> Tuple[bool, str]:
""" """
@@ -2227,7 +2227,7 @@ class WiFiManager:
logger.error(f"Error starting AP mode with nmcli: {e}") logger.error(f"Error starting AP mode with nmcli: {e}")
self._remove_nm_dnsmasq_captive_conf() self._remove_nm_dnsmasq_captive_conf()
self._show_led_message("Setup mode error", duration=5) self._show_led_message("Setup mode error", duration=5)
return False, f"Could not enable AP mode ({type(e).__name__}); see logs for details" return False, str(e)
def _get_ap_status_nmcli(self) -> Dict: def _get_ap_status_nmcli(self) -> Dict:
""" """
@@ -2409,10 +2409,10 @@ class WiFiManager:
return True, "AP mode disabled" return True, "AP mode disabled"
except Exception as e: except Exception as e:
logger.error(f"Error stopping AP services: {e}") logger.error(f"Error stopping AP services: {e}")
return False, f"Could not disable AP mode ({type(e).__name__}); see logs for details" return False, str(e)
except Exception as e: except Exception as e:
logger.error(f"Error disabling AP mode: {e}") logger.error(f"Error disabling AP mode: {e}")
return False, f"Could not disable AP mode ({type(e).__name__}); see logs for details" return False, str(e)
def _create_hostapd_config(self): def _create_hostapd_config(self):
"""Create hostapd configuration file""" """Create hostapd configuration file"""
+13 -3
View File
@@ -7,7 +7,7 @@ manifest.json off disk and reimplemented PluginManager's own fallbacks.
""" """
import json import json
from unittest.mock import MagicMock from unittest.mock import MagicMock, patch
import pytest import pytest
@@ -147,8 +147,7 @@ class TestOneBadConfigSectionDoesNotBlankTheList:
side_effect=RuntimeError("disk is gone")) side_effect=RuntimeError("disk is gone"))
resp = api_v3_client.get('/api/v3/display/modes') resp = api_v3_client.get('/api/v3/display/modes')
assert resp.status_code == 500 assert resp.status_code == 500
assert resp.get_json()['details'] == 'RuntimeError' assert 'disk is gone' in resp.get_json()['details']
assert 'disk is gone' not in json.dumps(resp.get_json())
def test_credentials_in_the_exception_are_redacted(self, api_v3_module, api_v3_client): def test_credentials_in_the_exception_are_redacted(self, api_v3_module, api_v3_client):
"""describe_exception is what makes returning detail safe.""" """describe_exception is what makes returning detail safe."""
@@ -156,3 +155,14 @@ class TestOneBadConfigSectionDoesNotBlankTheList:
side_effect=RuntimeError("GET https://x/y?api_key=SEC123 failed")) side_effect=RuntimeError("GET https://x/y?api_key=SEC123 failed"))
body = api_v3_client.get('/api/v3/display/modes').get_json() body = api_v3_client.get('/api/v3/display/modes').get_json()
assert 'SEC123' not in json.dumps(body) assert 'SEC123' not in json.dumps(body)
class TestOnDemandUsesTheRegisteredSpelling:
def test_a_mode_differing_in_case_is_sent_as_registered(self, client):
with patch('web_interface.blueprints.api_v3.display._deliver_on_demand',
return_value=('socket', None)) as deliver:
response = client.post('/api/v3/display/on-demand/start',
json={'plugin_id': 'football-scoreboard',
'mode': 'NFL_LIVE', 'start_service': False})
assert response.status_code == 200, response.get_json()
assert deliver.call_args.args[0]['mode'] == 'nfl_live'
-147
View File
@@ -1,147 +0,0 @@
"""No API response carries an exception's message (CodeQL py/stack-trace-exposure).
One representative route per file that had open alerts. Each forces a failure
whose message holds a marker and asserts the marker is nowhere in the body:
the message goes to the log, the client gets a fixed message plus a reason
code (describe_exception: the type, and the errno for an OSError).
"""
import json
import sys
from pathlib import Path
from unittest.mock import MagicMock, patch
import pytest
sys.path.insert(0, str(Path(__file__).parent.parent))
from test._api_v3_test_helpers import api_v3_client, api_v3_module # noqa: F401,E402
LEAK = "LEAKED-/home/pi/secret token=abc123"
API = "web_interface.blueprints.api_v3"
def _assert_no_leak(response):
body = response.get_data(as_text=True)
assert "LEAKED" not in body, body
assert "abc123" not in body, body
return json.loads(body)
def test_display_service_status_drops_systemctl_output(api_v3_module, api_v3_client,
monkeypatch):
"""display.py: the on-demand routes return the service status verbatim."""
api_v3_module.api_v3.cache_manager.get.return_value = None
monkeypatch.setattr(f"{API}.display.display_state.read_state", lambda: None)
with patch(f"{API}.subprocess.run", side_effect=OSError(13, LEAK)):
body = _assert_no_leak(api_v3_client.get("/api/v3/display/on-demand/status"))
assert body["data"]["service"] == {"active": False, "returncode": -1}
@pytest.mark.parametrize("helper", ["_ensure_display_service_running",
"_stop_display_service"])
def test_service_results_keep_returncode_but_not_output(api_v3_module, helper):
"""display.py start/stop: returncode/active/started stay, stdout/stderr go."""
failed = MagicMock(returncode=1, stdout=LEAK, stderr=LEAK)
with patch(f"{API}.subprocess.run", return_value=failed):
result = getattr(api_v3_module, helper)()
assert "LEAKED" not in json.dumps(result)
assert result["returncode"] == 1 and result["active"] is False
assert "stdout" not in result and "stderr" not in result
def test_wifi_connect_failure(api_v3_client):
"""wifi.py: a raising connect, and the attempt /wifi/status reports after."""
with patch("src.wifi_manager.WiFiManager") as cls:
cls.return_value._is_ap_mode_active.return_value = False
cls.return_value.connect_to_network.side_effect = RuntimeError(LEAK)
body = _assert_no_leak(api_v3_client.post(
"/api/v3/wifi/connect", json={"ssid": "HomeNet", "password": "pw"}))
assert body["details"] == "RuntimeError"
cls.return_value.get_wifi_status.return_value = MagicMock(
connected=False, ssid=None, ip_address=None, signal=0, ap_mode_active=False)
cls.return_value.config = {}
status = _assert_no_leak(api_v3_client.get("/api/v3/wifi/status"))
assert status["data"]["last_connect_attempt"]["message"] == (
"Failed to connect to network (RuntimeError)")
def test_wifi_manager_messages_carry_no_exception_text():
"""src/wifi_manager.py: its (success, message) is what the wifi routes return."""
from src.wifi_manager import WiFiManager
manager = WiFiManager.__new__(WiFiManager) # no __init__: no host access
manager.get_wifi_status = MagicMock(side_effect=OSError(5, LEAK))
success, message = manager.disconnect_from_network()
assert success is False
assert "LEAKED" not in message and "OSError" in message
def test_system_action_exception(api_v3_client):
"""system.py: execute_system_action's catch-all."""
with patch("subprocess.run", side_effect=OSError(5, LEAK)):
body = _assert_no_leak(api_v3_client.post(
"/api/v3/system/action", json={"action": "stop_display"}))
assert body["details"] == "OSError:EIO"
def test_calendar_registration_failure(api_v3_client, tmp_path, monkeypatch):
"""plugin_calendar.py: the auth script could not be run."""
plugin_dir = tmp_path / "calendar"
plugin_dir.mkdir()
(plugin_dir / "credentials.json").write_text("{}", encoding="utf-8")
(plugin_dir / "calendar_registration.py").write_text("", encoding="utf-8")
monkeypatch.setattr(f"{API}._calendar_plugin_dir", lambda: plugin_dir)
with patch(f"{API}.subprocess.run", side_effect=OSError(13, LEAK)):
body = _assert_no_leak(api_v3_client.post(
"/api/v3/plugins/calendar/authenticate", json={"code": "x"}))
assert "EACCES" in body["message"]
def test_health_failure(api_v3_client, monkeypatch):
"""misc.py: get_health's catch-all."""
def boom():
raise RuntimeError(LEAK)
monkeypatch.setattr(f"{API}.misc._get_display_service_status", boom)
body = _assert_no_leak(api_v3_client.get("/api/v3/health"))
assert body["details"] == "RuntimeError"
def test_config_route_failure(api_v3_module, api_v3_client):
"""error_handler.py: create_error_response, as config.py's routes use it."""
api_v3_module.api_v3.config_manager.load_config.side_effect = RuntimeError(LEAK)
body = _assert_no_leak(api_v3_client.get("/api/v3/config/schedule"))
assert body["details"] == "RuntimeError"
def test_plugin_route_failure(api_v3_module, api_v3_client):
"""plugins.py: an unhandled error in a plugin route."""
api_v3_module.api_v3.plugin_catalog.get_all_plugin_info.side_effect = RuntimeError(LEAK)
body = _assert_no_leak(api_v3_client.get("/api/v3/plugins/installed"))
assert body["details"] == "RuntimeError"
def test_starlark_route_failure(api_v3_client):
"""starlark.py: one of its catch-alls."""
with patch(f"{API}._get_starlark_plugin", side_effect=RuntimeError(LEAK)):
body = _assert_no_leak(api_v3_client.get("/api/v3/starlark/status"))
assert body["details"] == "RuntimeError"
def test_unit_refresh_failure(monkeypatch):
"""system.py git_pull: perform_core_update appends unit_refresh's message."""
from web_interface import unit_refresh
def boom(*_a, **_k):
raise RuntimeError(LEAK)
monkeypatch.setattr(unit_refresh, "stale_units", boom)
result = unit_refresh.refresh_after_update()
assert result["status"] == unit_refresh.FAILED
assert "LEAKED" not in result["message"]
def test_install_base_requirements_failure(api_v3_client):
"""system.py: a pip install that could not start, in the action's output."""
with patch(f"{API}.system._pip_install_requirements", side_effect=OSError(5, LEAK)):
body = _assert_no_leak(api_v3_client.post(
"/api/v3/system/action", json={"action": "install_base_requirements"}))
assert "Failed: OSError:EIO" in body["output"]
+3 -4
View File
@@ -95,10 +95,9 @@ class TestRefreshPluginStore:
RuntimeError("failed at /home/user/LEDMatrix/src/secret.py line 42")) RuntimeError("failed at /home/user/LEDMatrix/src/secret.py line 42"))
body = api_v3_client.post(self.URL, json={}).get_json() body = api_v3_client.post(self.URL, json={}).get_json()
assert "Traceback" not in str(body) assert "Traceback" not in str(body)
# `details` is describe_exception output: the type, never the # `details` is describe_exception output: one line, type-named,
# message or a stack. # credential-redacted. It may quote the message, but never a stack.
assert body["details"] == "RuntimeError" assert body["details"].startswith("RuntimeError:")
assert "secret.py" not in str(body)
assert "\n" not in body["details"] assert "\n" not in body["details"]
+265
View File
@@ -0,0 +1,265 @@
"""The web interface sees the display modes the display actually registered (#668).
A plugin may compute its modes from its config: soccer-scoreboard registers
``soccer_<league>_live/recent/upcoming`` for every league the user adds under
``custom_leagues``, and no manifest can list those ahead of time. The display
always rotated them -- DisplayController._register_loaded_plugin prefers
``plugin.modes`` -- but the web process reads plugins as files, so its mode
listing (/display/modes, the on-demand dialog) and find_plugin_for_mode
(/display/on-demand/start with a mode and no plugin_id) saw only manifests.
The display now records each plugin's registered modes in its plugin state,
the runtime snapshot carries them, and PluginCatalog prefers them while the
snapshot is live, falling back to the manifest when it is not.
"""
import json
import sys
from pathlib import Path
from unittest.mock import MagicMock
import pytest
sys.path.insert(0, str(Path(__file__).parent.parent))
from src.cache_manager import CacheManager # noqa: E402
from src.plugin_system import plugin_runtime as rt # noqa: E402
from src.plugin_system.plugin_catalog import PluginCatalog # noqa: E402
from src.plugin_system.plugin_runtime import ( # noqa: E402
PluginRuntimePublisher, build_runtime_snapshot, read_plugin_runtime,
view_from_snapshot,
)
from src.plugin_system.plugin_state import PluginState, PluginStateManager # noqa: E402
from test._api_v3_test_helpers import api_v3_client, api_v3_module # noqa: F401,E402
DECLARED = ["soccer_eng.1_live", "soccer_eng.1_recent", "soccer_eng.1_upcoming"]
CUSTOM = ["soccer_sco.1_live", "soccer_sco.1_recent", "soccer_sco.1_upcoming"]
REGISTERED = DECLARED + CUSTOM
def _loaded_states(modes=None):
states = PluginStateManager()
states.set_state("soccer-scoreboard", PluginState.ENABLED)
states.record_loaded("soccer-scoreboard", "2.24.1")
if modes is not None:
states.record_modes("soccer-scoreboard", modes)
return states
@pytest.fixture
def shared_cache(tmp_path, monkeypatch):
"""Two cache managers over one directory: the display's and the web's."""
monkeypatch.setattr(CacheManager, "_get_writable_cache_dir",
lambda self: str(tmp_path / "cache"))
(tmp_path / "cache").mkdir()
display_cache, web_cache = CacheManager(), CacheManager()
yield display_cache, web_cache
display_cache.stop_cleanup_thread()
web_cache.stop_cleanup_thread()
@pytest.fixture
def plugins_dir(tmp_path):
root = tmp_path / "plugins"
for plugin_id, modes in (("soccer-scoreboard", DECLARED), ("clock-simple", ["clock"])):
(root / plugin_id).mkdir(parents=True)
(root / plugin_id / "manifest.json").write_text(json.dumps({
"id": plugin_id, "name": plugin_id, "version": "1.0.0",
"class_name": "P", "display_modes": modes}), encoding="utf-8")
return root
# --- The display records what it registered ---------------------------------
class TestStateManagerRecordsModes:
def test_runtime_records_carry_them(self):
assert _loaded_states(REGISTERED).runtime_records()[
"soccer-scoreboard"]["modes"] == REGISTERED
def test_none_until_registered(self):
assert _loaded_states().runtime_records()["soccer-scoreboard"]["modes"] is None
def test_a_new_list_is_a_change_the_same_one_is_not(self):
"""change_count drives the publisher: re-registering an unchanged
plugin must not cost an SD-card write."""
states = _loaded_states(DECLARED)
before = states.change_count
states.record_modes("soccer-scoreboard", list(DECLARED))
assert states.change_count == before
states.record_modes("soccer-scoreboard", REGISTERED)
assert states.change_count == before + 1
def test_ignored_for_a_plugin_that_is_not_loaded(self):
states = PluginStateManager()
states.record_modes("ghost", ["ghost"])
assert "ghost" not in states.runtime_records()
def test_unload_forgets_them(self):
states = _loaded_states(REGISTERED)
states.clear_state("soccer-scoreboard")
assert "soccer-scoreboard" not in states.runtime_records()
def test_a_reload_starts_without_them_until_registered_again(self):
states = _loaded_states(REGISTERED)
states.record_loaded("soccer-scoreboard", "2.25.0")
assert states.runtime_records()["soccer-scoreboard"]["modes"] is None
class TestControllerRecordsOnRegistration:
def test_plugin_modes_reach_the_state_manager(self, test_display_controller):
"""_register_loaded_plugin is the one path every load, enable and
reload goes through."""
c = test_display_controller
states = _loaded_states()
plugin = MagicMock()
plugin.modes = list(REGISTERED)
c.plugin_manager.state_manager = states
c.plugin_manager.get_plugin = MagicMock(return_value=plugin)
c.plugin_manager.plugin_manifests = {"soccer-scoreboard": {"display_modes": DECLARED}}
c._register_loaded_plugin("soccer-scoreboard")
assert states.runtime_records()["soccer-scoreboard"]["modes"] == REGISTERED
def test_a_failing_state_manager_does_not_break_registration(self, test_display_controller):
c = test_display_controller
plugin = MagicMock()
plugin.modes = ["clock"]
c.plugin_manager.state_manager.record_modes = MagicMock(side_effect=RuntimeError("x"))
c.plugin_manager.get_plugin = MagicMock(return_value=plugin)
c.plugin_manager.plugin_manifests = {}
assert c._register_loaded_plugin("clock-simple") == ["clock"]
assert c.mode_to_plugin_id["clock"] == "clock-simple"
# --- The snapshot carries them; only a live view reports them ---------------
class TestSnapshotAndView:
NOW = 1_800_000_000.0
def _view(self, states, running=True, published_at=None):
snapshot = build_runtime_snapshot(states, started_at=1.0, now=self.NOW,
running=running)
if published_at is not None:
snapshot["published_at"] = published_at
return view_from_snapshot(snapshot, now=self.NOW)
def test_live_view_reports_the_registered_modes(self):
assert self._view(_loaded_states(REGISTERED)).display_modes(
"soccer-scoreboard") == REGISTERED
def test_stale_and_stopped_views_report_nothing(self):
states = _loaded_states(REGISTERED)
assert self._view(states, published_at=self.NOW - 10_000).display_modes(
"soccer-scoreboard") is None
assert self._view(states, running=False).display_modes("soccer-scoreboard") is None
def test_unregistered_or_unknown_plugins_report_nothing(self):
view = self._view(_loaded_states())
assert view.display_modes("soccer-scoreboard") is None
assert view.display_modes("not-loaded") is None
def test_a_runaway_list_is_bounded(self):
modes = [f"m{i}" for i in range(1000)] + ["x" * 500]
snapshot = build_runtime_snapshot(_loaded_states(modes), started_at=1.0, now=self.NOW)
published = snapshot["plugins"]["soccer-scoreboard"]["modes"]
assert len(published) == rt._MAX_MODES
def test_a_mode_name_is_kept_whole_or_dropped(self):
long_mode = "x" * (rt._ID_CHARS + 1)
snapshot = build_runtime_snapshot(_loaded_states(["ok", long_mode]),
started_at=1.0, now=self.NOW)
assert snapshot["plugins"]["soccer-scoreboard"]["modes"] == ["ok"]
def test_non_strings_from_a_hand_made_snapshot_are_dropped(self):
snapshot = {"schema": rt.SNAPSHOT_SCHEMA, "running": True,
"published_at": self.NOW, "plugins": {
"p": {"loaded": True, "modes": ["a", 3, None]}}}
assert view_from_snapshot(snapshot, now=self.NOW).display_modes("p") == ["a"]
# --- The web's catalog prefers them -------------------------------------------
class TestCatalog:
def _catalog(self, plugins_dir, web_cache):
catalog = PluginCatalog(plugins_dir,
runtime_source=lambda: read_plugin_runtime(web_cache))
catalog.discover_plugins()
return catalog
def test_live_display_modes_win_over_the_manifest(self, plugins_dir, shared_cache):
display_cache, web_cache = shared_cache
PluginRuntimePublisher(display_cache, _loaded_states(REGISTERED)).tick()
catalog = self._catalog(plugins_dir, web_cache)
assert catalog.get_plugin_display_modes("soccer-scoreboard") == REGISTERED
def test_a_custom_league_mode_resolves_to_its_plugin(self, plugins_dir, shared_cache):
"""What /display/on-demand/start does with a mode and no plugin_id."""
display_cache, web_cache = shared_cache
PluginRuntimePublisher(display_cache, _loaded_states(REGISTERED)).tick()
catalog = self._catalog(plugins_dir, web_cache)
assert catalog.find_plugin_for_mode("SOCCER_SCO.1_LIVE") == "soccer-scoreboard"
def test_a_plugin_the_display_has_not_loaded_falls_back_to_its_manifest(
self, plugins_dir, shared_cache):
display_cache, web_cache = shared_cache
PluginRuntimePublisher(display_cache, _loaded_states(REGISTERED)).tick()
catalog = self._catalog(plugins_dir, web_cache)
assert catalog.get_plugin_display_modes("clock-simple") == ["clock"]
assert catalog.find_plugin_for_mode("clock") == "clock-simple"
def test_a_mode_the_display_dropped_does_not_resolve_by_manifest(
self, plugins_dir, shared_cache):
display_cache, web_cache = shared_cache
PluginRuntimePublisher(display_cache, _loaded_states(CUSTOM)).tick()
catalog = self._catalog(plugins_dir, web_cache)
assert catalog.find_plugin_for_mode("soccer_eng.1_live") is None
def test_a_stopped_display_falls_back_to_manifests(self, plugins_dir, shared_cache):
display_cache, web_cache = shared_cache
publisher = PluginRuntimePublisher(display_cache, _loaded_states(REGISTERED))
publisher.tick()
publisher.stop()
catalog = self._catalog(plugins_dir, web_cache)
assert catalog.get_plugin_display_modes("soccer-scoreboard") == DECLARED
assert catalog.find_plugin_for_mode("soccer_sco.1_live") is None
def test_no_runtime_source_is_manifests_only(self, plugins_dir):
catalog = PluginCatalog(plugins_dir)
catalog.discover_plugins()
assert catalog.get_plugin_display_modes("soccer-scoreboard") == DECLARED
def test_a_failing_runtime_source_is_manifests_only(self, plugins_dir):
def broken():
raise OSError("cache gone")
catalog = PluginCatalog(plugins_dir, runtime_source=broken)
catalog.discover_plugins()
assert catalog.get_plugin_display_modes("soccer-scoreboard") == DECLARED
def test_one_listing_reads_the_view_once(self, plugins_dir):
source = MagicMock(return_value=None)
catalog = PluginCatalog(plugins_dir, runtime_source=source)
catalog.discover_plugins()
for _ in range(10):
catalog.get_plugin_display_modes("soccer-scoreboard")
catalog.find_plugin_for_mode("clock")
assert source.call_count == 1
class TestDisplayModesRoute:
def test_lists_the_custom_league_modes(self, api_v3_module, api_v3_client, # noqa: F811
plugins_dir, shared_cache):
display_cache, web_cache = shared_cache
PluginRuntimePublisher(display_cache, _loaded_states(REGISTERED)).tick()
api = api_v3_module.api_v3
api.plugin_catalog = PluginCatalog(
plugins_dir, runtime_source=lambda: read_plugin_runtime(web_cache))
api.config_manager.load_config = MagicMock(return_value={
"soccer-scoreboard": {"enabled": True}})
response = api_v3_client.get("/api/v3/display/modes")
assert response.status_code == 200, response.get_data(as_text=True)
modes = {m["mode"]: m for m in response.get_json()["data"]["modes"]}
assert set(modes) == set(REGISTERED)
assert modes["soccer_sco.1_live"]["plugin_id"] == "soccer-scoreboard"
+1 -1
View File
@@ -162,7 +162,7 @@ class TestPublisher:
assert snapshot["stale_after"] == rt.STALE_AFTER assert snapshot["stale_after"] == rt.STALE_AFTER
assert snapshot["plugins"] == {"clock": { assert snapshot["plugins"] == {"clock": {
"loaded": True, "state": "enabled", "error": None, "loaded": True, "state": "enabled", "error": None,
"version": "1.0.0", "loaded_at": 10.0}} "version": "1.0.0", "loaded_at": 10.0, "modes": None}}
def test_changes_are_throttled_and_quiet_displays_refresh(self): def test_changes_are_throttled_and_quiet_displays_refresh(self):
cache = MagicMock() cache = MagicMock()
+42 -12
View File
@@ -8,7 +8,9 @@ loses those tests with it.
The parity class is what keeps "byte-identical" true after this lands. Point The parity class is what keeps "byte-identical" true after this lands. Point
LEDMATRIX_PLUGINS at a ledmatrix-plugins checkout and every promoted body is LEDMATRIX_PLUGINS at a ledmatrix-plugins checkout and every promoted body is
compared, as a docstring-stripped AST, against every plugin copy that carries compared, as a docstring-stripped AST, against every plugin copy that carries
it. Without the variable it skips rather than fails, since core CI has no it. A copy that is gone counts as adopted when the plugin imports
src.common.sports_helpers (plugins#563/#564 did that for every scoreboard).
Without the variable it skips rather than fails, since core CI has no
plugins checkout; ledmatrix-plugins CI runs the same comparison against core plugins checkout; ledmatrix-plugins CI runs the same comparison against core
(scripts/check_sports_helpers_parity.py, ledmatrix-plugins#495). (scripts/check_sports_helpers_parity.py, ledmatrix-plugins#495).
""" """
@@ -572,6 +574,24 @@ def _core_definitions():
return out return out
def _sports_source(root, sport):
return (root / f"{sport}-scoreboard" / "sports.py").read_text(encoding="utf-8")
def _adopted(source):
"""Gone is fine once the plugin uses the module; otherwise the finder is
not seeing its copy."""
name = sports_helpers.__name__
for node in ast.walk(ast.parse(source)):
if isinstance(node, ast.ImportFrom):
if node.module == name or any(
f"{node.module}.{a.name}" == name for a in node.names):
return True
elif isinstance(node, ast.Import) and any(a.name == name for a in node.names):
return True
return False
class TestParityWithPlugins: class TestParityWithPlugins:
@pytest.mark.parametrize("name", sorted(PROMOTED)) @pytest.mark.parametrize("name", sorted(PROMOTED))
def test_body_matches_every_plugin_copy(self, name): def test_body_matches_every_plugin_copy(self, name):
@@ -580,11 +600,11 @@ class TestParityWithPlugins:
ours = _dump(_core_definitions()[name]) ours = _dump(_core_definitions()[name])
drifted, missing = [], [] drifted, missing = [], []
for sport in carriers: for sport in carriers:
defs = _definitions(ast.parse( source = _sports_source(root, sport)
(root / f"{sport}-scoreboard" / "sports.py").read_text(encoding="utf-8"))) theirs = _definitions(ast.parse(source))[where].get(plugin_name)
theirs = defs[where].get(plugin_name)
if theirs is None: if theirs is None:
missing.append(sport) if not _adopted(source):
missing.append(sport)
elif _dump(theirs) != ours: elif _dump(theirs) != ours:
drifted.append(sport) drifted.append(sport)
assert missing == [], f"{plugin_name} no longer in: {missing}" assert missing == [], f"{plugin_name} no longer in: {missing}"
@@ -594,10 +614,20 @@ class TestParityWithPlugins:
@pytest.mark.parametrize("sport", SCOREBOARDS) @pytest.mark.parametrize("sport", SCOREBOARDS)
def test_constants_match(self, sport): def test_constants_match(self, sport):
root = _plugins_root() source = _sports_source(_plugins_root(), sport)
defs = _definitions(ast.parse( defs = _definitions(ast.parse(source))
(root / f"{sport}-scoreboard" / "sports.py").read_text(encoding="utf-8"))) expected = {
assert ast.literal_eval(defs["module"]["_MIN_WINDOW_DAYS"].value) == MIN_WINDOW_DAYS ("module", "_MIN_WINDOW_DAYS"): MIN_WINDOW_DAYS,
assert ast.literal_eval(defs["module"]["_MAX_WINDOW_DAYS"].value) == MAX_WINDOW_DAYS ("module", "_MAX_WINDOW_DAYS"): MAX_WINDOW_DAYS,
gap = defs["SportsCore"]["_DWELL_REENTRY_GAP_SECONDS"].value ("SportsCore", "_DWELL_REENTRY_GAP_SECONDS"):
assert math.isclose(ast.literal_eval(gap), SportsHelpersMixin._DWELL_REENTRY_GAP_SECONDS) SportsHelpersMixin._DWELL_REENTRY_GAP_SECONDS,
}
missing = []
for (where, name), value in expected.items():
node = defs[where].get(name)
if node is None:
if not _adopted(source):
missing.append(name)
else:
assert math.isclose(ast.literal_eval(node.value), value), name
assert missing == [], f"not found in {sport}: {missing}"
+27 -31
View File
@@ -11,32 +11,26 @@ than even logging it.
import pytest import pytest
from src.web_interface.error_handler import describe_exception, redact_text from src.web_interface.error_handler import describe_exception
class TestDescribeException: class TestDescribeException:
"""describe_exception is a reason code: type and errno, never the message. def test_names_the_type_and_message(self):
detail = describe_exception(OSError(5, "Input/output error", "systemctl"))
assert detail == "OSError: [Errno 5] Input/output error: 'systemctl'"
The message can quote paths, URLs or credentials (CodeQL def test_the_reported_failure_is_legible(self):
py/stack-trace-exposure), so it goes to the log; the code still names the # The whole point: this string is the diagnosis.
fault, as "[Errno 5]" did. assert "Input/output error" in describe_exception(
""" OSError(5, "Input/output error", "systemctl"))
def test_an_oserror_names_its_errno(self):
assert describe_exception(
OSError(5, "Input/output error", "systemctl")) == "OSError:EIO"
def test_a_bare_exception_still_names_its_type(self): def test_a_bare_exception_still_names_its_type(self):
# A PermissionError with no message still says more than "unknown".
assert describe_exception(PermissionError()) == "PermissionError" assert describe_exception(PermissionError()) == "PermissionError"
assert describe_exception(Exception()) == "Exception" assert describe_exception(Exception()) == "Exception"
def test_the_message_never_reaches_the_code(self): def test_message_is_kept_when_present(self):
assert describe_exception(ValueError("bad port /etc/secret")) == "ValueError" assert describe_exception(ValueError("bad port")) == "ValueError: bad port"
def test_the_message_is_logged_instead(self, caplog):
describe_exception(RuntimeError("disk on fire token=abc123"))
assert "disk on fire" in caplog.text
assert "abc123" not in caplog.text
class TestCredentialRedaction: class TestCredentialRedaction:
@@ -62,44 +56,47 @@ class TestCredentialRedaction:
("authorization: barecredential", "barecredential"), ("authorization: barecredential", "barecredential"),
]) ])
def test_credentials_never_reach_the_response(self, secret_text, leaked): def test_credentials_never_reach_the_response(self, secret_text, leaked):
detail = redact_text(secret_text) detail = describe_exception(RuntimeError(secret_text))
assert leaked not in detail assert leaked not in detail
assert "<redacted>" in detail assert "<redacted>" in detail
def test_the_parameter_name_survives_redaction(self): def test_the_parameter_name_survives_redaction(self):
# Knowing *which* credential was involved is part of the diagnosis. # Knowing *which* credential was involved is part of the diagnosis.
detail = redact_text("https://x/y?api_key=SEC123") detail = describe_exception(RuntimeError("https://x/y?api_key=SEC123"))
assert "api_key" in detail assert "api_key" in detail
def test_unknown_schemes_keep_their_name(self): def test_unknown_schemes_keep_their_name(self):
for scheme in ("ApiKey", "Negotiate", "NTLM", "AWS4-HMAC-SHA256"): for scheme in ("ApiKey", "Negotiate", "NTLM", "AWS4-HMAC-SHA256"):
detail = redact_text("Authorization: %s SECRETVALUE" % scheme) detail = describe_exception(
RuntimeError("Authorization: %s SECRETVALUE" % scheme))
assert scheme in detail, detail assert scheme in detail, detail
assert "SECRETVALUE" not in detail, detail assert "SECRETVALUE" not in detail, detail
def test_auth_scheme_and_username_survive(self): def test_auth_scheme_and_username_survive(self):
# Which kind of credential, and whose, without the credential itself. # Which kind of credential, and whose, without the credential itself.
assert "Bearer" in redact_text("Authorization: Bearer eyJ.SECRET.sig") assert "Bearer" in describe_exception(
assert "user" in redact_text("https://user:hunter2@example.com") RuntimeError("Authorization: Bearer eyJ.SECRET.sig"))
assert "user" in describe_exception(
RuntimeError("https://user:hunter2@example.com"))
def test_non_secret_context_is_preserved(self): def test_non_secret_context_is_preserved(self):
detail = redact_text("https://api.x.com/v1?city=Tampa") detail = describe_exception(RuntimeError("https://api.x.com/v1?city=Tampa"))
assert "city=Tampa" in detail assert "city=Tampa" in detail
assert "<redacted>" not in detail assert "<redacted>" not in detail
class TestBounds: class TestBounds:
def test_long_messages_are_truncated(self): def test_long_messages_are_truncated(self):
detail = redact_text("x" * 5000) detail = describe_exception(ValueError("x" * 5000))
assert len(detail) <= 400 assert len(detail) <= 400
def test_newlines_are_collapsed_to_one_line(self): def test_newlines_are_collapsed_to_one_line(self):
detail = redact_text("line one\nline two\tthree") detail = describe_exception(ValueError("line one\nline two\tthree"))
assert "\n" not in detail and "\t" not in detail assert "\n" not in detail and "\t" not in detail
assert detail == "line one line two three" assert detail == "ValueError: line one line two three"
def test_custom_length_is_honoured(self): def test_custom_length_is_honoured(self):
assert len(redact_text("y" * 500, max_length=50)) <= 50 assert len(describe_exception(ValueError("y" * 500), max_length=50)) <= 50
class TestHandlersCarryDetail: class TestHandlersCarryDetail:
@@ -322,10 +319,10 @@ class TestHandlersCarryDetail:
assert resp.status_code == 405, "a wrong method must stay a 405" assert resp.status_code == 405, "a wrong method must stay a 405"
assert resp.get_json()["error_code"] == "METHOD_NOT_ALLOWED" assert resp.get_json()["error_code"] == "METHOD_NOT_ALLOWED"
# A genuine server fault still reports as one, with its reason code. # A genuine server fault still reports as one, with its detail.
resp = client.get("/boom") resp = client.get("/boom")
assert resp.status_code == 500 assert resp.status_code == 500
assert resp.get_json()["details"] == "OSError:EIO" assert "Input/output error" in resp.get_json()["details"]
def test_global_handler_reports_the_underlying_error(self): def test_global_handler_reports_the_underlying_error(self):
from flask import Flask, jsonify from flask import Flask, jsonify
@@ -348,5 +345,4 @@ class TestHandlersCarryDetail:
client = app.test_client() client = app.test_client()
body = client.get("/boom").get_json() body = client.get("/boom").get_json()
assert body["error_code"] == "UNKNOWN_ERROR" assert body["error_code"] == "UNKNOWN_ERROR"
assert body["details"] == "OSError:EIO" assert "Input/output error" in body["details"]
assert "Input/output error" not in str(body)
@@ -114,11 +114,12 @@ def test_the_answer_is_what_the_catch_all_returned(client, caplog, method, url,
assert records[-1].exc_info[1] is FORCED assert records[-1].exc_info[1] is FORCED
def test_the_exception_message_never_reaches_the_detail(client): def test_credentials_are_redacted_from_the_detail(client):
body = client.get("/api/v3/plugins/installed").get_json() body = client.get("/api/v3/plugins/installed").get_json()
for secret in ("SECRET123", "pw1", "K1", "forced failure"): for secret in ("SECRET123", "pw1", "K1"):
assert secret not in str(body) assert secret not in body["details"]
assert body["details"] == "RuntimeError" assert "<redacted>" in body["details"]
assert body["details"].startswith("RuntimeError: forced failure")
def _raise_415(): def _raise_415():
@@ -217,7 +218,7 @@ class TestPluginActionStep1:
encoding="utf-8") encoding="utf-8")
return d return d
def test_the_script_error_is_reported_by_type(self, plugin_dir, monkeypatch): def test_the_script_error_reaches_the_response(self, plugin_dir, monkeypatch):
from unittest.mock import MagicMock from unittest.mock import MagicMock
manager = MagicMock() manager = MagicMock()
manager.get_plugin_directory.return_value = str(plugin_dir) manager.get_plugin_directory.return_value = str(plugin_dir)
@@ -231,6 +232,5 @@ class TestPluginActionStep1:
assert resp.status_code == 500 assert resp.status_code == 500
body = resp.get_json() body = resp.get_json()
assert body["details"] == "RuntimeError" assert body["details"] == "RuntimeError: the auth script failed"
assert "the auth script failed" not in str(body)
assert body["message"] == 'An error occurred; see logs for details' assert body["message"] == 'An error occurred; see logs for details'
@@ -946,27 +946,21 @@ class TestTheStoreReportsWhyItIsEmpty:
class TestACrashCarriesItsDetail: class TestACrashCarriesItsDetail:
"""Seventeen Starlark handlers answered 5xx with no detail at all. """Seventeen Starlark handlers answered 5xx with no detail at all."""
The detail is a reason code (the exception type), not the exception's def test_browse_returns_the_exception_detail(self, client):
message, which stays in the log (CodeQL py/stack-trace-exposure).
"""
def test_browse_returns_the_reason_code(self, client):
with patch('web_interface.blueprints.api_v3._get_tronbyte_repository_class', with patch('web_interface.blueprints.api_v3._get_tronbyte_repository_class',
side_effect=ImportError("No module named 'yaml'")): side_effect=ImportError("No module named 'yaml'")):
body = client.get('/api/v3/starlark/repository/browse').get_json() body = client.get('/api/v3/starlark/repository/browse').get_json()
assert body.get('details') == 'ImportError', body assert 'yaml' in body.get('details', ''), body
assert 'yaml' not in str(body), body
def test_status_returns_the_reason_code(self, client): def test_status_returns_the_exception_detail(self, client):
with patch('web_interface.blueprints.api_v3._get_starlark_plugin', with patch('web_interface.blueprints.api_v3._get_starlark_plugin',
side_effect=RuntimeError("plugin manager is not attached")): side_effect=RuntimeError("plugin manager is not attached")):
body = client.get('/api/v3/starlark/status').get_json() body = client.get('/api/v3/starlark/status').get_json()
assert body.get('details') == 'RuntimeError', body assert 'plugin manager is not attached' in body.get('details', ''), body
assert 'plugin manager is not attached' not in str(body), body
class TestTheListingIsNotCappedAtOneThousand: class TestTheListingIsNotCappedAtOneThousand:
+8
View File
@@ -159,10 +159,18 @@ schema_manager = SchemaManager(
# saves reach the running plugins through the display's config watcher; what # saves reach the running plugins through the display's config watcher; what
# the display knows at run time (health, metrics, errors, current mode) it # the display knows at run time (health, metrics, errors, current mode) it
# publishes to the shared cache. See docs/ARCHITECTURE.md. # publishes to the shared cache. See docs/ARCHITECTURE.md.
def _catalog_runtime_view():
"""The display's runtime view, for the catalog's mode lookups. Imported
on call, as the startup reconciliation below imports it."""
from web_interface.blueprints.api_v3 import _plugin_runtime_view
return _plugin_runtime_view()
plugin_catalog = PluginCatalog( plugin_catalog = PluginCatalog(
plugins_dir=plugins_dir, plugins_dir=plugins_dir,
config_manager=config_manager, config_manager=config_manager,
schema_manager=schema_manager, schema_manager=schema_manager,
runtime_source=_catalog_runtime_view,
) )
# Initialize operation queue for plugin operations # Initialize operation queue for plugin operations
+21 -26
View File
@@ -222,7 +222,7 @@ def _save_config_atomic(config_manager, config_data, create_backup=True):
config_manager.save_config(config_data) config_manager.save_config(config_data)
return True, None return True, None
except Exception as e: except Exception as e:
return False, f"Failed to save configuration ({describe_exception(e)})" return False, str(e)
def _coerce_to_bool(value): def _coerce_to_bool(value):
""" """
Coerce a form value to a proper Python boolean. Coerce a form value to a proper Python boolean.
@@ -246,11 +246,7 @@ def _coerce_to_bool(value):
return value.lower() in ('true', 'on', '1', 'yes') return value.lower() in ('true', 'on', '1', 'yes')
return False return False
def _get_display_service_status(): def _get_display_service_status():
"""Return status information about the ledmatrix service. """Return status information about the ledmatrix service."""
active/returncode only: this goes back in API responses, and systemctl's
output (or an exception's text) is logged rather than returned.
"""
try: try:
result = subprocess.run( result = subprocess.run(
['systemctl', 'is-active', 'ledmatrix'], ['systemctl', 'is-active', 'ledmatrix'],
@@ -258,18 +254,26 @@ def _get_display_service_status():
text=True, text=True,
timeout=3 timeout=3
) )
if result.stderr.strip():
logger.debug('systemctl is-active ledmatrix: %s', result.stderr.strip())
return { return {
'active': result.stdout.strip() == 'active', 'active': result.stdout.strip() == 'active',
'returncode': result.returncode, 'returncode': result.returncode,
'stdout': result.stdout.strip(),
'stderr': result.stderr.strip()
} }
except subprocess.TimeoutExpired: except subprocess.TimeoutExpired:
logger.warning('systemctl is-active ledmatrix timed out') return {
return {'active': False, 'returncode': -1} 'active': False,
except Exception: 'returncode': -1,
logger.warning('Could not query ledmatrix.service status', exc_info=True) 'stdout': '',
return {'active': False, 'returncode': -1} 'stderr': 'timeout'
}
except Exception as err:
return {
'active': False,
'returncode': -1,
'stdout': '',
'stderr': str(err)
}
def _run_systemctl_command(args): def _run_systemctl_command(args):
"""Run a systemctl command safely.""" """Run a systemctl command safely."""
try: try:
@@ -291,26 +295,18 @@ def _run_systemctl_command(args):
'stderr': 'timeout' 'stderr': 'timeout'
} }
except Exception as err: except Exception as err:
logger.warning('%s failed', ' '.join(args), exc_info=True)
return { return {
'returncode': -1, 'returncode': -1,
'stdout': '', 'stdout': '',
'stderr': describe_exception(err) 'stderr': str(err)
} }
def _public_service_result(result):
"""A _run_systemctl_command result fit for a response: no stdout/stderr."""
if result.get('returncode') != 0:
logger.error('systemctl exited %s: %s', result.get('returncode'),
(result.get('stderr') or '').strip())
return {k: v for k, v in result.items() if k not in ('stdout', 'stderr')}
def _ensure_display_service_running(): def _ensure_display_service_running():
"""Ensure the ledmatrix display service is running.""" """Ensure the ledmatrix display service is running."""
status = _get_display_service_status() status = _get_display_service_status()
if status.get('active'): if status.get('active'):
status['started'] = False status['started'] = False
return status return status
result = _public_service_result( result = _run_systemctl_command(['sudo', 'systemctl', 'start', 'ledmatrix.service'])
_run_systemctl_command(['sudo', 'systemctl', 'start', 'ledmatrix.service']))
service_status = _get_display_service_status() service_status = _get_display_service_status()
result['started'] = result.get('returncode') == 0 result['started'] = result.get('returncode') == 0
result['active'] = service_status.get('active') result['active'] = service_status.get('active')
@@ -318,8 +314,7 @@ def _ensure_display_service_running():
return result return result
def _stop_display_service(): def _stop_display_service():
"""Stop the ledmatrix display service.""" """Stop the ledmatrix display service."""
result = _public_service_result( result = _run_systemctl_command(['sudo', 'systemctl', 'stop', 'ledmatrix.service'])
_run_systemctl_command(['sudo', 'systemctl', 'stop', 'ledmatrix.service']))
status = _get_display_service_status() status = _get_display_service_status()
result['active'] = status.get('active') result['active'] = status.get('active')
result['status'] = status result['status'] = status
@@ -717,7 +712,7 @@ def _do_transactional_uninstall(plugin_id, preserve_config):
success = api_v3.plugin_store_manager.uninstall_plugin(plugin_id) success = api_v3.plugin_store_manager.uninstall_plugin(plugin_id)
except Exception as remove_err: except Exception as remove_err:
_rollback() _rollback()
return False, f"Failed to remove plugin {plugin_id} ({describe_exception(remove_err)})" return False, f"Failed to remove plugin {plugin_id}: {remove_err}"
if not success: if not success:
_rollback() _rollback()
+15 -4
View File
@@ -153,10 +153,12 @@ def get_display_modes():
same list the force-display dialog offers, from the source that owns it. same list the force-display dialog offers, from the source that owns it.
Knowing each mode's plugin_id also matters because /display/on-demand/start Knowing each mode's plugin_id also matters because /display/on-demand/start
falls back to find_plugin_for_mode when plugin_id is omitted, and that falls back to find_plugin_for_mode when plugin_id is omitted. While the
lookup only sees modes declared in a static manifest -- a plugin whose display is running, both that lookup and this list use the modes it
modes are generated (each installed Starlark app is one) 404s there. registered, so modes a plugin generates from its config (each installed
Sending the plugin_id from this list skips the lookup entirely. Starlark app, each soccer custom league) are found (#668); with the
display stopped they see only what manifests declare. Sending the
plugin_id from this list skips the lookup entirely.
Query params: Query params:
include_disabled: '1' to list modes of disabled plugins too. They can include_disabled: '1' to list modes of disabled plugins too. They can
@@ -277,6 +279,15 @@ def start_on_demand_display():
if not resolved_plugin: if not resolved_plugin:
return jsonify({'status': 'error', 'message': f'Mode {resolved_mode} not found'}), 404 return jsonify({'status': 'error', 'message': f'Mode {resolved_mode} not found'}), 404
# The display matches mode names exactly: pass the registered spelling
# when the caller's differs only in case.
if api_v3.plugin_catalog and resolved_plugin and resolved_mode:
wanted = resolved_mode.strip().lower()
for registered in api_v3.plugin_catalog.get_plugin_display_modes(resolved_plugin):
if isinstance(registered, str) and registered.lower() == wanted:
resolved_mode = registered
break
# On-demand works with disabled plugins: the running display loads one # On-demand works with disabled plugins: the running display loads one
# for the session and unloads it afterwards, leaving config.json alone # for the session and unloads it afterwards, leaving config.json alone
# (DisplayController._load_plugin_for_on_demand). Logged for debugging. # (DisplayController._load_plugin_for_on_demand). Logged for debugging.
+3 -2
View File
@@ -150,8 +150,9 @@ def get_installed_plugins():
vegas_participation, vegas_participation_source = _vegas_participation( vegas_participation, vegas_participation_source = _vegas_participation(
plugin_id, plugin_config, plugin_info) plugin_id, plugin_config, plugin_info)
# The modes the manifest declares, from the catalog as /display/modes # The plugin's modes, from the catalog as /display/modes and
# and on-demand/start read them. The on-demand modal offers these; # on-demand/start read them: what the running display registered,
# else what the manifest declares. The on-demand modal offers these;
# without them it offered only the plugin id, which the display # without them it offered only the plugin id, which the display
# turns into the first mode. Strings only: a manifest is hand-edited. # turns into the first mode. Strings only: a manifest is hand-edited.
declared_modes = api_v3.plugin_catalog.get_plugin_display_modes(plugin_id) declared_modes = api_v3.plugin_catalog.get_plugin_display_modes(plugin_id)
@@ -983,6 +983,7 @@ def stop_pixlet_editor():
'status': 'error', 'status': 'error',
'message': 'Editor force-stopped, but the display could not be ' 'message': 'Editor force-stopped, but the display could not be '
'restarted automatically - start it manually.', 'restarted automatically - start it manually.',
'details': (result.get('stderr') or '').strip(),
'data': {'running': False}}), 500 'data': {'running': False}}), 500
return jsonify({'status': 'success', return jsonify({'status': 'success',
'message': 'Editor force-stopped; the display has been ' 'message': 'Editor force-stopped; the display has been '
+4 -3
View File
@@ -689,7 +689,7 @@ def execute_system_action():
logger.warning("install_base_requirements timed out for %s", label) logger.warning("install_base_requirements timed out for %s", label)
except OSError as install_err: except OSError as install_err:
all_ok = False all_ok = False
outputs.append(f"== {label} ==\nFailed: {describe_exception(install_err)}") outputs.append(f"== {label} ==\nFailed: {install_err}")
logger.warning("install_base_requirements errored for %s: %s", label, install_err) logger.warning("install_base_requirements errored for %s: %s", label, install_err)
return jsonify({ return jsonify({
'status': 'success' if all_ok else 'error', 'status': 'success' if all_ok else 'error',
@@ -784,10 +784,11 @@ def execute_system_action():
return jsonify({'status': 'error', 'message': 'Command timed out', 'returncode': -1, 'stderr': 'timeout'}) return jsonify({'status': 'error', 'message': 'Command timed out', 'returncode': -1, 'stderr': 'timeout'})
except Exception as e: except Exception as e:
logger.error("execute_system_action failed: %s", e, exc_info=True) logger.error("execute_system_action failed: %s", e, exc_info=True)
detail = describe_exception(e)
resp = { resp = {
'status': 'error', 'status': 'error',
'message': _sudo_hint_for(str(e)) or 'Action failed; see logs for details', 'message': _sudo_hint_for(detail) or 'Action failed; see logs for details',
'details': describe_exception(e), 'details': detail,
} }
return jsonify(resp), 500 return jsonify(resp), 500
@api_v3.route('/system/git-info', methods=['GET']) @api_v3.route('/system/git-info', methods=['GET'])
+2 -2
View File
@@ -67,7 +67,7 @@ def _run_background_connect(ssid, password):
payload = _connect_result_payload(ssid, success, message) payload = _connect_result_payload(ssid, success, message)
except Exception as e: except Exception as e:
logger.error("Background WiFi connect failed", exc_info=True) logger.error("Background WiFi connect failed", exc_info=True)
payload = {'status': 'error', 'message': f'Failed to connect to network ({describe_exception(e)})'} payload = {'status': 'error', 'message': describe_exception(e)}
_record_connect_result(ssid, payload) _record_connect_result(ssid, payload)
@@ -276,7 +276,7 @@ def connect_wifi():
try: try:
success, message = wifi_manager.connect_to_network(ssid, password) success, message = wifi_manager.connect_to_network(ssid, password)
except Exception as e: except Exception as e:
_record_connect_result(ssid, {'status': 'error', 'message': f'Failed to connect to network ({describe_exception(e)})'}) _record_connect_result(ssid, {'status': 'error', 'message': describe_exception(e)})
raise raise
payload = _connect_result_payload(ssid, success, message) payload = _connect_result_payload(ssid, success, message)
_record_connect_result(ssid, payload) _record_connect_result(ssid, payload)
+2 -2
View File
@@ -86,7 +86,7 @@ def refresh_after_update(run=None, systemd_dir=None, helper_source=None, helper_
except Exception as e: # a broken template must not fail the update itself except Exception as e: # a broken template must not fail the update itself
if type(e).__name__ != 'UnitsUnreadable': if type(e).__name__ != 'UnitsUnreadable':
logger.warning("Could not compare the installed systemd units with the new templates: %s", e) logger.warning("Could not compare the installed systemd units with the new templates: %s", e)
return _result(FAILED, 'The service settings could not be checked; see logs for details.') return _result(FAILED, f'The service settings could not be checked: {e}.')
# Units installed mode 0600 (install_service.sh run on its own, before # Units installed mode 0600 (install_service.sh run on its own, before
# it set 0644): only root can compare them, so let the helper decide. # it set 0644): only root can compare them, so let the helper decide.
stale = [] stale = []
@@ -110,7 +110,7 @@ def refresh_after_update(run=None, systemd_dir=None, helper_source=None, helper_
timeout=TIMEOUT_SECONDS) timeout=TIMEOUT_SECONDS)
except (subprocess.SubprocessError, OSError) as e: except (subprocess.SubprocessError, OSError) as e:
logger.warning("Refreshing the systemd units failed: %s", e) logger.warning("Refreshing the systemd units failed: %s", e)
return _result(FAILED, f'Updating the service settings ({names}) failed; see logs for details.', stale) return _result(FAILED, f'Updating the service settings ({names}) failed: {e}.', stale)
if result.returncode == 0: if result.returncode == 0:
# The helper says what it did: "units refreshed: a b" or "units: up to date". # The helper says what it did: "units refreshed: a b" or "units: up to date".
done = next((line.split(':', 1)[1].split() for line in (result.stdout or '').splitlines() done = next((line.split(':', 1)[1].split() for line in (result.stdout or '').splitlines()