fix(logs): include ledmatrix-web logs in viewer and log subprocess stderr on failure

[ChuckBuilds]

Summary

• Log viewer now shows both services: ledmatrix.service and ledmatrix-web.service are merged in both the live SSE stream (app.py) and the REST log endpoint (api_v3.py). Added --output=short-iso so timestamps from the two services sort cleanly when interleaved. Previously, any Python exception logged inside the Flask process was going to ledmatrix-web and never appearing in the viewer — producing the "check the logs" toast with an empty log panel.
• Subprocess stderr is now logged on failure: When execute_system_action() runs a systemctl command that returns non-zero, the error text from result.stderr is now logged at ERROR level. Same fix for the early-return start_display branch. Previously only the return code was recorded, so failures like sudo misconfiguration or a missing service unit were silently swallowed.

Test plan

• Trigger a failed system action (e.g. restart display while service doesn't exist or sudo isn't configured) — confirm the stderr reason now appears in the log viewer
• Open the log viewer normally — confirm it shows interleaved entries from both ledmatrix and ledmatrix-web with ISO timestamps
• Confirm successful actions still log at INFO level with no spurious ERROR output

🤖 Generated with Claude Code

Summary by CodeRabbit

• Bug Fixes
  • System logs now include entries from both display and web services for more comprehensive diagnostics.
  • Improved error responses for system operations: non-zero failures now include exit code and trimmed error output to aid troubleshooting.
  • SSE log stream now reports when log tooling is unavailable and updates the “no logs available” message to reference both services.
  • API now returns a 503 when required log tooling is missing.
  • Log output timestamps standardized for clearer reading.

[coderabbitai]

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

• @coderabbitai resume to resume automatic reviews.
• @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

• ▶️ Resume reviews
• 🔍 Trigger review

📝 Walkthrough

Walkthrough

Queries for logs now include both ledmatrix.service and ledmatrix-web.service; systemctl and journalctl binaries are resolved via shutil.which; execute_system_action logs and returns trimmed stderr and returncode on failures.

Changes

Logging and Diagnostics Improvements

Layer / File(s)	Summary
Resolve system binaries & use in status checks web_interface/app.py, web_interface/blueprints/api_v3.py	Adds shutil imports and module-level _JOURNALCTL, _SYSTEMCTL, and _SUDO resolution; system_status_generator now calls the resolved _SYSTEMCTL when available.
Capture and return stderr on failed system actions web_interface/blueprints/api_v3.py	execute_system_action now trims and logs stderr on non-zero systemctl exit codes (including start_display with mode) and includes returncode and trimmed stderr in JSON error responses.
Expand journalctl queries to both services web_interface/app.py, web_interface/blueprints/api_v3.py	SSE logs_generator and get_logs now query both ledmatrix.service and ledmatrix-web.service via resolved _JOURNALCTL/_SUDO, request formatted output (--output=short-iso / 100 lines), and update empty-log messages to reference both services.

🎯 3 (Moderate) | ⏱️ ~20 minutes

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately describes the main objectives: including ledmatrix-web logs in the viewer and logging subprocess stderr on failure, which aligns with the core changes in app.py and api_v3.py.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch fix/log-viewer-missing-web-service-logs

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[codacy-production]

Not up to standards ⛔

🔴 Issues1 critical · 1 medium

Alerts:
⚠ 2 issues (≤ 0 issues of at least minor severity)

Results:
2 new issues

Category	Results
Security	1 critical 1 medium

View in Codacy

🟢 Metrics0 complexity · 0 duplication

Metric	Results
Complexity	0
Duplication	0

View in Codacy

_{NEW Get contextual insights on your PRs based on Codacy's metrics, along with PR and Jira context, without leaving GitHub. Enable AI reviewer
TIP This summary will be updated as you push new changes.}

[coderabbitai]

Actionable comments posted: 2

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

web_interface/blueprints/api_v3.py (1)

6434-6447: ⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Update the no-logs message to match multi-service query scope.

The fallback message still says “ledmatrix service” even though logs now include both ledmatrix.service and ledmatrix-web.service, which can mislead troubleshooting.

