From 34b186125a340412efab3444594ac066adaf129a Mon Sep 17 00:00:00 2001 From: Chuck <33324927+ChuckBuilds@users.noreply.github.com> Date: Tue, 26 May 2026 09:55:59 -0400 Subject: [PATCH] fix(logs): include ledmatrix-web logs in viewer and log subprocess stderr on failure (#350) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two bugs conspired to produce "check the logs" toasts with an empty log viewer: 1. The log viewer (both SSE stream and REST endpoint) only queried ledmatrix.service via journalctl. Web API errors are logged by the Flask process running as ledmatrix-web.service, so they never appeared in the viewer. Add -u ledmatrix-web.service to both calls; also add --output=short-iso so timestamps from the two services sort cleanly when interleaved. Use shutil.which-resolved absolute paths for sudo/journalctl (S607 compliance) in api_v3.py; fall back to known Pi paths if which returns None. 2. app.py: resolve journalctl and systemctl to absolute paths via shutil.which at module init (_JOURNALCTL, _SYSTEMCTL). Replace bare names in logs_generator() and the cached systemctl is-active check. Guard both sites: logs_generator yields a clear SSE error message and sleeps 60 s if journalctl is not found; the systemctl block is skipped entirely if systemctl is not found, leaving the cache at its last-known value. 3. When execute_system_action() ran a systemctl command that returned non-zero, only the return code was logged — result.stderr was silently discarded. Log it at ERROR level and include returncode and stderr in the JSON response so callers get actionable failure details. Same fix applied to the early-return start_display branch. Co-authored-by: Chuck Co-authored-by: Claude Sonnet 4.6 --- web_interface/app.py | 26 +++++++++---- web_interface/blueprints/api_v3.py | 60 ++++++++++++++++++++++-------- 2 files changed, 62 insertions(+), 24 deletions(-) diff --git a/web_interface/app.py b/web_interface/app.py index 97287119..f8ffcbd3 100644 --- a/web_interface/app.py +++ b/web_interface/app.py @@ -3,6 +3,7 @@ import json import logging import os import queue +import shutil import sys import subprocess import threading @@ -24,6 +25,9 @@ from src.plugin_system.state_manager import PluginStateManager from src.plugin_system.operation_history import OperationHistory from src.plugin_system.health_monitor import PluginHealthMonitor +_JOURNALCTL = shutil.which('journalctl') +_SYSTEMCTL = shutil.which('systemctl') + # Create Flask app app = Flask(__name__) app.secret_key = os.urandom(24) @@ -492,12 +496,13 @@ def system_status_generator(): # Check if display service is running (cached to avoid per-client subprocess forks) now = time.time() if (now - _ledmatrix_service_cache['timestamp']) >= _LEDMATRIX_SERVICE_CACHE_TTL: - try: - result = subprocess.run(['systemctl', 'is-active', 'ledmatrix'], - capture_output=True, text=True, timeout=2) - _ledmatrix_service_cache['active'] = result.stdout.strip() == 'active' - except (subprocess.SubprocessError, OSError): - pass + if _SYSTEMCTL: + try: + result = subprocess.run([_SYSTEMCTL, 'is-active', 'ledmatrix'], + capture_output=True, text=True, timeout=2) + _ledmatrix_service_cache['active'] = result.stdout.strip() == 'active' + except (subprocess.SubprocessError, OSError) as e: + app.logger.warning("systemctl status check failed: %s", e) _ledmatrix_service_cache['timestamp'] = now service_active = _ledmatrix_service_cache['active'] @@ -589,8 +594,13 @@ def logs_generator(): # Get recent logs from journalctl (simplified version) # Note: User should be in systemd-journal group to read logs without sudo try: + if not _JOURNALCTL: + yield {'timestamp': time.time(), 'logs': 'journalctl not found; cannot read logs'} + time.sleep(60) + continue result = subprocess.run( - ['journalctl', '-u', 'ledmatrix.service', '-n', '50', '--no-pager'], + [_JOURNALCTL, '-u', 'ledmatrix.service', '-u', 'ledmatrix-web.service', + '-n', '50', '--no-pager', '--output=short-iso'], capture_output=True, text=True, timeout=5 ) @@ -606,7 +616,7 @@ def logs_generator(): # No logs available logs_data = { 'timestamp': time.time(), - 'logs': 'No logs available from ledmatrix service' + 'logs': 'No logs available from ledmatrix or ledmatrix-web service' } yield logs_data else: diff --git a/web_interface/blueprints/api_v3.py b/web_interface/blueprints/api_v3.py index d434afc2..c38b5598 100644 --- a/web_interface/blueprints/api_v3.py +++ b/web_interface/blueprints/api_v3.py @@ -4,6 +4,7 @@ import os import re import stat import sys +import shutil import subprocess import tempfile import time @@ -25,6 +26,9 @@ from src.web_interface.validators import ( ) from src.error_aggregator import get_error_aggregator +_SUDO = shutil.which('sudo') +_JOURNALCTL = shutil.which('journalctl') + # Will be initialized when blueprint is registered config_manager = None plugin_manager = None @@ -1456,31 +1460,41 @@ def execute_system_action(): if mode: # For on-demand modes, we would need to integrate with the display controller # For now, just start the display service - result = subprocess.run(['sudo', 'systemctl', 'start', 'ledmatrix'], - capture_output=True, text=True) + try: + result = subprocess.run(['sudo', 'systemctl', 'start', 'ledmatrix'], + capture_output=True, text=True, timeout=10) + except subprocess.TimeoutExpired as e: + logger.error("start_display (%s) timed out: %s", mode, e) + return jsonify({'status': 'error', 'message': 'Command timed out', 'returncode': -1, 'stderr': 'timeout'}) logger.info("start_display (%s) returned code %d", mode, result.returncode) - return jsonify({ + if result.returncode != 0 and result.stderr: + logger.error("start_display (%s) stderr: %s", mode, result.stderr.strip()) + resp = { 'status': 'success' if result.returncode == 0 else 'error', 'message': 'Display started' if result.returncode == 0 else 'Failed to start display', - }) + } + if result.returncode != 0: + resp['returncode'] = result.returncode + resp['stderr'] = result.stderr.strip() + return jsonify(resp) else: result = subprocess.run(['sudo', 'systemctl', 'start', 'ledmatrix'], - capture_output=True, text=True) + capture_output=True, text=True, timeout=10) elif action == 'stop_display': result = subprocess.run(['sudo', 'systemctl', 'stop', 'ledmatrix'], - capture_output=True, text=True) + capture_output=True, text=True, timeout=10) elif action == 'enable_autostart': result = subprocess.run(['sudo', 'systemctl', 'enable', 'ledmatrix'], - capture_output=True, text=True) + capture_output=True, text=True, timeout=10) elif action == 'disable_autostart': result = subprocess.run(['sudo', 'systemctl', 'disable', 'ledmatrix'], - capture_output=True, text=True) + capture_output=True, text=True, timeout=10) elif action == 'reboot_system': result = subprocess.run(['sudo', 'reboot'], - capture_output=True, text=True) + capture_output=True, text=True, timeout=10) elif action == 'shutdown_system': result = subprocess.run(['sudo', 'poweroff'], - capture_output=True, text=True) + capture_output=True, text=True, timeout=10) elif action == 'git_pull': # Use PROJECT_ROOT instead of hardcoded path project_dir = str(PROJECT_ROOT) @@ -1555,20 +1569,29 @@ def execute_system_action(): }) elif action == 'restart_display_service': result = subprocess.run(['sudo', 'systemctl', 'restart', 'ledmatrix'], - capture_output=True, text=True) + capture_output=True, text=True, timeout=10) elif action == 'restart_web_service': # Try to restart the web service (assuming it's ledmatrix-web.service) result = subprocess.run(['sudo', 'systemctl', 'restart', 'ledmatrix-web'], - capture_output=True, text=True) + capture_output=True, text=True, timeout=10) else: return jsonify({'status': 'error', 'message': 'Unknown action'}), 400 logger.info("system action '%s' returncode=%d", action, result.returncode) - return jsonify({ + if result.returncode != 0 and result.stderr: + logger.error("system action '%s' stderr: %s", action, result.stderr.strip()) + resp = { 'status': 'success' if result.returncode == 0 else 'error', 'message': 'Action completed' if result.returncode == 0 else 'Action failed; check logs for details', - }) + } + if result.returncode != 0: + resp['returncode'] = result.returncode + resp['stderr'] = result.stderr.strip() + return jsonify(resp) + except subprocess.TimeoutExpired as e: + logger.error("system action '%s' timed out: %s", action, e) + return jsonify({'status': 'error', 'message': 'Command timed out', 'returncode': -1, 'stderr': 'timeout'}) except Exception as e: logger.error("execute_system_action failed: %s", e, exc_info=True) return jsonify({'status': 'error', 'message': 'Action failed; see logs for details'}), 500 @@ -6425,9 +6448,14 @@ def list_plugin_assets(): def get_logs(): """Get system logs from journalctl""" try: + if not _JOURNALCTL: + return jsonify({'status': 'error', 'message': 'journalctl not found on this system'}), 503 # Get recent logs from journalctl + _cmd = ([_SUDO, _JOURNALCTL] if _SUDO else [_JOURNALCTL]) + [ + '-u', 'ledmatrix.service', '-u', 'ledmatrix-web.service', + '-n', '100', '--no-pager', '--output=short-iso'] result = subprocess.run( - ['sudo', 'journalctl', '-u', 'ledmatrix.service', '-n', '100', '--no-pager'], + _cmd, capture_output=True, text=True, timeout=5 @@ -6438,7 +6466,7 @@ def get_logs(): return jsonify({ 'status': 'success', 'data': { - 'logs': logs_text if logs_text else 'No logs available from ledmatrix service' + 'logs': logs_text if logs_text else 'No logs available from ledmatrix or ledmatrix-web service' } }) else: