Files
LEDMatrix/src/plugin_system/plugin_executor.py
T
ChuckandClaude Opus 5.5 84afa9d64f refactor: delete dead Python code in the core (and stop storing Wi-Fi passwords) (#608)
* refactor(plugins): remove the no-op PluginHealthMonitor

Its monitor loop did nothing (`if callbacks: pass`), register_health_check
had no callers and api_v3.health_monitor was never read by any route. The
live health data comes from PluginHealthTracker, which is untouched.

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

* refactor(store): drop the never-set uninstall tombstones

Nothing in production called mark_recently_uninstalled, so the
reconciler's was_recently_uninstalled check was always False. The
persistent uninstall registry is what actually stops resurrection; the
reconciler test now exercises that gate instead.

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

* refactor(common): delete unused config/display/game helpers, utils and error_handler

Nothing in core, the web UI, scripts or the plugin monorepo imports
config_helper, display_helper, game_helper, utils or error_handler; only
their own tests did. The error_handler re-exports leave src.common's
__all__; APIHelper, TextHelper, ScrollHelper, LogoHelper and the adaptive
layout exports are unchanged.

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

* refactor(config): drop ConfigService's unused versioning and save API

ConfigVersion, get_version/get_version_history/get_version_config,
rollback, save_config, reload, get_plugin_config and the backward-compat
load_config/get_config_path/get_secrets_path had no callers. The display
controller only uses get_config, subscribe, unsubscribe and shutdown,
plus the file watcher. Change detection now compares against the
current checksum instead of the last history entry.

The subscriber tests asserted `callback.called or True`; they now
reload the way the watcher does and assert the notification.

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

* refactor(plugins): drop unread plugin state history and callbacks

plugin_state.PluginStateManager kept a bounded per-plugin transition
history that only get_state_history (tests only) read; get_state_info
reports a separate lifetime count, which stays. set_error_info and
record_display had no callers, and set_state_with_error's `error`
argument only fed the history.

The web-side state_manager.PluginStateManager loses
subscribe_to_state_changes, _notify_callbacks, set_plugin_error and
get_state_version, none of which had callers; with no subscribers the
old-state copy in update_plugin_state went with them.

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

* refactor(plugins): remove unused PluginManager methods and attribute guards

update_all_plugins was only called by a test (the display loop uses
run_scheduled_updates); get_plugin_health_metrics,
get_plugin_resource_metrics and get_plugin_state had no callers; and
plugin_modules was written but never read. plugin_directories is now
initialised in __init__, so the hasattr() guards around it go.

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

* refactor(plugins): remove unused executor, loader, store and package helpers

- PluginExecutor.execute_safe: no callers.
- PluginLoader._parse_semver: only its own tests; compatibility.parse_semver
  is the live copy and test_compatibility.py already covers it.
- PluginStoreManager.get_installed_plugin_info: no callers.
- PluginResourceMonitor._local: never read.
- src.plugin_system.get_store_manager and __api_version__: no importers in
  core, scripts or the plugin monorepo.

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

* fix(wifi): stop storing Wi-Fi passwords in wifi_config.json

WiFiManager appended every joined network's SSID and password, in
plaintext, to saved_networks in config/wifi_config.json, and nothing
(web UI, backup restore, scripts) ever read them back: NetworkManager
keeps its own credentials. The writes are gone, and loading the config
now drops any saved_networks key and rewrites the file, so passwords
already on disk are scrubbed.

Also removes _check_dnsmasq_conflict (never called) and _detect_trixie,
whose result only reached one log line, along with the
NM_CONNECTIONS_PATHS constant only it used.

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

* refactor(display): remove unreachable and unused DisplayController code

- _follower_rebuild_scroll_image: never called.
- mode_duration (never read) and last_mode_change (write-only).
- The `chosen_cap <= 0` branch: chosen_cap is either the minimum of
  caps already filtered to > 0 or DEFAULT_DYNAMIC_DURATION_CAP (180).
- The `max_duration < min_duration` branch directly after
  `max_duration = max(min_duration, max_duration)`.
- The circuit-breaker branch's `display_result = False` and
  `manager_to_display = None`: the first is overwritten a few lines
  later, the second is already None there.
- The bool-to-bool conversion of execute_display's result, which is
  always a bool.
- The `loaded_plugins` lookup in _update_modules: PluginManager has no
  such attribute, so it always fell through to `plugins`.

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

* refactor(vegas): remove unused config update, boundary finder and refresh

VegasModeConfig.update had no callers outside its own tests (the
coordinator rebuilds the config with from_config on a change);
geometry.find_item_boundary and StreamManager._refresh_plugin_content
had no callers at all.

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

* refactor(run): drop the debug block that pretended to import the plugin system

In debug mode run.py put src/plugin_system itself on sys.path and printed
"Plugin system import successful" without importing anything. Nothing
imports plugin_system modules by bare name, so the path entry did
nothing either.

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

* test: delete tests that test nothing

- test/plugins/test_{basketball_scoreboard,calendar,clock_simple,
  odds_ticker,soccer_scoreboard,text_display}.py skip everywhere the named
  plugins are not installed, including CI (LEDMATRIX_PLUGINS_DIR holds only
  the fixture plugin); test_plugin_matrix.py already covers every
  discovered plugin. Their PluginTestBase and the fixtures only it used
  (plugins_dir, mock_display_manager, mock_cache_manager,
  mock_plugin_manager, base_plugin_config in test/plugins/conftest.py) go
  with them.
- test_plugin_system.py: test_discover_plugins (body was `pass`) and
  test_dependency_check (a comment), plus the test_plugin_manager fixture
  only the former requested.
- test_display_manager.py: test_draw_image asserted that an image it had
  just assigned was not None.
- test_display_controller.py: the rotation and schedule-override tests
  re-implemented the run-loop arithmetic inline and asserted on their own
  result without calling the controller.

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

* test: expect one plugin_last_update success stamp after update_all_plugins

EveryStampRecordsACompletion required at least two success-path stamps;
the second was update_all_plugins, removed as test-only. The worker and
synchronous paths share the remaining stamp in _execute_update_now, and
the check that every stamp calls _note_update_completed is unchanged.

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

---------

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
2026-09-23 12:36:26 -04:00

241 lines
9.0 KiB
Python

"""
Plugin Executor
Handles plugin execution (update() and display() calls) with timeout handling,
error isolation, and performance monitoring.
"""
import time
from typing import Any, Optional, Callable
from threading import Thread
import logging
from src.exceptions import PluginError
from src.logging_config import get_logger
from src.error_aggregator import record_error
class PluginTimeoutError(Exception):
"""Raised when a plugin operation times out."""
class PluginExecutor:
"""Handles plugin execution with timeout and error isolation."""
def __init__(
self,
default_timeout: float = 30.0,
logger: Optional[logging.Logger] = None
) -> None:
"""
Initialize the plugin executor.
Args:
default_timeout: Default timeout in seconds for plugin operations
logger: Optional logger instance
"""
self.default_timeout = default_timeout
self.logger = logger or get_logger(__name__)
def execute_with_timeout(
self,
operation: Callable[[], Any],
timeout: Optional[float] = None,
plugin_id: Optional[str] = None
) -> Any:
"""
Execute a plugin operation with timeout.
Args:
operation: Function to execute
timeout: Timeout in seconds (None = use default)
plugin_id: Optional plugin ID for logging
Returns:
Result of operation
Raises:
PluginTimeoutError: If operation times out
PluginError: If operation raises an exception
"""
timeout = timeout or self.default_timeout
plugin_context = f"plugin {plugin_id}" if plugin_id else "plugin"
# Use threading-based timeout (more reliable than signal-based)
result_container = {'value': None, 'exception': None, 'completed': False}
def target():
try:
result_container['value'] = operation()
result_container['completed'] = True
except Exception as e:
result_container['exception'] = e
result_container['completed'] = True
thread = Thread(target=target, daemon=True)
thread.start()
thread.join(timeout=timeout)
# NB: this timeout is advisory. Nothing cancels the thread -- Python
# has no way to -- so on expiry the operation keeps running to
# completion in the background and only this caller gives up waiting.
# A plugin that hangs permanently leaks one daemon thread per attempt.
# Callers that hold a resource across the call must release it from
# inside the wrapped callable rather than after this returns; see the
# _release_display_lock guard inside DisplayController.run().
if not result_container['completed']:
error_msg = f"{plugin_context} operation timed out after {timeout}s"
self.logger.error(error_msg)
timeout_error = PluginTimeoutError(error_msg)
record_error(timeout_error, plugin_id=plugin_id, operation="timeout")
raise timeout_error
if result_container['exception']:
error = result_container['exception']
error_msg = f"{plugin_context} operation failed: {error}"
self.logger.error(error_msg, exc_info=True)
record_error(error, plugin_id=plugin_id, operation="execute")
raise PluginError(error_msg, plugin_id=plugin_id) from error
return result_container['value']
def execute_update(
self,
plugin: Any,
plugin_id: str,
timeout: Optional[float] = None
) -> bool:
"""
Execute plugin update() method with error handling.
Args:
plugin: Plugin instance
plugin_id: Plugin identifier
timeout: Timeout in seconds (None = use default)
Returns:
True if update succeeded, False otherwise
"""
try:
start_time = time.time()
self.execute_with_timeout(
lambda: plugin.update(),
timeout=timeout,
plugin_id=plugin_id
)
duration = time.time() - start_time
if duration > 5.0: # Warn if update takes more than 5 seconds
self.logger.warning(
"Plugin %s update() took %.2fs (consider optimizing)",
plugin_id,
duration
)
return True
except PluginTimeoutError:
self.logger.error("Plugin %s update() timed out", plugin_id)
return False
except PluginError:
# Already logged and recorded in execute_with_timeout
return False
except Exception as e:
self.logger.error(
"Unexpected error executing update() for plugin %s: %s",
plugin_id,
e,
exc_info=True
)
record_error(e, plugin_id=plugin_id, operation="update")
return False
def execute_display(
self,
plugin: Any,
plugin_id: str,
force_clear: bool = False,
display_mode: Optional[str] = None,
timeout: Optional[float] = None,
accepts_display_mode: Optional[bool] = None
) -> bool:
"""
Execute plugin display() method with error handling.
Args:
plugin: Plugin instance
plugin_id: Plugin identifier
force_clear: Whether to force clear display
display_mode: Optional display mode parameter
timeout: Timeout in seconds (None = use default)
accepts_display_mode: Whether plugin.display() takes a
display_mode keyword. Pass it when the caller already knows;
None falls back to inspecting the callable.
Returns:
True if display succeeded, False otherwise
"""
try:
start_time = time.time()
# Does display() take a display_mode keyword? The caller usually
# knows and caches the answer, so prefer what it passed.
#
# Inspecting here was not merely redundant, it could never be
# cached: display_controller wraps the real plugin in a fresh
# SimpleNamespace per call, so inspect.signature() saw a new
# callable every time and paid ~55us on a Pi 4 to re-derive a
# value the caller had computed one line earlier and stored in
# self._plugin_accepts_display_mode.
if accepts_display_mode is None:
import inspect
accepts_display_mode = (
'display_mode' in inspect.signature(plugin.display).parameters)
has_display_mode = accepts_display_mode
# Capture the return value from the plugin's display() method
if has_display_mode and display_mode:
result = self.execute_with_timeout(
lambda: plugin.display(display_mode=display_mode, force_clear=force_clear),
timeout=timeout,
plugin_id=plugin_id
)
else:
result = self.execute_with_timeout(
lambda: plugin.display(force_clear=force_clear),
timeout=timeout,
plugin_id=plugin_id
)
duration = time.time() - start_time
if duration > 2.0: # Warn if display takes more than 2 seconds
self.logger.warning(
"Plugin %s display() took %.2fs (consider optimizing)",
plugin_id,
duration
)
# Return the actual result from the plugin's display() method
# If it's a boolean, use it directly. Otherwise, treat None/other as True for backward compatibility
if isinstance(result, bool):
self.logger.debug(f"Plugin {plugin_id} display() returned boolean: {result}")
return result
# For backward compatibility: if plugin returns None or something else, treat as success
self.logger.debug(f"Plugin {plugin_id} display() returned non-boolean: {result}, treating as True")
return True
except PluginTimeoutError:
self.logger.error("Plugin %s display() timed out", plugin_id)
return False
except PluginError:
# Already logged and recorded in execute_with_timeout
return False
except Exception as e:
self.logger.error(
"Unexpected error executing display() for plugin %s: %s",
plugin_id,
e,
exc_info=True
)
record_error(e, plugin_id=plugin_id, operation="display")
return False