Update error handling in update coordinator (#32452)

homeassistant/components/coronavirus/__init__.py

@@ -3,7 +3,6 @@ import asyncio
 from datetime import timedelta
 import logging
 
-import aiohttp
 import async_timeout
 import coronavirus
 
@@ -73,16 +72,13 @@ async def get_coordinator(hass):
         return hass.data[DOMAIN]
 
     async def async_get_cases():
-        try:
-            with async_timeout.timeout(10):
-                return {
-                    case.country: case
-                    for case in await coronavirus.get_cases(
-                        aiohttp_client.async_get_clientsession(hass)
-                    )
-                }
-        except (asyncio.TimeoutError, aiohttp.ClientError):
-            raise update_coordinator.UpdateFailed
+        with async_timeout.timeout(10):
+            return {
+                case.country: case
+                for case in await coronavirus.get_cases(
+                    aiohttp_client.async_get_clientsession(hass)
+                )
+            }
 
     hass.data[DOMAIN] = update_coordinator.DataUpdateCoordinator(
         hass,

homeassistant/components/hue/light.py

@@ -1,11 +1,9 @@
 """Support for the Philips Hue lights."""
-import asyncio
 from datetime import timedelta
 from functools import partial
 import logging
 import random
 
-from aiohttp import client_exceptions
 import aiohue
 import async_timeout
 
@@ -172,13 +170,9 @@ async def async_safe_fetch(bridge, fetch_method):
             return await bridge.async_request_call(fetch_method)
     except aiohue.Unauthorized:
         await bridge.handle_unauthorized_error()
-        raise UpdateFailed
-    except (
-        asyncio.TimeoutError,
-        aiohue.AiohueException,
-        client_exceptions.ClientError,
-    ):
-        raise UpdateFailed
+        raise UpdateFailed("Unauthorized")
+    except (aiohue.AiohueException,) as err:
+        raise UpdateFailed(f"Hue error: {err}")
 
 
 @callback

homeassistant/components/hue/sensor_base.py

@@ -1,9 +1,7 @@
 """Support for the Philips Hue sensors as a platform."""
-import asyncio
 from datetime import timedelta
 import logging
 
-from aiohttp import client_exceptions
 from aiohue import AiohueException, Unauthorized
 from aiohue.sensors import TYPE_ZLL_PRESENCE
 import async_timeout
@@ -60,9 +58,9 @@ class SensorManager:
                 )
         except Unauthorized:
             await self.bridge.handle_unauthorized_error()
-            raise UpdateFailed
-        except (asyncio.TimeoutError, AiohueException, client_exceptions.ClientError):
-            raise UpdateFailed
+            raise UpdateFailed("Unauthorized")
+        except AiohueException as err:
+            raise UpdateFailed(f"Hue error: {err}")
 
     async def async_register_component(self, binary, async_add_entities):
         """Register async_add_entities methods for components."""

homeassistant/components/tankerkoenig/sensor.py

@@ -36,7 +36,7 @@ async def async_setup_platform(hass, config, async_add_entities, discovery_info=
         try:
             return await tankerkoenig.fetch_data()
         except LookupError:
-            raise UpdateFailed
+            raise UpdateFailed("Failed to fetch data")
 
     coordinator = DataUpdateCoordinator(
         hass,

homeassistant/components/updater/__init__.py

@@ -1,12 +1,10 @@
 """Support to check for available updates."""
-import asyncio
 from datetime import timedelta
 from distutils.version import StrictVersion
 import json
 import logging
 import uuid
 
-import aiohttp
 import async_timeout
 from distro import linux_distribution  # pylint: disable=import-error
 import voluptuous as vol
@@ -156,29 +154,27 @@ async def get_newest_version(hass, huuid, include_components):
         info_object = {}
 
     session = async_get_clientsession(hass)
-    try:
-        with async_timeout.timeout(5):
-            req = await session.post(UPDATER_URL, json=info_object)
-        _LOGGER.info(
-            (
-                "Submitted analytics to Home Assistant servers. "
-                "Information submitted includes %s"
-            ),
-            info_object,
-        )
-    except (asyncio.TimeoutError, aiohttp.ClientError):
-        _LOGGER.error("Could not contact Home Assistant Update to check for updates")
-        raise update_coordinator.UpdateFailed
+
+    with async_timeout.timeout(15):
+        req = await session.post(UPDATER_URL, json=info_object)
+
+    _LOGGER.info(
+        (
+            "Submitted analytics to Home Assistant servers. "
+            "Information submitted includes %s"
+        ),
+        info_object,
+    )
 
     try:
         res = await req.json()
     except ValueError:
-        _LOGGER.error("Received invalid JSON from Home Assistant Update")
-        raise update_coordinator.UpdateFailed
+        raise update_coordinator.UpdateFailed(
+            "Received invalid JSON from Home Assistant Update"
+        )
 
     try:
         res = RESPONSE_SCHEMA(res)
         return res["version"], res["release-notes"]
-    except vol.Invalid:
-        _LOGGER.error("Got unexpected response: %s", res)
-        raise update_coordinator.UpdateFailed
+    except vol.Invalid as err:
+        raise update_coordinator.UpdateFailed(f"Got unexpected response: {err}")

homeassistant/helpers/update_coordinator.py

@@ -5,6 +5,8 @@ import logging
 from time import monotonic
 from typing import Any, Awaitable, Callable, List, Optional
 
+import aiohttp
+
 from homeassistant.core import CALLBACK_TYPE, HomeAssistant, callback
 from homeassistant.helpers.event import async_track_point_in_utc_time
 from homeassistant.util.dt import utcnow
@@ -114,6 +116,16 @@ class DataUpdateCoordinator:
             start = monotonic()
             self.data = await self.update_method()
 
+        except asyncio.TimeoutError:
+            if self.last_update_success:
+                self.logger.error("Timeout fetching %s data", self.name)
+                self.last_update_success = False
+
+        except aiohttp.ClientError as err:
+            if self.last_update_success:
+                self.logger.error("Error requesting %s data: %s", self.name, err)
+                self.last_update_success = False
+
         except UpdateFailed as err:
             if self.last_update_success:
                 self.logger.error("Error fetching %s data: %s", self.name, err)

tests/components/updater/test_init.py

@@ -1,5 +1,4 @@
 """The tests for the Updater component."""
-import asyncio
 from unittest.mock import Mock
 
 from asynctest import patch
@@ -130,17 +129,6 @@ async def test_get_newest_version_analytics_when_huuid(hass, aioclient_mock):
         assert res == (MOCK_RESPONSE["version"], MOCK_RESPONSE["release-notes"])
 
 
-async def test_error_fetching_new_version_timeout(hass):
-    """Test we handle timeout error while fetching new version."""
-    with patch(
-        "homeassistant.helpers.system_info.async_get_system_info",
-        Mock(return_value=mock_coro({"fake": "bla"})),
-    ), patch("async_timeout.timeout", side_effect=asyncio.TimeoutError), pytest.raises(
-        UpdateFailed
-    ):
-        await updater.get_newest_version(hass, MOCK_HUUID, False)
-
-
 async def test_error_fetching_new_version_bad_json(hass, aioclient_mock):
     """Test we handle json error while fetching new version."""
     aioclient_mock.post(updater.UPDATER_URL, text="not json")

tests/helpers/test_update_coordinator.py

@@ -1,7 +1,9 @@
 """Tests for the update coordinator."""
+import asyncio
 from datetime import timedelta
 import logging
 
+import aiohttp
 from asynctest import CoroutineMock, Mock
 import pytest
 
@@ -70,25 +72,30 @@ async def test_request_refresh(crd):
     assert crd.last_update_success is True
 
 
-async def test_refresh_fail(crd, caplog):
-    """Test a failing update function."""
-    crd.update_method = CoroutineMock(side_effect=update_coordinator.UpdateFailed)
+@pytest.mark.parametrize(
+    "err_msg",
+    [
+        (asyncio.TimeoutError, "Timeout fetching test data"),
+        (aiohttp.ClientError, "Error requesting test data"),
+        (update_coordinator.UpdateFailed, "Error fetching test data"),
+    ],
+)
+async def test_refresh_known_errors(err_msg, crd, caplog):
+    """Test raising known errors."""
+    crd.update_method = CoroutineMock(side_effect=err_msg[0])
 
     await crd.async_refresh()
 
     assert crd.data is None
     assert crd.last_update_success is False
-    assert "Error fetching test data" in caplog.text
+    assert err_msg[1] in caplog.text
 
-    crd.update_method = CoroutineMock(return_value=1)
 
+async def test_refresh_fail_unknown(crd, caplog):
+    """Test raising unknown error."""
     await crd.async_refresh()
 
-    assert crd.data == 1
-    assert crd.last_update_success is True
-
     crd.update_method = CoroutineMock(side_effect=ValueError)
-    caplog.clear()
 
     await crd.async_refresh()