Merge pull request #27 from jdejaegh/timeout_handling

Improve handling of API errors such as timeout

custom_components/irm_kmi/api.py

@@ -71,7 +71,7 @@ class IrmKmiApiClient:
         """Get information from the API."""
 
         try:
-            async with async_timeout.timeout(10):
+            async with async_timeout.timeout(60):
                 response = await self._session.request(
                     method=method,
                     url=f"{self._base_url if base_url is None else base_url}{path}",

custom_components/irm_kmi/coordinator.py

@@ -13,8 +13,9 @@ from homeassistant.core import HomeAssistant
 from homeassistant.helpers import issue_registry
 from homeassistant.helpers.aiohttp_client import async_get_clientsession
 from homeassistant.helpers.device_registry import DeviceEntryType, DeviceInfo
-from homeassistant.helpers.update_coordinator import (DataUpdateCoordinator,
+from homeassistant.helpers.update_coordinator import (TimestampDataUpdateCoordinator,
                                                       UpdateFailed)
+from homeassistant.util.dt import utcnow
 
 from .api import IrmKmiApiClient, IrmKmiApiError
 from .const import CONF_DARK_MODE, CONF_STYLE, DOMAIN
@@ -31,7 +32,7 @@ from .utils import disable_from_config, get_config_value
 _LOGGER = logging.getLogger(__name__)
 
 
-class IrmKmiCoordinator(DataUpdateCoordinator):
+class IrmKmiCoordinator(TimestampDataUpdateCoordinator):
     """Coordinator to update data from IRM KMI"""
 
     def __init__(self, hass: HomeAssistant, entry: ConfigEntry):
@@ -67,7 +68,7 @@ class IrmKmiCoordinator(DataUpdateCoordinator):
         try:
             # Note: asyncio.TimeoutError and aiohttp.ClientError are already
             # handled by the data update coordinator.
-            async with async_timeout.timeout(10):
+            async with async_timeout.timeout(60):
                 api_data = await self._api_client.get_forecasts_coord(
                     {'lat': zone.attributes[ATTR_LATITUDE],
                      'long': zone.attributes[ATTR_LONGITUDE]}
@@ -76,7 +77,13 @@ class IrmKmiCoordinator(DataUpdateCoordinator):
                 _LOGGER.debug(f"Full data: {api_data}")
 
         except IrmKmiApiError as err:
-            raise UpdateFailed(f"Error communicating with API: {err}")
+            if self.last_update_success_time is not None \
+                    and self.last_update_success_time - utcnow() < 2.5 * self.update_interval:
+                _LOGGER.warning(f"Error communicating with API for general forecast: {err}. Keeping the old data.")
+                return self.data
+            else:
+                raise UpdateFailed(f"Error communicating with API for general forecast: {err}. "
+                                   f"Last success time is: {self.last_update_success_time}")
 
         if api_data.get('cityName', None) in OUT_OF_BENELUX:
             _LOGGER.error(f"The zone {self._zone} is now out of Benelux and forecast is only available in Benelux."
@@ -114,9 +121,9 @@ class IrmKmiCoordinator(DataUpdateCoordinator):
 
         try:
             images_from_api = await self.download_images_from_api(animation_data, country, localisation_layer_url)
-        except IrmKmiApiError:
+        except IrmKmiApiError as err:
-            _LOGGER.warning(f"Could not get images for weather radar")
+            _LOGGER.warning(f"Could not get images for weather radar: {err}.  Keep the existing radar data.")
-            return RadarAnimationData()
+            return self.data.get('animation', RadarAnimationData()) if self.data is not None else RadarAnimationData()
 
         localisation = images_from_api[0]
         images_from_api = images_from_api[1:]
@@ -148,9 +155,10 @@ class IrmKmiCoordinator(DataUpdateCoordinator):
         try:
             _LOGGER.debug(f"Requesting pollen SVG at url {svg_url}")
             pollen_svg: str = await self._api_client.get_svg(svg_url)
-        except IrmKmiApiError:
+        except IrmKmiApiError as err:
-            _LOGGER.warning(f"Could not get pollen data from the API")
+            _LOGGER.warning(f"Could not get pollen data from the API: {err}. Keeping the same data.")
-            return PollenParser.get_default_data()
+            return self.data.get('pollen', PollenParser.get_unavailable_data()) \
+                if self.data is not None else PollenParser.get_unavailable_data()
 
         return PollenParser(pollen_svg).get_pollen_data()
 
@@ -179,7 +187,7 @@ class IrmKmiCoordinator(DataUpdateCoordinator):
             if frame.get('uri', None) is not None:
                 coroutines.append(
                     self._api_client.get_image(frame.get('uri'), params={'rs': STYLE_TO_PARAM_MAP[self._style]}))
-        async with async_timeout.timeout(20):
+        async with async_timeout.timeout(60):
             images_from_api = await asyncio.gather(*coroutines)
 
         _LOGGER.debug(f"Just downloaded {len(images_from_api)} images")

custom_components/irm_kmi/pollen.py

@@ -8,6 +8,11 @@ from custom_components.irm_kmi.const import POLLEN_NAMES
 _LOGGER = logging.getLogger(__name__)
 
 
+def get_unavailable_data() -> dict:
+    """Return all the known pollen with 'none' value"""
+    return {k.lower(): 'none' for k in POLLEN_NAMES}
+
+
 class PollenParser:
     """
     The SVG looks as follows (see test fixture for a real example)
@@ -25,6 +30,7 @@ class PollenParser:
     For the color scale, look for a white dot (nearly) at the same level as the pollen name.  From the white dot
     horizontal position, determine the level
     """
+
     def __init__(
             self,
             xml_string: str
@@ -53,6 +59,11 @@ class PollenParser:
         """Return all the known pollen with 'none' value"""
         return {k.lower(): 'none' for k in POLLEN_NAMES}
 
+    @staticmethod
+    def get_unavailable_data() -> dict:
+        """Return all the known pollen with 'none' value"""
+        return {k.lower(): None for k in POLLEN_NAMES}
+
     @staticmethod
     def get_option_values() -> List[str]:
         """List all the values that the pollen can have"""
@@ -128,4 +139,3 @@ class PollenParser:
 
         _LOGGER.debug(f"Pollen data: {pollen_data}")
         return pollen_data
-

tests/conftest.py

@@ -262,3 +262,20 @@ def mock_coordinator(request: pytest.FixtureRequest) -> Generator[None, MagicMoc
         coord = coordinator_mock.return_value
         coord._async_animation_data.return_value = {'animation': None}
         yield coord
+
+
+@pytest.fixture()
+def mock_irm_kmi_api_works_but_pollen_and_radar_fail(request: pytest.FixtureRequest) -> Generator[
+    None, MagicMock, None]:
+    """Return a mocked IrmKmi api client."""
+    fixture: str = "forecast.json"
+
+    forecast = json.loads(load_fixture(fixture))
+    with patch(
+            "custom_components.irm_kmi.coordinator.IrmKmiApiClient", autospec=True
+    ) as irm_kmi_api_mock:
+        irm_kmi = irm_kmi_api_mock.return_value
+        irm_kmi.get_forecasts_coord.return_value = forecast
+        irm_kmi.get_svg.side_effect = IrmKmiApiError
+        irm_kmi.get_image.side_effect = IrmKmiApiError
+        yield irm_kmi

tests/test_coordinator.py

@@ -8,7 +8,9 @@ from homeassistant.core import HomeAssistant
 from pytest_homeassistant_custom_component.common import MockConfigEntry
 
 from custom_components.irm_kmi.coordinator import IrmKmiCoordinator
-from custom_components.irm_kmi.data import CurrentWeatherData, IrmKmiForecast
+from custom_components.irm_kmi.data import CurrentWeatherData, IrmKmiForecast, ProcessedCoordinatorData, \
+    RadarAnimationData
+from custom_components.irm_kmi.pollen import PollenParser
 from tests.conftest import get_api_data
 
 
@@ -134,3 +136,44 @@ def test_hourly_forecast() -> None:
     )
 
     assert result[8] == expected
+
+
+async def test_refresh_succeed_even_when_pollen_and_radar_fail(
+        hass: HomeAssistant,
+        mock_config_entry: MockConfigEntry,
+        mock_irm_kmi_api_works_but_pollen_and_radar_fail
+):
+    hass.states.async_set(
+        "zone.home",
+        0,
+        {"latitude": 50.738681639, "longitude": 4.054077148},
+    )
+
+    mock_config_entry.add_to_hass(hass)
+
+    coordinator = IrmKmiCoordinator(hass, mock_config_entry)
+
+    result = await coordinator._async_update_data()
+
+    assert result.get('current_weather').get('condition') == ATTR_CONDITION_CLOUDY
+
+    assert result.get('animation') == dict()
+
+    assert result.get('pollen') == PollenParser.get_unavailable_data()
+
+    existing_data = ProcessedCoordinatorData(
+            current_weather=CurrentWeatherData(),
+            daily_forecast=[],
+            hourly_forecast=[],
+            animation=RadarAnimationData(hint="This will remain unchanged"),
+            warnings=[],
+            pollen={'foo': 'bar'}
+        )
+    coordinator.data = existing_data
+    result = await coordinator._async_update_data()
+
+    assert result.get('current_weather').get('condition') == ATTR_CONDITION_CLOUDY
+
+    assert result.get('animation').get('hint') == "This will remain unchanged"
+
+    assert result.get('pollen') == {'foo': 'bar'}

tests/test_pollen.py

@@ -27,6 +27,7 @@ def test_svg_pollen_parsing():
     assert data == {'birch': 'none', 'oak': 'active', 'hazel': 'none', 'mugwort': 'none', 'alder': 'active',
                     'grasses': 'none', 'ash': 'active'}
 
+
 def test_pollen_options():
     assert PollenParser.get_option_values() == ['active', 'green', 'yellow', 'orange', 'red', 'purple', 'none']
 
@@ -50,7 +51,7 @@ async def test_pollen_data_from_api(
     assert result == expected
 
 
-async def test_pollen_error_leads_to_default_values(
+async def test_pollen_error_leads_to_unavailable_on_first_call(
         hass: HomeAssistant,
         mock_config_entry: MockConfigEntry,
         mock_exception_irm_kmi_api_svg_pollen: AsyncMock
@@ -59,5 +60,5 @@ async def test_pollen_error_leads_to_default_values(
     api_data = get_api_data("be_forecast_warning.json")
 
     result = await coordinator._async_pollen_data(api_data)
-    expected = PollenParser.get_default_data()
+    expected = PollenParser.get_unavailable_data()
     assert result == expected