mirror of
https://github.com/ChuckBuilds/LEDMatrix.git
synced 2026-10-16 11:59:13 +00:00
* feat(ipc)!: remove the cache-key mailboxes (control socket stage 5) The control socket is now the only way the web interface sends the display a command. The display stops reading display_on_demand_request and plugin_error_clear_request, and the web interface stops writing them. - Display: no mailbox poll (MailboxWatch, the 1 s / 0.25 s cadence, _consume_on_demand_request, the deprecation log) and no persisted display_on_demand_processed_id guard; the error publisher reads no clear request. CacheManager.file_signature and MailboxWatch are removed. - A write to either retired key is dropped by CacheManager.save_cache and logged once per writer, naming the plugin from the call stack (or the request's plugin_id), with the API to move to. - Web: on-demand start with no display listening starts the service (when start_service) and sends the request again once the socket answers (45 s, 10 s for a running service without a socket yet); every other failure is a 503 (400 for invalid_args). Stop answers 503 when no display listens, unless stop_service. errors/clear answers 503 with a reason-specific message instead of writing a request; clear_pending is always false. src.ipc.client.should_fall_back is replaced by display_not_listening. - Kept: display_current_state, display_on_demand_state, plugin_runtime_snapshot and the heartbeat (read whenever the socket cannot answer), and display_on_demand_config (the display's resume record). Tests: mailbox-only tests removed (test_on_demand_mailbox.py, the mailbox cadence, file_signature and MailboxWatch tests); tests that injected requests through the mailbox now use the socket queue or a plugin's in-process request. The run-loop harness sends on-demand requests over its fake control socket, so four golden traces change: on-demand starts and stops land at the request instant instead of the next 0.25 s mailbox look (one frame fewer on the screen they end), and in vegas.json within one frame instead of 263 ms, which shifts the later 1 s-throttled WiFi-notice check by under a second. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * test: the on-demand routes' other callers ack over the socket, not the mailbox test_api_v3_bool_coercion, test_api_v3_lazy_plugin_discovery and test_web_api's stop test checked the mailbox write; they now get an ack from a mocked control socket. The stop route touches no manager any more, so it leaves the removed-catch-all sample. A request that was sent is never 'not listening' (pins the sent check the mutation run let through). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * feat(web): a start that waits for the display answers 202 and is delivered in the background The start route held a request open for up to 45 s while a cold-started display loaded its plugins; the MQTT bridge (15 s timeout) and browsers reported a failure for a request that was then delivered. Now, when no display is listening, the route starts the service if asked and answers 202 with status "starting" at once. A single worker in the web process (web_interface/on_demand_dispatch.py) sends the request until the display acknowledges it or the wait runs out (45 s cold start, 10 s for a running service without a socket yet). A newer start supersedes the pending one; a stop cancels it (and succeeds, with cancelled_request_id, even with no display listening). The outcome is reported by /display/on-demand/status (source "web": starting, or error with start-timeout or the socket's reason, until the display publishes something newer) and by /display/current-status as on_demand_pending. Callers: the web UI's on-demand modal and "Preview on display" treat "starting" as taken (an info toast); the MQTT bridge already treats any non-error 2xx as success (now pinned by a test). Tests: the dispatcher (ack, retry then ack, start-timeout, other failures, superseded, an in-flight ack for a superseded start, stop while pending, a per-start wait, outcome lifetime); the routes (202, status routes while pending and after a timeout, a later display state replacing the failure, stop while pending, a new start superseding); a JS suite for app.js. Mutation check: 20 mutants on the worker, the routes and app.js, 20 killed. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * fix(web): a cancelled on-demand start never reaches the display after its replacement CI caught a race in test_a_new_start_supersedes_the_pending_one: the dispatcher's worker could read the old start, the route cancel it and send the new one, and the worker's send of the old one land after it. cancel() now waits out a send already in flight (the worker holds a send lock while it reads the pending start and sends it), so whatever the caller sends next lands after it. Pinned by test_a_cancel_waits_for_a_send_in_flight, which fails without the wait. The Linux-only TestRealSocket test for a display that went away now expects the 202 and the start-timeout that follows, as the route answers since the background dispatcher. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * fix(web): a delivered on-demand start reads as starting until the display acts on it On ledpi (three cold starts) the display acknowledged the start as its socket opened, then took ~5 s to act on it while Vegas built its first strip; the status routes meanwhile showed the display's own idle state, so a UI polling every 700 ms flashed idle. The display's on-demand state now names the request it answers (request_id). A delivered start keeps reading as status "starting" with delivered: true, in /display/on-demand/status and as on_demand_pending in /display/current-status, until the display publishes state for that request id (a display without the field: any state newer than the delivery), for at most DELIVERED_SHOWN_SECONDS (30 s). The display's startup state, which can be published after the acknowledgement, names no request and does not end it. Tests: stays starting against the startup idle state (no id, an older id); the matching active state and the matching error take over; an older display's newer state takes over; the 30 s cap; the display's state names its request. Mutation check: 11 mutants, 11 killed. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * fix(display): on_demand_request_id has a class default for controllers built without __init__ _on_demand_state now publishes it, and test_state_stream_readers builds controllers with __new__. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * test: the case-spelling on-demand test patches _send_on_demand now the mailbox is gone Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com> * docs: blank line before 3.8.7 heading --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
236 lines
9.2 KiB
Python
236 lines
9.2 KiB
Python
"""An exception no api_v3 route catches is answered once, by the blueprint.
|
|
|
|
Fifty-three routes used to end in a copy of the same catch-all:
|
|
|
|
except Exception as e:
|
|
logger.error(..., exc_info=True)
|
|
return jsonify({'status': 'error',
|
|
'message': 'An error occurred; see logs for details',
|
|
'details': describe_exception(e)}), 500
|
|
|
|
They were removed in favour of one errorhandler on the api_v3 blueprint. These
|
|
tests pin that the answer did not change: the same status, exactly the same
|
|
keys and values, credentials still redacted, and the traceback still logged.
|
|
|
|
The exact-equality matters. web_interface/app.py's global handler answers with
|
|
an extra `error_code: UNKNOWN_ERROR`, and the plugin API client routes a body
|
|
that carries an error_code to a different UI path (api_client.js) -- so
|
|
"falls through to the global handler" would not have been the same answer.
|
|
"""
|
|
|
|
import logging
|
|
|
|
import pytest
|
|
from flask import Flask
|
|
from werkzeug.exceptions import UnsupportedMediaType
|
|
|
|
from src.web_interface.error_handler import describe_exception
|
|
from web_interface.blueprints.api_v3 import api_v3
|
|
|
|
# A credential in three of the forms describe_exception redacts.
|
|
FORCED = RuntimeError(
|
|
"forced failure token=SECRET123 at https://u:pw1@example.com/x?api_key=K1")
|
|
|
|
# What every removed catch-all returned, written out rather than imported so
|
|
# a change to the shared payload cannot also change the expectation.
|
|
EXPECTED = {
|
|
'status': 'error',
|
|
'message': 'An error occurred; see logs for details',
|
|
'details': describe_exception(FORCED),
|
|
}
|
|
|
|
MANAGERS = ("config_manager", "plugin_catalog", "plugin_store_manager",
|
|
"saved_repositories_manager", "schema_manager", "operation_queue",
|
|
"operation_history", "cache_manager")
|
|
|
|
|
|
class Boom:
|
|
"""A manager that fails on any use -- attribute, truthiness, call."""
|
|
|
|
def _raise(self, *args, **kwargs):
|
|
raise FORCED
|
|
|
|
__getattr__ = _raise
|
|
__bool__ = _raise
|
|
__call__ = _raise
|
|
__iter__ = _raise
|
|
__len__ = _raise
|
|
|
|
|
|
@pytest.fixture
|
|
def exploding_managers(monkeypatch):
|
|
for name in MANAGERS:
|
|
monkeypatch.setattr(api_v3, name, Boom(), raising=False)
|
|
# The WiFi routes build their own manager rather than using one above.
|
|
import src.wifi_manager
|
|
monkeypatch.setattr(src.wifi_manager, "WiFiManager", Boom())
|
|
|
|
|
|
@pytest.fixture
|
|
def client(exploding_managers):
|
|
"""The blueprint alone, on an app with no error handlers of its own."""
|
|
app = Flask(__name__)
|
|
app.register_blueprint(api_v3, url_prefix="/api/v3")
|
|
return app.test_client()
|
|
|
|
|
|
# A sample of routes whose catch-all was removed, across every module that
|
|
# lost one. Each reaches a manager (or WiFiManager) inside what used to be
|
|
# the try block.
|
|
REMOVED_CATCH_ALLS = [
|
|
("GET", "/api/v3/config/main", None),
|
|
("GET", "/api/v3/config/secrets", None),
|
|
("GET", "/api/v3/display/modes", None),
|
|
("GET", "/api/v3/cache/list", None),
|
|
("GET", "/api/v3/plugins/installed", None),
|
|
("GET", "/api/v3/plugins/health", None),
|
|
("GET", "/api/v3/plugins/metrics/some-plugin", None),
|
|
("GET", "/api/v3/plugins/schema?plugin_id=some-plugin", None),
|
|
("GET", "/api/v3/plugins/store/list", None),
|
|
("GET", "/api/v3/plugins/saved-repositories", None),
|
|
("POST", "/api/v3/plugins/install", {"plugin_id": "some-plugin"}),
|
|
("POST", "/api/v3/plugins/config/reset?plugin_id=some-plugin", {}),
|
|
("GET", "/api/v3/plugins/limits/some-plugin", None),
|
|
("GET", "/api/v3/wifi/status", None),
|
|
("POST", "/api/v3/wifi/disconnect", {}),
|
|
]
|
|
|
|
|
|
@pytest.mark.parametrize("method,url,body", REMOVED_CATCH_ALLS,
|
|
ids=[f"{m} {u}" for m, u, _ in REMOVED_CATCH_ALLS])
|
|
def test_the_answer_is_what_the_catch_all_returned(client, caplog, method, url, body):
|
|
with caplog.at_level(logging.ERROR, logger="web_interface.blueprints.api_v3"):
|
|
resp = client.open(url, method=method, json=body)
|
|
|
|
assert resp.status_code == 500
|
|
assert resp.get_json() == EXPECTED
|
|
|
|
# The promise in the message: the traceback is in the log.
|
|
records = [r for r in caplog.records
|
|
if r.name == "web_interface.blueprints.api_v3"
|
|
and r.levelno >= logging.ERROR and r.exc_info]
|
|
assert records, "the unhandled exception was not logged with its traceback"
|
|
assert records[-1].exc_info[1] is FORCED
|
|
|
|
|
|
def test_the_exception_message_never_reaches_the_detail(client):
|
|
body = client.get("/api/v3/plugins/installed").get_json()
|
|
for secret in ("SECRET123", "pw1", "K1", "forced failure"):
|
|
assert secret not in str(body)
|
|
assert body["details"] == "RuntimeError"
|
|
|
|
|
|
def _raise_415():
|
|
raise UnsupportedMediaType(
|
|
"Did not attempt to load JSON data because the request Content-Type "
|
|
"was not 'application/json'.")
|
|
|
|
|
|
# An api_v3 route raising a 415 from inside. No route does that on its own
|
|
# any more (the asset delete route, which used to, now reads its body with
|
|
# get_json(silent=True)), so one route's view is swapped for one that does;
|
|
# the endpoint stays api_v3's, so its error handler is the one that answers.
|
|
_SWAPPED_ENDPOINT = "api_v3.delete_plugin_asset"
|
|
|
|
|
|
def test_a_client_error_keeps_its_own_status(client, monkeypatch):
|
|
"""HTTPExceptions subclass Exception; a 415 must not become a 500."""
|
|
monkeypatch.setitem(client.application.view_functions, _SWAPPED_ENDPOINT, _raise_415)
|
|
resp = client.post("/api/v3/plugins/assets/delete", data="not json",
|
|
content_type="text/plain")
|
|
assert resp.status_code == 415
|
|
body = resp.get_json()
|
|
assert body == {
|
|
'status': 'error',
|
|
'error_code': 'UNSUPPORTED_MEDIA_TYPE',
|
|
'message': body['message'],
|
|
}
|
|
assert "Content-Type" in body['message']
|
|
|
|
|
|
class TestInTheRealApp:
|
|
"""Mounted in web_interface/app.py, beside its global handlers."""
|
|
|
|
@pytest.fixture
|
|
def web_app(self):
|
|
import web_interface.app as web_app
|
|
return web_app
|
|
|
|
def test_the_blueprint_handler_answers_not_the_global_one(
|
|
self, web_app, exploding_managers):
|
|
resp = web_app.app.test_client().get("/api/v3/plugins/installed")
|
|
assert resp.status_code == 500
|
|
assert resp.get_json() == EXPECTED
|
|
|
|
def test_client_errors_read_the_same_as_the_global_handler(
|
|
self, web_app, exploding_managers, monkeypatch):
|
|
"""The blueprint's 4xx shape must not drift from app.py's."""
|
|
monkeypatch.setitem(web_app.app.view_functions, _SWAPPED_ENDPOINT, _raise_415)
|
|
resp = web_app.app.test_client().post(
|
|
"/api/v3/plugins/assets/delete", data="not json",
|
|
content_type="text/plain")
|
|
with web_app.app.test_request_context():
|
|
global_resp, global_status = web_app.handle_exception(
|
|
UnsupportedMediaType(description=resp.get_json()['message']))
|
|
assert resp.status_code == global_status == 415
|
|
assert resp.get_json() == global_resp.get_json()
|
|
|
|
def test_global_handler_shape(self, web_app):
|
|
"""Everything outside api_v3 still gets app.py's answer."""
|
|
with web_app.app.test_request_context("/somewhere"):
|
|
resp, status = web_app.handle_exception(FORCED)
|
|
body = resp.get_json()
|
|
assert status == 500
|
|
assert body == {
|
|
'status': 'error',
|
|
'error_code': 'UNKNOWN_ERROR',
|
|
'message': 'An error occurred; see logs for details',
|
|
'details': describe_exception(FORCED),
|
|
}
|
|
assert "SECRET123" not in body["details"]
|
|
|
|
|
|
class TestPluginActionStep1:
|
|
"""execute_plugin_action's OAuth step-1 handler reports the script's error.
|
|
|
|
The route bound a local `logger` in its JSON-parsing arm, which made
|
|
`logger` local to the whole function; every other `logger.error` in it
|
|
then raised UnboundLocalError. The step-1 handler therefore answered
|
|
"UnboundLocalError: cannot access local variable 'logger'" instead of
|
|
whatever the plugin's auth script actually raised.
|
|
"""
|
|
|
|
@pytest.fixture
|
|
def plugin_dir(self, tmp_path):
|
|
import json
|
|
d = tmp_path / "demo-plugin"
|
|
d.mkdir()
|
|
(d / "manifest.json").write_text(json.dumps({
|
|
"id": "demo-plugin",
|
|
"web_ui_actions": [{"id": "auth", "type": "script",
|
|
"script": "auth.py", "oauth_flow": True}],
|
|
}), encoding="utf-8")
|
|
(d / "auth.py").write_text(
|
|
"def get_auth_url():\n"
|
|
" raise RuntimeError('the auth script failed')\n",
|
|
encoding="utf-8")
|
|
return d
|
|
|
|
def test_the_script_error_is_reported_by_type(self, plugin_dir, monkeypatch):
|
|
from unittest.mock import MagicMock
|
|
manager = MagicMock()
|
|
manager.get_plugin_directory.return_value = str(plugin_dir)
|
|
monkeypatch.setattr(api_v3, "plugin_catalog", manager, raising=False)
|
|
app = Flask(__name__)
|
|
app.register_blueprint(api_v3, url_prefix="/api/v3")
|
|
|
|
resp = app.test_client().post(
|
|
"/api/v3/plugins/action",
|
|
json={"plugin_id": "demo-plugin", "action_id": "auth"})
|
|
|
|
assert resp.status_code == 500
|
|
body = resp.get_json()
|
|
assert body["details"] == "RuntimeError"
|
|
assert "the auth script failed" not in str(body)
|
|
assert body["message"] == 'An error occurred; see logs for details'
|