From 8ab396d77c77606393b925dede574f11390cbc28 Mon Sep 17 00:00:00 2001 From: Stefan Agner Date: Tue, 20 Aug 2024 17:55:53 +0200 Subject: [PATCH] Improve and extend error handling on D-Bus connect (#5245) * Improve and extend error handling on D-Bus connect Avoid initializing the Supervisor board since it does not support the Properties interface (see https://github.com/home-assistant/os-agent/issues/206). This prevents the following somewhat confusing warning: No OS-Agent support on the host. Some Host functions have been disabled. The OS Agent is actually installed on the host, it is just a single object which caused issues. No functionalty was actually lost, as the Supervisor board object has no features currently, and all other interfaces got properly initialized still (thanks to gather()). Print warnings more fine graned so it is clear which object exactly causes an issue. Also print a log message on the lowest layer when an error occures on calling D-Bus. This allows to easier track the actual D-Bus error source. Fixes: #5241 * Fix tests * Use local variable * Avoid stack trace when board support fails to load * Fix tests * Use override mechanism to disable Properties support Instead of disable loading of Supervised entirly override initialization to prevent loading the Properties interface. * Revert "Fix tests" This reverts commit 1e3c491ace2ebef60945c45573acd8d4464ea83e. --- supervisor/dbus/agent/__init__.py | 22 +++++++++++++++++----- supervisor/dbus/agent/boards/__init__.py | 8 ++++++-- supervisor/dbus/agent/boards/supervised.py | 12 ++++++++++++ supervisor/dbus/manager.py | 6 ++++-- supervisor/utils/dbus.py | 7 +++++++ tests/dbus/agent/test_agent.py | 21 +++++++++++++++------ 6 files changed, 61 insertions(+), 15 deletions(-) diff --git a/supervisor/dbus/agent/__init__.py b/supervisor/dbus/agent/__init__.py index 57bcf0d34..21c14afe5 100644 --- a/supervisor/dbus/agent/__init__.py +++ b/supervisor/dbus/agent/__init__.py @@ -8,7 +8,7 @@ from typing import Any from awesomeversion import AwesomeVersion from dbus_fast.aio.message_bus import MessageBus -from ...exceptions import DBusError, DBusInterfaceError, DBusServiceUnkownError +from ...exceptions import DBusInterfaceError, DBusServiceUnkownError from ..const import ( DBUS_ATTR_DIAGNOSTICS, DBUS_ATTR_VERSION, @@ -96,13 +96,25 @@ class OSAgent(DBusInterfaceProxy): _LOGGER.info("Load dbus interface %s", self.name) try: await super().connect(bus) - await asyncio.gather(*[dbus.connect(bus) for dbus in self.all]) - except DBusError: - _LOGGER.warning("Can't connect to OS-Agent") except (DBusServiceUnkownError, DBusInterfaceError): - _LOGGER.warning( + _LOGGER.error( "No OS-Agent support on the host. Some Host functions have been disabled." ) + return + + errors = await asyncio.gather( + *[dbus.connect(bus) for dbus in self.all], return_exceptions=True + ) + + for err in errors: + if err: + dbus = self.all[errors.index(err)] + _LOGGER.error( + "Can't load OS Agent dbus interface %s %s: %s", + dbus.bus_name, + dbus.object_path, + err, + ) @dbus_connected async def update(self, changed: dict[str, Any] | None = None) -> None: diff --git a/supervisor/dbus/agent/boards/__init__.py b/supervisor/dbus/agent/boards/__init__.py index c05a6d3f4..67eb3c129 100644 --- a/supervisor/dbus/agent/boards/__init__.py +++ b/supervisor/dbus/agent/boards/__init__.py @@ -4,7 +4,7 @@ import logging from dbus_fast.aio.message_bus import MessageBus -from ....exceptions import BoardInvalidError +from ....exceptions import BoardInvalidError, DBusInterfaceError, DBusServiceUnkownError from ...const import ( DBUS_ATTR_BOARD, DBUS_IFACE_HAOS_BOARDS, @@ -75,6 +75,10 @@ class BoardManager(DBusInterfaceProxy): self._board_proxy = Green() elif self.board == BOARD_NAME_SUPERVISED: self._board_proxy = Supervised() + else: + return - if self._board_proxy: + try: await self._board_proxy.connect(bus) + except (DBusServiceUnkownError, DBusInterfaceError) as ex: + _LOGGER.warning("OS-Agent board support initialization failed: %s", ex) diff --git a/supervisor/dbus/agent/boards/supervised.py b/supervisor/dbus/agent/boards/supervised.py index 0ab4af796..e46292c00 100644 --- a/supervisor/dbus/agent/boards/supervised.py +++ b/supervisor/dbus/agent/boards/supervised.py @@ -1,5 +1,9 @@ """Supervised board management.""" +from typing import Any + +from supervisor.dbus.utils import dbus_connected + from .const import BOARD_NAME_SUPERVISED from .interface import BoardProxy @@ -11,3 +15,11 @@ class Supervised(BoardProxy): """Initialize properties.""" super().__init__(BOARD_NAME_SUPERVISED) self.sync_properties: bool = False + + @dbus_connected + async def update(self, changed: dict[str, Any] | None = None) -> None: + """Do nothing as there are no properties. + + Currently unused, avoid using the Properties interface to avoid a bug in + Go D-Bus, see: https://github.com/home-assistant/os-agent/issues/206 + """ diff --git a/supervisor/dbus/manager.py b/supervisor/dbus/manager.py index d2397063b..11474620a 100644 --- a/supervisor/dbus/manager.py +++ b/supervisor/dbus/manager.py @@ -129,9 +129,11 @@ class DBusManager(CoreSysAttributes): for err in errors: if err: + dbus = self.all[errors.index(err)] _LOGGER.warning( - "Can't load dbus interface %s: %s", - self.all[errors.index(err)].name, + "Can't load dbus interface %s %s: %s", + dbus.name, + dbus.object_path, err, ) diff --git a/supervisor/utils/dbus.py b/supervisor/utils/dbus.py index 0e3c0c721..3953c3df6 100644 --- a/supervisor/utils/dbus.py +++ b/supervisor/utils/dbus.py @@ -112,6 +112,13 @@ class DBus: ) return await getattr(proxy_interface, method)(*args) except DBusFastDBusError as err: + _LOGGER.debug( + "D-Bus fast error on call - %s.%s on %s: %s", + proxy_interface.introspection.name, + method, + proxy_interface.path, + err, + ) raise DBus.from_dbus_error(err) from None except Exception as err: # pylint: disable=broad-except capture_exception(err) diff --git a/tests/dbus/agent/test_agent.py b/tests/dbus/agent/test_agent.py index 055415857..5298ff146 100644 --- a/tests/dbus/agent/test_agent.py +++ b/tests/dbus/agent/test_agent.py @@ -44,15 +44,24 @@ async def test_dbus_osagent( @pytest.mark.parametrize( - "skip_service", + "skip_service,error", [ - "os_agent", - "agent_apparmor", - "agent_datadisk", + ("os_agent", "No OS-Agent support on the host"), + ( + "agent_apparmor", + "Can't load OS Agent dbus interface io.hass.os /io/hass/os/AppArmor", + ), + ( + "agent_datadisk", + "Can't load OS Agent dbus interface io.hass.os /io/hass/os/DataDisk", + ), ], ) async def test_dbus_osagent_connect_error( - skip_service: str, dbus_session_bus: MessageBus, caplog: pytest.LogCaptureFixture + skip_service: str, + error: str, + dbus_session_bus: MessageBus, + caplog: pytest.LogCaptureFixture, ): """Test OS Agent errors during connect.""" os_agent_services = { @@ -73,4 +82,4 @@ async def test_dbus_osagent_connect_error( os_agent = OSAgent() await os_agent.connect(dbus_session_bus) - assert "No OS-Agent support on the host" in caplog.text + assert error in caplog.text