mirror of
https://github.com/esphome/esphome.git
synced 2026-09-02 02:56:01 +00:00
[api] Don't tear down log connection on stack-trace decode failure
When 'esphome logs' processes a crash backtrace from the device, process_stacktrace -> _decode_pc -> _run_idedata can raise EsphomeError if the local build dir hasn't been populated (e.g. the device was flashed from a different machine). on_log runs inside an asyncio protocol callback, so the unhandled exception triggers 'Fatal error: protocol.data_received() call failed.', the loop tears the connection down, and ReconnectLogic immediately reconnects. The device replays the same crash trace and we loop forever. Wrap the per-line decode in a helper that swallows EsphomeError so the connection stays up. Also covered with unit tests for the new helper.
This commit is contained in:
@@ -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
|
||||
|
||||
@@ -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
|
||||
Reference in New Issue
Block a user