Support deCONZ library with exception handling (#21952)

homeassistant/components/deconz/__init__.py

@@ -12,7 +12,7 @@ from .config_flow import configured_hosts
 from .const import DEFAULT_PORT, DOMAIN, _LOGGER
 from .gateway import DeconzGateway
 
-REQUIREMENTS = ['pydeconz==52']
+REQUIREMENTS = ['pydeconz==53']
 
 CONFIG_SCHEMA = vol.Schema({
     DOMAIN: vol.Schema({
@@ -124,8 +124,7 @@ async def async_setup_entry(hass, config_entry):
         scenes = set(gateway.api.scenes.keys())
         sensors = set(gateway.api.sensors.keys())
 
-        if not await gateway.api.async_load_parameters():
+        await gateway.api.async_load_parameters()
-            return
 
         gateway.async_add_device_callback(
             'group', [group

homeassistant/components/deconz/config_flow.py

@@ -1,4 +1,6 @@
 """Config flow to configure deCONZ component."""
+import asyncio
+import async_timeout
 import voluptuous as vol
 
 from homeassistant import config_entries
@@ -32,15 +34,12 @@ class DeconzFlowHandler(config_entries.ConfigFlow):
         self.deconz_config = {}
 
     async def async_step_user(self, user_input=None):
-        """Handle a flow initialized by the user."""
-        return await self.async_step_init(user_input)
-
-    async def async_step_init(self, user_input=None):
         """Handle a deCONZ config flow start.
 
         Only allows one instance to be set up.
         If only one bridge is found go to link step.
         If more than one bridge is found let user choose bridge to link.
+        If no bridge is found allow user to manually input configuration.
         """
         from pydeconz.utils import async_discovery
 
@@ -52,11 +51,18 @@ class DeconzFlowHandler(config_entries.ConfigFlow):
                 if bridge[CONF_HOST] == user_input[CONF_HOST]:
                     self.deconz_config = bridge
                     return await self.async_step_link()
+
             self.deconz_config = user_input
             return await self.async_step_link()
 
         session = aiohttp_client.async_get_clientsession(self.hass)
-        self.bridges = await async_discovery(session)
+
+        try:
+            with async_timeout.timeout(10):
+                self.bridges = await async_discovery(session)
+
+        except asyncio.TimeoutError:
+            self.bridges = []
 
         if len(self.bridges) == 1:
             self.deconz_config = self.bridges[0]
@@ -64,8 +70,10 @@ class DeconzFlowHandler(config_entries.ConfigFlow):
 
         if len(self.bridges) > 1:
             hosts = []
+
             for bridge in self.bridges:
                 hosts.append(bridge[CONF_HOST])
+
             return self.async_show_form(
                 step_id='init',
                 data_schema=vol.Schema({
@@ -74,7 +82,7 @@ class DeconzFlowHandler(config_entries.ConfigFlow):
             )
 
         return self.async_show_form(
-            step_id='user',
+            step_id='init',
             data_schema=vol.Schema({
                 vol.Required(CONF_HOST): str,
                 vol.Required(CONF_PORT, default=DEFAULT_PORT): int,
@@ -83,18 +91,27 @@ class DeconzFlowHandler(config_entries.ConfigFlow):
 
     async def async_step_link(self, user_input=None):
         """Attempt to link with the deCONZ bridge."""
+        from pydeconz.errors import ResponseError, RequestError
         from pydeconz.utils import async_get_api_key
         errors = {}
 
         if user_input is not None:
             if configured_hosts(self.hass):
                 return self.async_abort(reason='one_instance_only')
+
             session = aiohttp_client.async_get_clientsession(self.hass)
-            api_key = await async_get_api_key(session, **self.deconz_config)
+
-            if api_key:
+            try:
+                with async_timeout.timeout(10):
+                    api_key = await async_get_api_key(
+                        session, **self.deconz_config)
+
+            except (ResponseError, RequestError, asyncio.TimeoutError):
+                errors['base'] = 'no_key'
+
+            else:
                 self.deconz_config[CONF_API_KEY] = api_key
                 return await self.async_step_options()
-            errors['base'] = 'no_key'
 
         return self.async_show_form(
             step_id='link',
@@ -117,8 +134,14 @@ class DeconzFlowHandler(config_entries.ConfigFlow):
 
             if CONF_BRIDGEID not in self.deconz_config:
                 session = aiohttp_client.async_get_clientsession(self.hass)
-                self.deconz_config[CONF_BRIDGEID] = await async_get_bridgeid(
+                try:
-                    session, **self.deconz_config)
+                    with async_timeout.timeout(10):
+                        self.deconz_config[CONF_BRIDGEID] = \
+                            await async_get_bridgeid(
+                                session, **self.deconz_config)
+
+                except asyncio.TimeoutError:
+                    return self.async_abort(reason='no_bridges')
 
             return self.async_create_entry(
                 title='deCONZ-' + self.deconz_config[CONF_BRIDGEID],

homeassistant/components/deconz/errors.py

@@ -0,0 +1,18 @@
+"""Errors for the deCONZ component."""
+from homeassistant.exceptions import HomeAssistantError
+
+
+class DeconzException(HomeAssistantError):
+    """Base class for deCONZ exceptions."""
+
+
+class AlreadyConfigured(DeconzException):
+    """Gateway is already configured."""
+
+
+class AuthenticationRequired(DeconzException):
+    """Unknown error occurred."""
+
+
+class CannotConnect(DeconzException):
+    """Unable to connect to the gateway."""

homeassistant/components/deconz/gateway.py

@@ -1,6 +1,9 @@
 """Representation of a deCONZ gateway."""
+import asyncio
+import async_timeout
+
 from homeassistant.exceptions import ConfigEntryNotReady
-from homeassistant.const import CONF_EVENT, CONF_ID
+from homeassistant.const import CONF_EVENT, CONF_HOST, CONF_ID
 from homeassistant.core import EventOrigin, callback
 from homeassistant.helpers import aiohttp_client
 from homeassistant.helpers.dispatcher import (
@@ -10,6 +13,7 @@ from homeassistant.util import slugify
 from .const import (
     _LOGGER, DECONZ_REACHABLE, CONF_ALLOW_CLIP_SENSOR, NEW_DEVICE, NEW_SENSOR,
     SUPPORTED_PLATFORMS)
+from .errors import AuthenticationRequired, CannotConnect
 
 
 class DeconzGateway:
@@ -26,18 +30,23 @@ class DeconzGateway:
         self.events = []
         self.listeners = []
 
-    async def async_setup(self, tries=0):
+    async def async_setup(self):
         """Set up a deCONZ gateway."""
         hass = self.hass
 
-        self.api = await get_gateway(
+        try:
-            hass, self.config_entry.data, self.async_add_device_callback,
+            self.api = await get_gateway(
-            self.async_connection_status_callback
+                hass, self.config_entry.data, self.async_add_device_callback,
-        )
+                self.async_connection_status_callback
+            )
 
-        if not self.api:
+        except CannotConnect:
             raise ConfigEntryNotReady
 
+        except Exception:  # pylint: disable=broad-except
+            _LOGGER.error('Error connecting with deCONZ gateway.')
+            return False
+
         for component in SUPPORTED_PLATFORMS:
             hass.async_create_task(
                 hass.config_entries.async_forward_entry_setup(
@@ -113,17 +122,26 @@ class DeconzGateway:
 async def get_gateway(hass, config, async_add_device_callback,
                       async_connection_status_callback):
     """Create a gateway object and verify configuration."""
-    from pydeconz import DeconzSession
+    from pydeconz import DeconzSession, errors
 
     session = aiohttp_client.async_get_clientsession(hass)
+
     deconz = DeconzSession(hass.loop, session, **config,
                            async_add_device=async_add_device_callback,
                            connection_status=async_connection_status_callback)
-    result = await deconz.async_load_parameters()
+    try:
-
+        with async_timeout.timeout(10):
-    if result:
+            await deconz.async_load_parameters()
         return deconz
-    return result
+
+    except errors.Unauthorized:
+        _LOGGER.warning("Invalid key for deCONZ at %s.", config[CONF_HOST])
+        raise AuthenticationRequired
+
+    except (asyncio.TimeoutError, errors.RequestError):
+        _LOGGER.error(
+            "Error connecting to deCONZ gateway at %s", config[CONF_HOST])
+        raise CannotConnect
 
 
 class DeconzEvent:

requirements_all.txt

@@ -1001,7 +1001,7 @@ pydaikin==1.1.0
 pydanfossair==0.0.7
 
 # homeassistant.components.deconz
-pydeconz==52
+pydeconz==53
 
 # homeassistant.components.zwave
 pydispatcher==2.0.5

requirements_test_all.txt

@@ -200,7 +200,7 @@ pyHS100==0.3.4
 pyblackbird==0.5
 
 # homeassistant.components.deconz
-pydeconz==52
+pydeconz==53
 
 # homeassistant.components.zwave
 pydispatcher==2.0.5

tests/components/deconz/test_climate.py

@@ -46,11 +46,14 @@ async def setup_gateway(hass, data, allow_clip_sensor=True):
     """Load the deCONZ sensor platform."""
     from pydeconz import DeconzSession
 
-    session = Mock(put=asynctest.CoroutineMock(
+    response = Mock(
-        return_value=Mock(status=200,
+        status=200, json=asynctest.CoroutineMock(),
-                          json=asynctest.CoroutineMock(),
+        text=asynctest.CoroutineMock())
-                          text=asynctest.CoroutineMock(),
+    response.content_type = 'application/json'
-                          )
+
+    session = Mock(
+        put=asynctest.CoroutineMock(
+            return_value=response
         )
     )
 

tests/components/deconz/test_config_flow.py

@@ -1,7 +1,8 @@
 """Tests for deCONZ config flow."""
-import pytest
+from unittest.mock import patch
+
+import asyncio
 
-import voluptuous as vol
 from homeassistant.components.deconz import config_flow
 from tests.common import MockConfigEntry
 
@@ -12,15 +13,17 @@ async def test_flow_works(hass, aioclient_mock):
     """Test that config flow works."""
     aioclient_mock.get(pydeconz.utils.URL_DISCOVER, json=[
         {'id': 'id', 'internalipaddress': '1.2.3.4', 'internalport': 80}
-    ])
+    ], headers={'content-type': 'application/json'})
     aioclient_mock.post('http://1.2.3.4:80/api', json=[
         {"success": {"username": "1234567890ABCDEF"}}
-    ])
+    ], headers={'content-type': 'application/json'})
 
     flow = config_flow.DeconzFlowHandler()
     flow.hass = hass
+
     await flow.async_step_user()
     await flow.async_step_link(user_input={})
+
     result = await flow.async_step_options(
         user_input={'allow_clip_sensor': True, 'allow_deconz_groups': True})
 
@@ -41,35 +44,53 @@ async def test_flow_already_registered_bridge(hass):
     MockConfigEntry(domain='deconz', data={
         'host': '1.2.3.4'
     }).add_to_hass(hass)
+
     flow = config_flow.DeconzFlowHandler()
     flow.hass = hass
 
-    result = await flow.async_step_init()
+    result = await flow.async_step_user()
     assert result['type'] == 'abort'
 
 
+async def test_flow_bridge_discovery_fails(hass, aioclient_mock):
+    """Test config flow works when discovery fails."""
+    flow = config_flow.DeconzFlowHandler()
+    flow.hass = hass
+
+    with patch('pydeconz.utils.async_discovery',
+               side_effect=asyncio.TimeoutError):
+        result = await flow.async_step_user()
+
+    assert result['type'] == 'form'
+    assert result['step_id'] == 'init'
+
+
 async def test_flow_no_discovered_bridges(hass, aioclient_mock):
     """Test config flow discovers no bridges."""
-    aioclient_mock.get(pydeconz.utils.URL_DISCOVER, json=[])
+    aioclient_mock.get(pydeconz.utils.URL_DISCOVER, json=[],
+                       headers={'content-type': 'application/json'})
+
     flow = config_flow.DeconzFlowHandler()
     flow.hass = hass
 
-    result = await flow.async_step_init()
+    result = await flow.async_step_user()
     assert result['type'] == 'form'
-    assert result['step_id'] == 'user'
+    assert result['step_id'] == 'init'
 
 
 async def test_flow_one_bridge_discovered(hass, aioclient_mock):
     """Test config flow discovers one bridge."""
     aioclient_mock.get(pydeconz.utils.URL_DISCOVER, json=[
         {'id': 'id', 'internalipaddress': '1.2.3.4', 'internalport': 80}
-    ])
+    ], headers={'content-type': 'application/json'})
+
     flow = config_flow.DeconzFlowHandler()
     flow.hass = hass
 
-    result = await flow.async_step_init()
+    result = await flow.async_step_user()
     assert result['type'] == 'form'
     assert result['step_id'] == 'link'
+    assert flow.deconz_config['host'] == '1.2.3.4'
 
 
 async def test_flow_two_bridges_discovered(hass, aioclient_mock):
@@ -77,19 +98,14 @@ async def test_flow_two_bridges_discovered(hass, aioclient_mock):
     aioclient_mock.get(pydeconz.utils.URL_DISCOVER, json=[
         {'id': 'id1', 'internalipaddress': '1.2.3.4', 'internalport': 80},
         {'id': 'id2', 'internalipaddress': '5.6.7.8', 'internalport': 80}
-    ])
+    ], headers={'content-type': 'application/json'})
+
     flow = config_flow.DeconzFlowHandler()
     flow.hass = hass
 
-    result = await flow.async_step_init()
+    result = await flow.async_step_user()
-    assert result['type'] == 'form'
+    assert result['data_schema']({'host': '1.2.3.4'})
-    assert result['step_id'] == 'init'
+    assert result['data_schema']({'host': '5.6.7.8'})
-
-    with pytest.raises(vol.Invalid):
-        assert result['data_schema']({'host': '0.0.0.0'})
-
-    result['data_schema']({'host': '1.2.3.4'})
-    result['data_schema']({'host': '5.6.7.8'})
 
 
 async def test_flow_two_bridges_selection(hass, aioclient_mock):
@@ -101,7 +117,7 @@ async def test_flow_two_bridges_selection(hass, aioclient_mock):
         {'bridgeid': 'id2', 'host': '5.6.7.8', 'port': 80}
     ]
 
-    result = await flow.async_step_init(user_input={'host': '1.2.3.4'})
+    result = await flow.async_step_user(user_input={'host': '1.2.3.4'})
     assert result['type'] == 'form'
     assert result['step_id'] == 'link'
     assert flow.deconz_config['host'] == '1.2.3.4'
@@ -110,25 +126,28 @@ async def test_flow_two_bridges_selection(hass, aioclient_mock):
 async def test_flow_manual_configuration(hass, aioclient_mock):
     """Test config flow with manual input."""
     aioclient_mock.get(pydeconz.utils.URL_DISCOVER, json=[])
+
     flow = config_flow.DeconzFlowHandler()
     flow.hass = hass
 
     user_input = {'host': '1.2.3.4', 'port': 80}
 
-    result = await flow.async_step_init(user_input)
+    result = await flow.async_step_user(user_input)
     assert result['type'] == 'form'
     assert result['step_id'] == 'link'
     assert flow.deconz_config == user_input
 
 
-async def test_link_no_api_key(hass, aioclient_mock):
+async def test_link_no_api_key(hass):
     """Test config flow should abort if no API key was possible to retrieve."""
-    aioclient_mock.post('http://1.2.3.4:80/api', json=[])
     flow = config_flow.DeconzFlowHandler()
     flow.hass = hass
     flow.deconz_config = {'host': '1.2.3.4', 'port': 80}
 
-    result = await flow.async_step_link(user_input={})
+    with patch('pydeconz.utils.async_get_api_key',
+               side_effect=pydeconz.errors.ResponseError):
+        result = await flow.async_step_link(user_input={})
+
     assert result['type'] == 'form'
     assert result['step_id'] == 'link'
     assert result['errors'] == {'base': 'no_key'}
@@ -143,6 +162,7 @@ async def test_link_already_registered_bridge(hass):
     MockConfigEntry(domain='deconz', data={
         'host': '1.2.3.4'
     }).add_to_hass(hass)
+
     flow = config_flow.DeconzFlowHandler()
     flow.hass = hass
     flow.deconz_config = {'host': '1.2.3.4', 'port': 80}
@@ -155,6 +175,7 @@ async def test_bridge_discovery(hass):
     """Test a bridge being discovered."""
     flow = config_flow.DeconzFlowHandler()
     flow.hass = hass
+
     result = await flow.async_step_discovery({
         'host': '1.2.3.4',
         'port': 80,
@@ -222,14 +243,18 @@ async def test_import_with_api_key(hass):
 async def test_options(hass, aioclient_mock):
     """Test that options work and that bridgeid can be requested."""
     aioclient_mock.get('http://1.2.3.4:80/api/1234567890ABCDEF/config',
-                       json={"bridgeid": "id"})
+                       json={"bridgeid": "id"},
+                       headers={'content-type': 'application/json'})
+
     flow = config_flow.DeconzFlowHandler()
     flow.hass = hass
     flow.deconz_config = {'host': '1.2.3.4',
                           'port': 80,
                           'api_key': '1234567890ABCDEF'}
+
     result = await flow.async_step_options(
         user_input={'allow_clip_sensor': False, 'allow_deconz_groups': False})
+
     assert result['type'] == 'create_entry'
     assert result['title'] == 'deCONZ-id'
     assert result['data'] == {

tests/components/deconz/test_gateway.py

@@ -4,10 +4,13 @@ from unittest.mock import Mock, patch
 import pytest
 
 from homeassistant.exceptions import ConfigEntryNotReady
-from homeassistant.components.deconz import gateway
+from homeassistant.components.deconz import errors, gateway
 
 from tests.common import mock_coro
 
+import pydeconz
+
+
 ENTRY_CONFIG = {
     "host": "1.2.3.4",
     "port": 80,
@@ -62,11 +65,25 @@ async def test_gateway_retry():
     deconz_gateway = gateway.DeconzGateway(hass, entry)
 
     with patch.object(
-            gateway, 'get_gateway', return_value=mock_coro(False)
+            gateway, 'get_gateway', side_effect=errors.CannotConnect), \
-    ), pytest.raises(ConfigEntryNotReady):
+            pytest.raises(ConfigEntryNotReady):
         await deconz_gateway.async_setup()
 
 
+async def test_gateway_setup_fails():
+    """Retry setup."""
+    hass = Mock()
+    entry = Mock()
+    entry.data = ENTRY_CONFIG
+
+    deconz_gateway = gateway.DeconzGateway(hass, entry)
+
+    with patch.object(gateway, 'get_gateway', side_effect=Exception):
+        result = await deconz_gateway.async_setup()
+
+    assert not result
+
+
 async def test_connection_status(hass):
     """Make sure that connection status triggers a dispatcher send."""
     entry = Mock()
@@ -170,10 +187,20 @@ async def test_get_gateway(hass):
         assert await gateway.get_gateway(hass, ENTRY_CONFIG, Mock(), Mock())
 
 
-async def test_get_gateway_fails(hass):
+async def test_get_gateway_fails_unauthorized(hass):
     """Failed call."""
     with patch('pydeconz.DeconzSession.async_load_parameters',
-               return_value=mock_coro(False)):
+               side_effect=pydeconz.errors.Unauthorized), \
+            pytest.raises(errors.AuthenticationRequired):
+        assert await gateway.get_gateway(
+            hass, ENTRY_CONFIG, Mock(), Mock()) is False
+
+
+async def test_get_gateway_fails_cannot_connect(hass):
+    """Failed call."""
+    with patch('pydeconz.DeconzSession.async_load_parameters',
+               side_effect=pydeconz.errors.RequestError), \
+            pytest.raises(errors.CannotConnect):
         assert await gateway.get_gateway(
             hass, ENTRY_CONFIG, Mock(), Mock()) is False
 

tests/components/deconz/test_init.py

@@ -1,6 +1,7 @@
 """Test deCONZ component setup process."""
 from unittest.mock import Mock, patch
 
+import asyncio
 import pytest
 import voluptuous as vol
 
@@ -76,13 +77,22 @@ async def test_setup_entry_already_registered_bridge(hass):
     assert await deconz.async_setup_entry(hass, {}) is False
 
 
+async def test_setup_entry_fails(hass):
+    """Test setup entry fails if deCONZ is not available."""
+    entry = Mock()
+    entry.data = {'host': '1.2.3.4', 'port': 80, 'api_key': '1234567890ABCDEF'}
+    with patch('pydeconz.DeconzSession.async_load_parameters',
+               side_effect=Exception):
+        await deconz.async_setup_entry(hass, entry)
+
+
 async def test_setup_entry_no_available_bridge(hass):
     """Test setup entry fails if deCONZ is not available."""
     entry = Mock()
     entry.data = {'host': '1.2.3.4', 'port': 80, 'api_key': '1234567890ABCDEF'}
     with patch(
         'pydeconz.DeconzSession.async_load_parameters',
-        return_value=mock_coro(False)
+        side_effect=asyncio.TimeoutError
     ), pytest.raises(ConfigEntryNotReady):
         await deconz.async_setup_entry(hass, entry)
 
@@ -185,6 +195,7 @@ async def test_service_refresh_devices(hass):
     })
     entry.add_to_hass(hass)
     mock_registry = Mock()
+
     with patch.object(deconz, 'DeconzGateway') as mock_gateway, \
         patch('homeassistant.helpers.device_registry.async_get_registry',
               return_value=mock_coro(mock_registry)):
@@ -196,6 +207,7 @@ async def test_service_refresh_devices(hass):
         await hass.services.async_call(
             'deconz', 'device_refresh', service_data={})
         await hass.async_block_till_done()
+
     with patch.object(hass.data[deconz.DOMAIN].api, 'async_load_parameters',
                       return_value=mock_coro(False)):
         await hass.services.async_call(