From ad0d3b48480d6d2dcaa77e0d7f8ce8298b4621d1 Mon Sep 17 00:00:00 2001 From: Jeff Irion Date: Sat, 29 Aug 2020 19:56:25 -0700 Subject: [PATCH] Improve handling of exceptions in Android TV (#39229) * Close the ADB connection when there is an exception * Add a test * Split a comment onto two lines * Fix test ('async_update' -> 'async_update_entity') * 'close' -> 'Close' --- .../components/androidtv/media_player.py | 8 ++- tests/components/androidtv/patchers.py | 8 ++- .../components/androidtv/test_media_player.py | 52 ++++++++++++++++--- 3 files changed, 57 insertions(+), 11 deletions(-) diff --git a/homeassistant/components/androidtv/media_player.py b/homeassistant/components/androidtv/media_player.py index a46685a1c65..959c85abd77 100644 --- a/homeassistant/components/androidtv/media_player.py +++ b/homeassistant/components/androidtv/media_player.py @@ -376,6 +376,12 @@ def adb_decorator(override_available=False): await self.aftv.adb_close() self._available = False return None + except Exception: + # An unforeseen exception occurred. Close the ADB connection so that + # it doesn't happen over and over again, then raise the exception. + await self.aftv.adb_close() + self._available = False # pylint: disable=protected-access + raise return _adb_exception_catcher @@ -421,10 +427,8 @@ class ADBDevice(MediaPlayerEntity): # Using "adb_shell" (Python ADB implementation) self.exceptions = ( AdbTimeoutError, - AttributeError, BrokenPipeError, ConnectionResetError, - TypeError, ValueError, InvalidChecksumError, InvalidCommandError, diff --git a/tests/components/androidtv/patchers.py b/tests/components/androidtv/patchers.py index 98c38719cf0..a89abe33de3 100644 --- a/tests/components/androidtv/patchers.py +++ b/tests/components/androidtv/patchers.py @@ -111,7 +111,7 @@ def patch_shell(response=None, error=False): async def shell_fail_python(self, cmd, *args, **kwargs): """Mock the `AdbDeviceTcpAsyncFake.shell` method when it fails.""" self.shell_cmd = cmd - raise AttributeError + raise ValueError async def shell_fail_server(self, cmd): """Mock the `DeviceAsyncFake.shell` method when it fails.""" @@ -182,3 +182,9 @@ def patch_androidtv_update( PATCH_LAUNCH_APP = patch("androidtv.basetv.basetv_async.BaseTVAsync.launch_app") PATCH_STOP_APP = patch("androidtv.basetv.basetv_async.BaseTVAsync.stop_app") + +# Cause the update to raise an unexpected type of exception +PATCH_ANDROIDTV_UPDATE_EXCEPTION = patch( + "androidtv.androidtv.androidtv_async.AndroidTVAsync.update", + side_effect=ZeroDivisionError, +) diff --git a/tests/components/androidtv/test_media_player.py b/tests/components/androidtv/test_media_player.py index 2641c2b349d..456a9313091 100644 --- a/tests/components/androidtv/test_media_player.py +++ b/tests/components/androidtv/test_media_player.py @@ -1,8 +1,10 @@ """The tests for the androidtv platform.""" import base64 +import copy import logging from androidtv.exceptions import LockNotAcquiredException +import pytest from homeassistant.components.androidtv.media_player import ( ANDROIDTV_DOMAIN, @@ -294,7 +296,7 @@ async def test_adb_shell_returns_none_firetv_adb_server(hass): async def test_setup_with_adbkey(hass): """Test that setup succeeds when using an ADB key.""" - config = CONFIG_ANDROIDTV_PYTHON_ADB.copy() + config = copy.deepcopy(CONFIG_ANDROIDTV_PYTHON_ADB) config[DOMAIN][CONF_ADBKEY] = hass.config.path("user_provided_adbkey") patch_key, entity_id = _setup(config) @@ -313,7 +315,7 @@ async def test_setup_with_adbkey(hass): async def _test_sources(hass, config0): """Test that sources (i.e., apps) are handled correctly for Android TV and Fire TV devices.""" - config = config0.copy() + config = copy.deepcopy(config0) config[DOMAIN][CONF_APPS] = { "com.app.test1": "TEST 1", "com.app.test3": None, @@ -394,7 +396,7 @@ async def test_firetv_sources(hass): async def _test_exclude_sources(hass, config0, expected_sources): """Test that sources (i.e., apps) are handled correctly when the `exclude_unnamed_apps` config parameter is provided.""" - config = config0.copy() + config = copy.deepcopy(config0) config[DOMAIN][CONF_APPS] = { "com.app.test1": "TEST 1", "com.app.test3": None, @@ -453,21 +455,21 @@ async def _test_exclude_sources(hass, config0, expected_sources): async def test_androidtv_exclude_sources(hass): """Test that sources (i.e., apps) are handled correctly for Android TV devices when the `exclude_unnamed_apps` config parameter is provided as true.""" - config = CONFIG_ANDROIDTV_ADB_SERVER.copy() + config = copy.deepcopy(CONFIG_ANDROIDTV_ADB_SERVER) config[DOMAIN][CONF_EXCLUDE_UNNAMED_APPS] = True assert await _test_exclude_sources(hass, config, ["TEST 1"]) async def test_firetv_exclude_sources(hass): """Test that sources (i.e., apps) are handled correctly for Fire TV devices when the `exclude_unnamed_apps` config parameter is provided as true.""" - config = CONFIG_FIRETV_ADB_SERVER.copy() + config = copy.deepcopy(CONFIG_FIRETV_ADB_SERVER) config[DOMAIN][CONF_EXCLUDE_UNNAMED_APPS] = True assert await _test_exclude_sources(hass, config, ["TEST 1"]) async def _test_select_source(hass, config0, source, expected_arg, method_patch): """Test that the methods for launching and stopping apps are called correctly when selecting a source.""" - config = config0.copy() + config = copy.deepcopy(config0) config[DOMAIN][CONF_APPS] = {"com.app.test1": "TEST 1", "com.app.test3": None} patch_key, entity_id = _setup(config) @@ -702,7 +704,7 @@ async def test_setup_two_devices(hass): config = { DOMAIN: [ CONFIG_ANDROIDTV_ADB_SERVER[DOMAIN], - CONFIG_FIRETV_ADB_SERVER[DOMAIN].copy(), + copy.deepcopy(CONFIG_FIRETV_ADB_SERVER[DOMAIN]), ] } config[DOMAIN][1][CONF_HOST] = "127.0.0.2" @@ -1145,7 +1147,7 @@ async def test_services_androidtv(hass): async def test_services_firetv(hass): """Test media player services for a Fire TV device.""" patch_key, entity_id = _setup(CONFIG_FIRETV_ADB_SERVER) - config = CONFIG_FIRETV_ADB_SERVER.copy() + config = copy.deepcopy(CONFIG_FIRETV_ADB_SERVER) config[DOMAIN][CONF_TURN_OFF_COMMAND] = "test off" config[DOMAIN][CONF_TURN_ON_COMMAND] = "test on" @@ -1177,3 +1179,37 @@ async def test_connection_closed_on_ha_stop(hass): hass.bus.async_fire(EVENT_HOMEASSISTANT_STOP) await hass.async_block_till_done() assert adb_close.called + + +async def test_exception(hass): + """Test that the ADB connection gets closed when there is an unforeseen exception. + + HA will attempt to reconnect on the next update. + """ + patch_key, entity_id = _setup(CONFIG_ANDROIDTV_PYTHON_ADB) + + with patchers.PATCH_ADB_DEVICE_TCP, patchers.patch_connect(True)[ + patch_key + ], patchers.patch_shell(SHELL_RESPONSE_OFF)[ + patch_key + ], patchers.PATCH_KEYGEN, patchers.PATCH_ANDROIDTV_OPEN, patchers.PATCH_SIGNER: + assert await async_setup_component(hass, DOMAIN, CONFIG_ANDROIDTV_PYTHON_ADB) + await hass.async_block_till_done() + + await hass.helpers.entity_component.async_update_entity(entity_id) + state = hass.states.get(entity_id) + assert state is not None + assert state.state == STATE_OFF + + # When an unforessen exception occurs, we close the ADB connection and raise the exception + with patchers.PATCH_ANDROIDTV_UPDATE_EXCEPTION, pytest.raises(Exception): + await hass.helpers.entity_component.async_update_entity(entity_id) + state = hass.states.get(entity_id) + assert state is not None + assert state.state == STATE_UNAVAILABLE + + # On the next update, HA will reconnect to the device + await hass.helpers.entity_component.async_update_entity(entity_id) + state = hass.states.get(entity_id) + assert state is not None + assert state.state == STATE_OFF