Hue to retry if hub errors out (#31616)

* Respect semaphore

* Add retries when connection reset

* Also catch OSError from aiohttp when authenticating

homeassistant/components/hue/bridge.py

@@ -1,6 +1,8 @@
 """Code to handle a Hue bridge."""
 import asyncio
+from functools import partial
 
+from aiohttp import client_exceptions
 import aiohue
 import async_timeout
 import slugify as unicode_slug
@@ -21,6 +23,8 @@ ATTR_SCENE_NAME = "scene_name"
 SCENE_SCHEMA = vol.Schema(
     {vol.Required(ATTR_GROUP_NAME): cv.string, vol.Required(ATTR_SCENE_NAME): cv.string}
 )
+# How long should we sleep if the hub is busy
+HUB_BUSY_SLEEP = 0.01
 
 
 class HueBridge:
@@ -101,11 +105,33 @@ class HueBridge:
         self.authorized = True
         return True
 
-    async def async_request_call(self, coro):
+    async def async_request_call(self, task):
-        """Process request batched."""
+        """Limit parallel requests to Hue hub.
 
+        The Hue hub can only handle a certain amount of parallel requests, total.
+        Although we limit our parallel requests, we still will run into issues because
+        other products are hitting up Hue.
+
+        ClientOSError means hub closed the socket on us.
+        ContentResponseError means hub raised an error.
+        Since we don't make bad requests, this is on them.
+        """
         async with self.parallel_updates_semaphore:
-            return await coro
+            for tries in range(4):
+                try:
+                    return await task()
+                except (
+                    client_exceptions.ClientOSError,
+                    client_exceptions.ClientResponseError,
+                ) as err:
+                    if tries == 3 or (
+                        # We only retry if it's a server error. So raise on all 4XX errors.
+                        isinstance(err, client_exceptions.ClientResponseError)
+                        and err.status < 500
+                    ):
+                        raise
+
+                    await asyncio.sleep(HUB_BUSY_SLEEP * tries)
 
     async def async_reset(self):
         """Reset this bridge to default state.
@@ -167,8 +193,8 @@ class HueBridge:
 
         # If we can't find it, fetch latest info.
         if not updated and (group is None or scene is None):
-            await self.api.groups.update()
+            await self.async_request_call(self.api.groups.update)
-            await self.api.scenes.update()
+            await self.async_request_call(self.api.scenes.update)
             await self.hue_activate_scene(call, updated=True)
             return
 
@@ -180,7 +206,7 @@ class HueBridge:
             LOGGER.warning("Unable to find scene %s", scene_name)
             return
 
-        await group.set_action(scene=scene.id)
+        await self.async_request_call(partial(group.set_action, scene=scene.id))
 
     async def handle_unauthorized_error(self):
         """Create a new config flow when the authorization is no longer valid."""
@@ -210,7 +236,7 @@ async def authenticate_bridge(hass: core.HomeAssistant, bridge: aiohue.Bridge):
 
     except (aiohue.LinkButtonNotPressed, aiohue.Unauthorized):
         raise AuthenticationRequired
-    except (asyncio.TimeoutError, aiohue.RequestError):
+    except (asyncio.TimeoutError, client_exceptions.ClientOSError):
         raise CannotConnect
     except aiohue.AiohueException:
         LOGGER.exception("Unknown Hue linking error occurred")

homeassistant/components/hue/const.py

@@ -3,7 +3,6 @@ import logging
 
 LOGGER = logging.getLogger(__package__)
 DOMAIN = "hue"
-API_NUPNP = "https://www.meethue.com/api/nupnp"
 
 # How long to wait to actually do the refresh after requesting it.
 # We wait some time so if we control multiple lights, we batch requests.

homeassistant/components/hue/light.py

@@ -153,7 +153,7 @@ async def async_safe_fetch(bridge, fetch_method):
     """Safely fetch data."""
     try:
         with async_timeout.timeout(4):
-            return await bridge.async_request_call(fetch_method())
+            return await bridge.async_request_call(fetch_method)
     except aiohue.Unauthorized:
         await bridge.handle_unauthorized_error()
         raise UpdateFailed
@@ -376,9 +376,13 @@ class HueLight(Light):
                 command["effect"] = "none"
 
         if self.is_group:
-            await self.bridge.async_request_call(self.light.set_action(**command))
+            await self.bridge.async_request_call(
+                partial(self.light.set_action, **command)
+            )
         else:
-            await self.bridge.async_request_call(self.light.set_state(**command))
+            await self.bridge.async_request_call(
+                partial(self.light.set_state, **command)
+            )
 
         await self.coordinator.async_request_refresh()
 
@@ -401,9 +405,13 @@ class HueLight(Light):
             command["alert"] = "none"
 
         if self.is_group:
-            await self.bridge.async_request_call(self.light.set_action(**command))
+            await self.bridge.async_request_call(
+                partial(self.light.set_action, **command)
+            )
         else:
-            await self.bridge.async_request_call(self.light.set_state(**command))
+            await self.bridge.async_request_call(
+                partial(self.light.set_state, **command)
+            )
 
         await self.coordinator.async_request_refresh()
 

homeassistant/components/hue/sensor_base.py

@@ -55,7 +55,7 @@ class SensorManager:
         try:
             with async_timeout.timeout(4):
                 return await self.bridge.async_request_call(
-                    self.bridge.api.sensors.update()
+                    self.bridge.api.sensors.update
                 )
         except Unauthorized:
             await self.bridge.handle_unauthorized_error()

tests/components/hue/test_config_flow.py

@@ -2,7 +2,9 @@
 import asyncio
 from unittest.mock import Mock
 
+from aiohttp import client_exceptions
 import aiohue
+from aiohue.discovery import URL_NUPNP
 from asynctest import CoroutineMock, patch
 import pytest
 import voluptuous as vol
@@ -84,7 +86,7 @@ async def test_flow_works(hass):
 
 async def test_flow_no_discovered_bridges(hass, aioclient_mock):
     """Test config flow discovers no bridges."""
-    aioclient_mock.get(const.API_NUPNP, json=[])
+    aioclient_mock.get(URL_NUPNP, json=[])
 
     result = await hass.config_entries.flow.async_init(
         const.DOMAIN, context={"source": "user"}
@@ -95,9 +97,7 @@ async def test_flow_no_discovered_bridges(hass, aioclient_mock):
 
 async def test_flow_all_discovered_bridges_exist(hass, aioclient_mock):
     """Test config flow discovers only already configured bridges."""
-    aioclient_mock.get(
+    aioclient_mock.get(URL_NUPNP, json=[{"internalipaddress": "1.2.3.4", "id": "bla"}])
-        const.API_NUPNP, json=[{"internalipaddress": "1.2.3.4", "id": "bla"}]
-    )
     MockConfigEntry(
         domain="hue", unique_id="bla", data={"host": "1.2.3.4"}
     ).add_to_hass(hass)
@@ -111,9 +111,7 @@ async def test_flow_all_discovered_bridges_exist(hass, aioclient_mock):
 
 async def test_flow_one_bridge_discovered(hass, aioclient_mock):
     """Test config flow discovers one bridge."""
-    aioclient_mock.get(
+    aioclient_mock.get(URL_NUPNP, json=[{"internalipaddress": "1.2.3.4", "id": "bla"}])
-        const.API_NUPNP, json=[{"internalipaddress": "1.2.3.4", "id": "bla"}]
-    )
 
     result = await hass.config_entries.flow.async_init(
         const.DOMAIN, context={"source": "user"}
@@ -130,7 +128,7 @@ async def test_flow_two_bridges_discovered(hass, aioclient_mock):
     ).add_to_hass(hass)
 
     aioclient_mock.get(
-        const.API_NUPNP,
+        URL_NUPNP,
         json=[
             {"internalipaddress": "1.2.3.4", "id": "bla"},
             {"internalipaddress": "5.6.7.8", "id": "beer"},
@@ -153,7 +151,7 @@ async def test_flow_two_bridges_discovered(hass, aioclient_mock):
 async def test_flow_two_bridges_discovered_one_new(hass, aioclient_mock):
     """Test config flow discovers two bridges."""
     aioclient_mock.get(
-        const.API_NUPNP,
+        URL_NUPNP,
         json=[
             {"internalipaddress": "1.2.3.4", "id": "bla"},
             {"internalipaddress": "5.6.7.8", "id": "beer"},
@@ -259,7 +257,7 @@ async def test_flow_link_button_not_pressed(hass):
 async def test_flow_link_unknown_host(hass):
     """Test config flow ."""
     mock_bridge = get_mock_bridge(
-        mock_create_user=CoroutineMock(side_effect=aiohue.RequestError),
+        mock_create_user=CoroutineMock(side_effect=client_exceptions.ClientOSError),
     )
     with patch(
         "homeassistant.components.hue.config_flow.discover_nupnp",

tests/components/hue/test_light.py

@@ -206,8 +206,8 @@ def mock_bridge(hass):
             return bridge.mock_group_responses.popleft()
         return None
 
-    async def async_request_call(coro):
+    async def async_request_call(task):
-        await coro
+        await task()
 
     bridge.async_request_call = async_request_call
     bridge.api.config.apiversion = "9.9.9"

tests/components/hue/test_sensor_base.py

@@ -279,8 +279,8 @@ def create_mock_bridge(hass):
             return bridge.mock_sensor_responses.popleft()
         return None
 
-    async def async_request_call(coro):
+    async def async_request_call(task):
-        await coro
+        await task()
 
     bridge.async_request_call = async_request_call
     bridge.api.config.apiversion = "9.9.9"