From 5db8082f91e22f16fd20d2a18d1ccbdc5a9c8606 Mon Sep 17 00:00:00 2001 From: ollo69 <60491700+ollo69@users.noreply.github.com> Date: Tue, 7 May 2024 01:00:12 +0200 Subject: [PATCH] Review AndroidTV decorator exception management (#114133) --- homeassistant/components/androidtv/entity.py | 31 +++++++++++++------ .../components/androidtv/test_media_player.py | 26 +++++++++++----- 2 files changed, 39 insertions(+), 18 deletions(-) diff --git a/homeassistant/components/androidtv/entity.py b/homeassistant/components/androidtv/entity.py index 11ae7bc6290..7df80c187cd 100644 --- a/homeassistant/components/androidtv/entity.py +++ b/homeassistant/components/androidtv/entity.py @@ -77,22 +77,33 @@ def adb_decorator( ) return None except self.exceptions as err: - _LOGGER.error( - ( - "Failed to execute an ADB command. ADB connection re-" - "establishing attempt in the next update. Error: %s" - ), - err, - ) + if self.available: + _LOGGER.error( + ( + "Failed to execute an ADB command. ADB connection re-" + "establishing attempt in the next update. Error: %s" + ), + err, + ) + await self.aftv.adb_close() self._attr_available = False return None - except Exception: + except Exception as err: # pylint: disable=broad-except # An unforeseen exception occurred. Close the ADB connection so that - # it doesn't happen over and over again, then raise the exception. + # it doesn't happen over and over again. + if self.available: + _LOGGER.error( + ( + "Unexpected exception executing an ADB command. ADB connection" + " re-establishing attempt in the next update. Error: %s" + ), + err, + ) + await self.aftv.adb_close() self._attr_available = False - raise + return None return _adb_exception_catcher diff --git a/tests/components/androidtv/test_media_player.py b/tests/components/androidtv/test_media_player.py index fe6b9962d14..af2927a23f3 100644 --- a/tests/components/androidtv/test_media_player.py +++ b/tests/components/androidtv/test_media_player.py @@ -1,5 +1,6 @@ """The tests for the androidtv platform.""" +from collections.abc import Generator from datetime import timedelta import logging from typing import Any @@ -170,7 +171,7 @@ CONFIG_FIRETV_DEFAULT = CONFIG_FIRETV_PYTHON_ADB @pytest.fixture(autouse=True) -def adb_device_tcp_fixture() -> None: +def adb_device_tcp_fixture() -> Generator[None, patchers.AdbDeviceTcpAsyncFake, None]: """Patch ADB Device TCP.""" with patch( "androidtv.adb_manager.adb_manager_async.AdbDeviceTcpAsync", @@ -180,7 +181,7 @@ def adb_device_tcp_fixture() -> None: @pytest.fixture(autouse=True) -def load_adbkey_fixture() -> None: +def load_adbkey_fixture() -> Generator[None, str, None]: """Patch load_adbkey.""" with patch( "homeassistant.components.androidtv.ADBPythonSync.load_adbkey", @@ -190,7 +191,7 @@ def load_adbkey_fixture() -> None: @pytest.fixture(autouse=True) -def keygen_fixture() -> None: +def keygen_fixture() -> Generator[None, Mock, None]: """Patch keygen.""" with patch( "homeassistant.components.androidtv.keygen", @@ -1181,7 +1182,7 @@ async def test_connection_closed_on_ha_stop(hass: HomeAssistant) -> None: assert adb_close.called -async def test_exception(hass: HomeAssistant) -> None: +async def test_exception(hass: HomeAssistant, caplog: pytest.LogCaptureFixture) -> None: """Test that the ADB connection gets closed when there is an unforeseen exception. HA will attempt to reconnect on the next update. @@ -1201,12 +1202,21 @@ async def test_exception(hass: HomeAssistant) -> None: assert state is not None assert state.state == STATE_OFF + caplog.clear() + caplog.set_level(logging.ERROR) + # When an unforeseen exception occurs, we close the ADB connection and raise the exception - with patchers.PATCH_ANDROIDTV_UPDATE_EXCEPTION, pytest.raises(Exception): + with patchers.PATCH_ANDROIDTV_UPDATE_EXCEPTION: await async_update_entity(hass, entity_id) - state = hass.states.get(entity_id) - assert state is not None - assert state.state == STATE_UNAVAILABLE + + state = hass.states.get(entity_id) + assert state is not None + assert state.state == STATE_UNAVAILABLE + assert len(caplog.record_tuples) == 1 + assert caplog.record_tuples[0][1] == logging.ERROR + assert caplog.record_tuples[0][2].startswith( + "Unexpected exception executing an ADB command" + ) # On the next update, HA will reconnect to the device await async_update_entity(hass, entity_id)