As per coding guidelines, "Provide clear error messages for troubleshooting."

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@web_interface/blueprints/api_v3.py` around lines 6434 - 6447, The fallback
message is misleading because the journalctl call queries both ledmatrix.service
and ledmatrix-web.service; update the returned 'No logs available' string to
reference both services (e.g., "No logs available from ledmatrix and
ledmatrix-web services" or "No logs available from ledmatrix services") where
logs_text is used in the jsonify response in api_v3.py (look for result,
logs_text and the jsonify call returning 'logs' when result.returncode == 0) so
the message accurately reflects the multi-service query.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@web_interface/blueprints/api_v3.py`:
- Around line 1462-1463: The API currently logs subprocess failures in
start_display (using logger.error and checking result.returncode/result.stderr)
but does not include stderr or returncode in the HTTP response; update the error
branch in the start_display handler (and the similar block around the other
failing system-action section) to include result.returncode and a trimmed
result.stderr in the JSON response object so callers/UI receive actionable
failure details, preserving the existing error message field and adding e.g.
"returncode" and "stderr" keys populated from result.returncode and
result.stderr.strip().
- Around line 6434-6435: The subprocess command list that currently uses 'sudo'
and 'journalctl' as bare names should be changed to use absolute executable
paths on Raspberry Pi: resolve '/usr/bin/sudo' and the system journalctl binary
(e.g., '/bin/journalctl' or '/usr/bin/journalctl') rather than relying on PATH.
Update the command construction in web_interface/blueprints/api_v3.py where the
list ['sudo', 'journalctl', ...] is created (the log-fetching function that
builds this command) to obtain absolute paths via shutil.which at module init
(and raise/log a clear error if not found) or hardcode the known Pi paths, then
replace the bare names in the list with those resolved absolute paths so
subprocess calls are deterministic and Ruff S607-compliant.
---
Outside diff comments:
In `@web_interface/blueprints/api_v3.py`:
- Around line 6434-6447: The fallback message is misleading because the
journalctl call queries both ledmatrix.service and ledmatrix-web.service; update
the returned 'No logs available' string to reference both services (e.g., "No
logs available from ledmatrix and ledmatrix-web services" or "No logs available
from ledmatrix services") where logs_text is used in the jsonify response in
api_v3.py (look for result, logs_text and the jsonify call returning 'logs' when
result.returncode == 0) so the message accurately reflects the multi-service
query.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 6065eeaf-b705-4536-b381-c5140136e39f

📥 Commits

Reviewing files that changed from the base of the PR and between 0c7d03a and fe8fabd.

📒 Files selected for processing (2)

• web_interface/app.py
• web_interface/blueprints/api_v3.py

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@web_interface/app.py`:
- Around line 593-594: The logs_generator() function currently calls
subprocess.run(['journalctl', ...]) (and earlier there's a cached
subprocess.run(['systemctl', 'is-active', ...]) call) using bare executable
names; change both to resolve absolute paths via shutil.which('journalctl') and
shutil.which('systemctl') once at startup or before use, and replace the list
entries with the returned absolute paths; if which() returns None, emit a clear
SSE error message to the client and continue streaming (do not raise), so the
service degrades gracefully while avoiding PATH dependence and satisfying the
Ruff S607 requirement.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 7dbed653-7c6e-4d3a-9c53-81dd332c3935

📥 Commits

Reviewing files that changed from the base of the PR and between fe8fabd and d3c2638.

📒 Files selected for processing (2)

• web_interface/app.py
• web_interface/blueprints/api_v3.py

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@web_interface/app.py`:
- Around line 28-29: The fallback literal strings defeat your availability
checks: remove the "or '/usr/bin/journalctl'" and "or '/usr/bin/systemctl'" so
_JOURNALCTL = shutil.which('journalctl') and _SYSTEMCTL =
shutil.which('systemctl') (or, if you need to support fixed paths on specific
platforms, explicitly check os.path.exists('/usr/bin/journalctl') /
os.path.exists('/usr/bin/systemctl') and set the variables only when the path is
present); update code paths that rely on truthiness of _JOURNALCTL/_SYSTEMCTL
(e.g., the checks around _JOURNALCTL and _SYSTEMCTL and the subprocess
invocations) so they fail fast with clear "not found" messages rather than
relying on subprocess to raise FileNotFoundError.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 25e63b04-c0bb-4eb0-87b4-ac4073bc4f9f

📥 Commits

Reviewing files that changed from the base of the PR and between d3c2638 and 6a02623.

📒 Files selected for processing (2)

• web_interface/app.py
• web_interface/blueprints/api_v3.py

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@web_interface/blueprints/api_v3.py`:
- Around line 6444-6449: The route currently hard-fails when _SUDO is falsy even
though _JOURNALCTL alone may work; change the logic in the logs endpoint to only
error when _JOURNALCTL is missing, and otherwise construct the subprocess
command conditionally using _SUDO if present (e.g., cmd = [_SUDO, _JOURNALCTL,
...] if _SUDO else [_JOURNALCTL, ...]) before calling subprocess.run; keep the
existing flags ('-u' 'ledmatrix.service', '-u' 'ledmatrix-web.service', '-n',
'100', '--no-pager', '--output=short-iso') and return the same jsonify response
on success or a 503 only when journalctl is not available.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 69904103-e16b-4c52-be63-045f7802e19a

📥 Commits

Reviewing files that changed from the base of the PR and between 6a02623 and 0e54eaf.

📒 Files selected for processing (2)

• web_interface/app.py
• web_interface/blueprints/api_v3.py

[coderabbitai]

Actionable comments posted: 1

🧹 Nitpick comments (1)

web_interface/app.py (1)

504-505: 💤 Low value

Consider logging systemctl failures for remote debugging.

The exception handler silently ignores errors when checking service status. For a Raspberry Pi deployment where remote debugging is important, logging the failure would help diagnose permission issues, systemd availability problems, or network timeouts.

As per coding guidelines, "Implement comprehensive logging for remote debugging on Raspberry Pi."

📝 Proposed enhancement

 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+ except (subprocess.SubprocessError, OSError) as e:+ app.logger.debug(f"[System Status] Service check failed: {e}")

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@web_interface/app.py` around lines 504 - 505, The except block that currently
swallows errors (except (subprocess.SubprocessError, OSError): pass) should log
the exception for remote debugging; replace the silent pass with a call to the
application's logger (e.g., app.logger.error or logger.exception) including a
descriptive message like "systemctl status check failed" and the exception
details (use exc_info=True or logger.exception to capture stacktrace). Keep the
same exception tuple and context so the surrounding service-status-check logic
is unchanged but now records failures for Raspberry Pi / remote troubleshooting.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@web_interface/blueprints/api_v3.py`:
- Around line 1466-1475: The subprocess.run calls in the start_display handler
(and the similar systemctl invocations later) lack timeouts and can hang the
Flask worker; add a sensible timeout argument (e.g., timeout=10 or configurable)
to each subprocess.run invocation (including the call referenced in the
start_display flow and the other systemctl calls around the later block) and
wrap the call in try/except to catch subprocess.TimeoutExpired; on timeout, log
via logger.error with the exception details and return a JSON error response
(set resp['status']='error', include a clear 'stderr' or 'message' indicating a
timeout and set resp['returncode'] appropriately) so the endpoint fails fast and
doesn't block the worker.
---
Nitpick comments:
In `@web_interface/app.py`:
- Around line 504-505: The except block that currently swallows errors (except
(subprocess.SubprocessError, OSError): pass) should log the exception for remote
debugging; replace the silent pass with a call to the application's logger
(e.g., app.logger.error or logger.exception) including a descriptive message
like "systemctl status check failed" and the exception details (use
exc_info=True or logger.exception to capture stacktrace). Keep the same
exception tuple and context so the surrounding service-status-check logic is
unchanged but now records failures for Raspberry Pi / remote troubleshooting.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: dcea3410-e4ad-4379-bd01-2a5444994bb5

📥 Commits

Reviewing files that changed from the base of the PR and between 0e54eaf and 2f05d9f.

📒 Files selected for processing (2)

• web_interface/app.py
• web_interface/blueprints/api_v3.py