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'
This commit is contained in:
Jeff Irion 2020-08-29 19:56:25 -07:00 committed by GitHub
parent 904f5c4346
commit ad0d3b4848
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 57 additions and 11 deletions

View File

@ -376,6 +376,12 @@ def adb_decorator(override_available=False):
await self.aftv.adb_close() await self.aftv.adb_close()
self._available = False self._available = False
return None 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 return _adb_exception_catcher
@ -421,10 +427,8 @@ class ADBDevice(MediaPlayerEntity):
# Using "adb_shell" (Python ADB implementation) # Using "adb_shell" (Python ADB implementation)
self.exceptions = ( self.exceptions = (
AdbTimeoutError, AdbTimeoutError,
AttributeError,
BrokenPipeError, BrokenPipeError,
ConnectionResetError, ConnectionResetError,
TypeError,
ValueError, ValueError,
InvalidChecksumError, InvalidChecksumError,
InvalidCommandError, InvalidCommandError,

View File

@ -111,7 +111,7 @@ def patch_shell(response=None, error=False):
async def shell_fail_python(self, cmd, *args, **kwargs): async def shell_fail_python(self, cmd, *args, **kwargs):
"""Mock the `AdbDeviceTcpAsyncFake.shell` method when it fails.""" """Mock the `AdbDeviceTcpAsyncFake.shell` method when it fails."""
self.shell_cmd = cmd self.shell_cmd = cmd
raise AttributeError raise ValueError
async def shell_fail_server(self, cmd): async def shell_fail_server(self, cmd):
"""Mock the `DeviceAsyncFake.shell` method when it fails.""" """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_LAUNCH_APP = patch("androidtv.basetv.basetv_async.BaseTVAsync.launch_app")
PATCH_STOP_APP = patch("androidtv.basetv.basetv_async.BaseTVAsync.stop_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,
)

View File

@ -1,8 +1,10 @@
"""The tests for the androidtv platform.""" """The tests for the androidtv platform."""
import base64 import base64
import copy
import logging import logging
from androidtv.exceptions import LockNotAcquiredException from androidtv.exceptions import LockNotAcquiredException
import pytest
from homeassistant.components.androidtv.media_player import ( from homeassistant.components.androidtv.media_player import (
ANDROIDTV_DOMAIN, ANDROIDTV_DOMAIN,
@ -294,7 +296,7 @@ async def test_adb_shell_returns_none_firetv_adb_server(hass):
async def test_setup_with_adbkey(hass): async def test_setup_with_adbkey(hass):
"""Test that setup succeeds when using an ADB key.""" """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") config[DOMAIN][CONF_ADBKEY] = hass.config.path("user_provided_adbkey")
patch_key, entity_id = _setup(config) patch_key, entity_id = _setup(config)
@ -313,7 +315,7 @@ async def test_setup_with_adbkey(hass):
async def _test_sources(hass, config0): async def _test_sources(hass, config0):
"""Test that sources (i.e., apps) are handled correctly for Android TV and Fire TV devices.""" """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] = { config[DOMAIN][CONF_APPS] = {
"com.app.test1": "TEST 1", "com.app.test1": "TEST 1",
"com.app.test3": None, "com.app.test3": None,
@ -394,7 +396,7 @@ async def test_firetv_sources(hass):
async def _test_exclude_sources(hass, config0, expected_sources): 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.""" """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] = { config[DOMAIN][CONF_APPS] = {
"com.app.test1": "TEST 1", "com.app.test1": "TEST 1",
"com.app.test3": None, "com.app.test3": None,
@ -453,21 +455,21 @@ async def _test_exclude_sources(hass, config0, expected_sources):
async def test_androidtv_exclude_sources(hass): 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.""" """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 config[DOMAIN][CONF_EXCLUDE_UNNAMED_APPS] = True
assert await _test_exclude_sources(hass, config, ["TEST 1"]) assert await _test_exclude_sources(hass, config, ["TEST 1"])
async def test_firetv_exclude_sources(hass): 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.""" """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 config[DOMAIN][CONF_EXCLUDE_UNNAMED_APPS] = True
assert await _test_exclude_sources(hass, config, ["TEST 1"]) assert await _test_exclude_sources(hass, config, ["TEST 1"])
async def _test_select_source(hass, config0, source, expected_arg, method_patch): 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.""" """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} config[DOMAIN][CONF_APPS] = {"com.app.test1": "TEST 1", "com.app.test3": None}
patch_key, entity_id = _setup(config) patch_key, entity_id = _setup(config)
@ -702,7 +704,7 @@ async def test_setup_two_devices(hass):
config = { config = {
DOMAIN: [ DOMAIN: [
CONFIG_ANDROIDTV_ADB_SERVER[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" 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): async def test_services_firetv(hass):
"""Test media player services for a Fire TV device.""" """Test media player services for a Fire TV device."""
patch_key, entity_id = _setup(CONFIG_FIRETV_ADB_SERVER) 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_OFF_COMMAND] = "test off"
config[DOMAIN][CONF_TURN_ON_COMMAND] = "test on" 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) hass.bus.async_fire(EVENT_HOMEASSISTANT_STOP)
await hass.async_block_till_done() await hass.async_block_till_done()
assert adb_close.called 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