diff --git a/esphome/components/api/client.py b/esphome/components/api/client.py index 312d937f01..31ae8163da 100644 --- a/esphome/components/api/client.py +++ b/esphome/components/api/client.py @@ -18,7 +18,7 @@ with warnings.catch_warnings(): import contextlib from esphome.const import CONF_KEY, CONF_PORT, __version__ -from esphome.core import CORE +from esphome.core import CORE, EsphomeError from esphome.platformio_api import process_stacktrace from . import CONF_ENCRYPTION @@ -32,6 +32,32 @@ if TYPE_CHECKING: _LOGGER = logging.getLogger(__name__) +def _process_stacktrace_line( + config: dict[str, Any], + raw_line: str, + backtrace_state: bool, + platform_process_stacktrace: Any | None, +) -> bool: + """Run the stack-trace decoder for a single log line. + + on_log runs inside an asyncio protocol callback; if an exception + escapes, the loop tears the transport down with "Fatal error: + protocol.data_received() call failed." and ReconnectLogic + immediately reconnects, the device replays the same crash trace, + and we loop forever. Stack-trace decoding requires a populated + build dir for the device, which may not exist (e.g. flashed from + another machine); log and continue instead of killing the + connection. + """ + try: + if platform_process_stacktrace: + return platform_process_stacktrace(config, raw_line, backtrace_state) + return process_stacktrace(config, raw_line, backtrace_state=backtrace_state) + except EsphomeError as exc: + _LOGGER.debug("Stack-trace decoding failed: %s", exc) + return False + + async def async_run_logs( config: dict[str, Any], addresses: list[str], @@ -84,14 +110,9 @@ async def async_run_logs( for parsed_msg in parse_log_message(text, timestamp): print(parsed_msg.replace("\033", "\\033") if dashboard else parsed_msg) for raw_line in text.splitlines(): - if platform_process_stacktrace: - backtrace_state = platform_process_stacktrace( - config, raw_line, backtrace_state - ) - else: - backtrace_state = process_stacktrace( - config, raw_line, backtrace_state=backtrace_state - ) + backtrace_state = _process_stacktrace_line( + config, raw_line, backtrace_state, platform_process_stacktrace + ) # Safe to fall back to plaintext here only for this diagnostics use # case: the stream is one-way from device to client, and this code diff --git a/tests/unit_tests/components/api/__init__.py b/tests/unit_tests/components/api/__init__.py new file mode 100644 index 0000000000..e69de29bb2 diff --git a/tests/unit_tests/components/api/test_client.py b/tests/unit_tests/components/api/test_client.py new file mode 100644 index 0000000000..9f038b093d --- /dev/null +++ b/tests/unit_tests/components/api/test_client.py @@ -0,0 +1,80 @@ +"""Tests for esphome.components.api.client.""" + +from __future__ import annotations + +from unittest.mock import patch + +from esphome.components.api import client as api_client +from esphome.core import EsphomeError + + +def test_process_stacktrace_line_swallows_esphome_error() -> None: + """A failing stack-trace decode must not propagate. + + on_log runs inside an asyncio protocol callback; if EsphomeError + escapes, the loop reports "Fatal error: protocol.data_received() + call failed.", tears the connection down, and ReconnectLogic loops + forever as the device replays the same crash trace on every + reconnect. + """ + config = {"esphome": {"name": "test"}} + + with patch.object( + api_client, "process_stacktrace", side_effect=EsphomeError("no idedata") + ) as mock_process: + result = api_client._process_stacktrace_line( + config, "PC: 0x4010496e", True, None + ) + + assert mock_process.called + assert result is False + + +def test_process_stacktrace_line_swallows_platform_handler_error() -> None: + """The same protection must apply to the platform-specific handler.""" + config = {"esphome": {"name": "test"}} + + def platform_handler(_config, _line, _state): + raise EsphomeError("no idedata") + + result = api_client._process_stacktrace_line( + config, "PC: 0x4010496e", True, platform_handler + ) + + assert result is False + + +def test_process_stacktrace_line_returns_handler_result() -> None: + """When decoding succeeds, the handler's result is returned unchanged.""" + config = {"esphome": {"name": "test"}} + + with patch.object( + api_client, "process_stacktrace", return_value=True + ) as mock_process: + result = api_client._process_stacktrace_line( + config, "PC: 0x4010496e", False, None + ) + + mock_process.assert_called_once_with( + config, "PC: 0x4010496e", backtrace_state=False + ) + assert result is True + + +def test_process_stacktrace_line_uses_platform_handler_when_provided() -> None: + """The platform handler is preferred over the generic one.""" + config = {"esphome": {"name": "test"}} + calls: list[tuple[object, str, bool]] = [] + + def platform_handler(cfg, line, state): + calls.append((cfg, line, state)) + return True + + with patch.object(api_client, "process_stacktrace") as mock_generic: + result = api_client._process_stacktrace_line( + config, "BT0: 0x4010496e", False, platform_handler + ) + + assert calls == [(config, "BT0: 0x4010496e", False)] + assert mock_generic.called is False + assert result is True