fix: do not fail config entry setup silently

custom_components/inpost_air/__init__.py

@@ -1,7 +1,7 @@
 """The InPost Air integration."""
 
 from __future__ import annotations
-from dataclasses import dataclass
+from dataclasses import asdict, dataclass
 import logging
 
 from dacite import from_dict
@@ -15,7 +15,7 @@
 from custom_components.inpost_air.models import ParcelLocker
 from custom_components.inpost_air.utils import get_device_info, get_parcel_locker_url
 
-from .api import InPostAirPoint, InPostApi
+from .api import InPostAirApiClientError, InPostAirPoint, InPostApi
 
 _LOGGER = logging.getLogger(__name__)
 
@@ -35,34 +35,80 @@ class InPostAirData:
 PLATFORMS: list[Platform] = [Platform.SENSOR]
 
 
+def get_configured_point(entry: InPostAirConfiEntry) -> InPostAirPoint:
+    """Read parcel locker data stored in the config entry."""
+    entry_data = entry.data.get("parcel_locker")
+
+    if entry_data is None:
+        raise ConfigEntryError(
+            "Config entry does not contain any parcel locker data. "
+            "Please remove the integration and set it up again."
+        )
+
+    if isinstance(entry_data, InPostAirPoint):
+        return entry_data
+
+    try:
+        return from_dict(InPostAirPoint, entry_data)
+    except Exception as ex:
+        raise ConfigEntryError(
+            f"Parcel locker data stored in the config entry is invalid: {ex}"
+        ) from ex
+
+
+async def resolve_parcel_locker(
+    hass: HomeAssistant,
+    entry: InPostAirConfiEntry,
+    api_client: InPostApi,
+    point: InPostAirPoint,
+) -> tuple[InPostAirPoint, str]:
+    """Find the parcel locker ID, refreshing outdated config entry data if needed."""
+    refreshed_point = None
+
+    try:
+        if (locker_id := await api_client.find_parcel_locker_id(point)) is not None:
+            return point, locker_id
+
+        _LOGGER.info(
+            "Refetching data of parcel locker %s - the one stored in the config entry might be outdated",
+            point.n,
+        )
+        refreshed_point = await api_client.refresh_parcel_locker(point.n)
+
+        if refreshed_point is not None and refreshed_point != point:
+            locker_id = await api_client.find_parcel_locker_id(refreshed_point)
+    except InPostAirApiClientError as err:
+        # Transient problem on InPost side - let Home Assistant retry the setup.
+        raise ConfigEntryNotReady(f"Error communicating with InPost: {err}") from err
+
+    if locker_id is None:
+        raise ConfigEntryNotReady(
+            f"Could not find air quality data of parcel locker {point.n} on {get_parcel_locker_url(point)}"
+        )
+
+    hass.config_entries.async_update_entry(
+        entry,
+        data={**entry.data, "parcel_locker": asdict(refreshed_point)},
+    )
+
+    return refreshed_point, locker_id
+
+
 async def async_setup_entry(hass: HomeAssistant, entry: InPostAirConfiEntry) -> bool:
     """Set up InPost Air from a config entry."""
     api_client = InPostApi(hass)
-    entry_data = entry.data.get("parcel_locker")
+    point = get_configured_point(entry)
 
-    if (
-        point := None
-        if entry_data is None
-        else entry_data
-        if isinstance(entry_data, InPostAirPoint)
-        else from_dict(InPostAirPoint, entry_data)
-    ) is None:
-        return False
-
-    if (parcel_locker_id := await api_client.find_parcel_locker_id(point)) is None:
-        return False
+    point, parcel_locker_id = await resolve_parcel_locker(
+        hass, entry, api_client, point
+    )
 
     parcel_locker = ParcelLocker(point.n, parcel_locker_id)
     coordinator = InPostAirDataCoordinator(hass, api_client, parcel_locker)
 
     entry.runtime_data = InPostAirData(parcel_locker, coordinator)
 
-    try:
-        await coordinator.async_config_entry_first_refresh()
-    except ConfigEntryNotReady as ex:
-        if "Air sensors are not available" in str(ex):
-            raise ConfigEntryError(ex)
-        raise ex
+    await coordinator.async_config_entry_first_refresh()
 
     device_registry = dr.async_get(hass)
     device_registry.async_get_or_create(
@@ -92,12 +138,19 @@ async def async_migrate_entry(hass: HomeAssistant, config_entry: InPostAirConfiE
 
     if config_entry.version > 2:
         # This means the user has downgraded from a future version
+        _LOGGER.error(
+            "Cannot migrate %s from version %s - it was created by a newer version of the integration",
+            config_entry.title,
+            config_entry.version,
+        )
         return False
 
     if config_entry.version == 1:
         hass.config_entries.async_update_entry(
             config_entry,
-            data={"parcel_locker": from_dict(InPostAirPoint, config_entry.data)},
+            data={
+                "parcel_locker": asdict(from_dict(InPostAirPoint, config_entry.data))
+            },
             version=2,
         )
 

custom_components/inpost_air/api.py

@@ -2,9 +2,11 @@
 
 import asyncio
 from dataclasses import dataclass
+import json
 import logging
 import re
-from aiohttp import ClientResponse, ClientResponseError
+from typing import Any
+from aiohttp import ClientResponseError
 from dacite import from_dict
 from homeassistant.core import HomeAssistant
 from homeassistant.helpers.aiohttp_client import async_create_clientsession
@@ -13,6 +15,24 @@
 
 _LOGGER = logging.getLogger(__name__)
 
+REQUEST_TIMEOUT = 30
+
+AIR_DATA_PATTERN = re.compile(
+    r"data-shipx-url=\"/shipx-point-data/(.*?)/(.*?)/air_index_level\""
+)
+
+
+@dataclass
+class ApiResponse:
+    """API response with an already read body."""
+
+    url: str
+    body: str
+
+    def json(self) -> Any:
+        """Parse the body as JSON."""
+        return json.loads(self.body)
+
 
 @dataclass
 class ParcelLockerListResponse:
@@ -43,34 +63,41 @@ async def _request(
         url: str,
         headers: dict | None = None,
         raise_client_response_error: bool = False,
-    ) -> ClientResponse:
-        """Get information from the API."""
+    ) -> ApiResponse:
+        """Get information from the API.
+
+        The body is read inside the timeout on purpose - `session.request`
+        returns as soon as the response headers arrive, so reading the body
+        outside of it would leave that part of the request without any limit.
+        """
         try:
-            async with asyncio.timeout(30):
+            async with asyncio.timeout(REQUEST_TIMEOUT):
                 response = await self.session.request(
                     method=method,
                     url=url,
                     headers=headers,
                 )
                 response.raise_for_status()
 
-                return response
+                return ApiResponse(url=str(response.url), body=await response.text())
 
         except TimeoutError as e:
-            _LOGGER.warning("Request timed out")
+            _LOGGER.warning("Request to %s timed out", url)
             raise InPostAirApiClientError("Request timed out") from e
         except ClientResponseError as e:
             if raise_client_response_error:
                 raise
-            raise InPostAirApiClientError("Something really wrong happened!") from e
+            _LOGGER.warning("Request to %s failed with status %s", url, e.status)
+            raise InPostAirApiClientError(
+                f"Request to {url} failed with status {e.status}"
+            ) from e
         except Exception as exception:  # pylint: disable=broad-except
+            _LOGGER.warning("Request to %s failed: %s", url, exception)
             raise InPostAirApiClientError(
                 "Something really wrong happened!"
             ) from exception
 
-    async def _search_easypack24_locker(
-        self, locker_code: str
-    ) -> InPostAirPoint | None:
+    async def _search_easypack24_locker(self, locker_code: str) -> dict | None:
         """Find info about given parcel locker."""
         if not locker_code or locker_code == "":
             return None
@@ -79,7 +106,7 @@ async def _search_easypack24_locker(
             method="get",
             url="https://api-shipx-pl.easypack24.net/v1/points/" + locker_code,
         )
-        resp = await response.json()
+        resp = response.json()
 
         error = resp.get("error")
         if error:
@@ -120,11 +147,7 @@ async def search_parcel_locker(self, locker_code: str) -> InPostAirPoint | None:
             method="get", url="https://inpost.pl/sites/default/files/points.json"
         )
         parcel_locker = next(
-            (
-                x
-                for x in (await response.json()).get("items")
-                if x.get("n") == locker_code
-            ),
+            (x for x in response.json().get("items") if x.get("n") == locker_code),
             None,
         )
 
@@ -133,27 +156,54 @@ async def search_parcel_locker(self, locker_code: str) -> InPostAirPoint | None:
 
         return from_dict(InPostAirPoint, parcel_locker) if parcel_locker else None
 
+    async def refresh_parcel_locker(self, locker_code: str) -> InPostAirPoint | None:
+        """Get current data of an already known parcel locker.
+
+        Data stored in a config entry gets outdated when InPost changes details
+        of a parcel locker, so it has to be refetched. The single point endpoint
+        is tried first - the full list is a few megabytes of JSON.
+        """
+        parcel_locker = await self._search_easypack24_locker(locker_code)
+
+        if parcel_locker is None:
+            return await self.search_parcel_locker(locker_code)
+
+        return from_dict(InPostAirPoint, parcel_locker)
+
     async def get_parcel_lockers_list(self) -> list[InPostAirPoint]:
         """Get parcel lockers list."""
         response = await self._request(
             method="get", url="https://inpost.pl/sites/default/files/points.json"
         )
-        response_data = from_dict(ParcelLockerListResponse, await response.json())
+        response_data = from_dict(ParcelLockerListResponse, response.json())
 
         return response_data.items
 
     async def find_parcel_locker_id(self, point: InPostAirPoint) -> str | None:
         """Find parcel locker ID by its code."""
-        response = await self._request(
-            method="get",
-            url=get_parcel_locker_url(point),
-        )
-        match = re.search(
-            r"data-shipx-url=\"/shipx-point-data/(.*?)/(.*?)/air_index_level\"",
-            await response.text(),
-        )
+        url = get_parcel_locker_url(point)
+        response = await self._request(method="get", url=url)
+
+        if response.url.rstrip("/") != url.rstrip("/"):
+            _LOGGER.warning(
+                "Page of parcel locker %s (%s) redirected to %s - data stored for that parcel locker is outdated",
+                point.n,
+                url,
+                response.url,
+            )
+            return None
+
+        match = AIR_DATA_PATTERN.search(response.body)
+
+        if match is None:
+            _LOGGER.warning(
+                "Could not find air quality data of parcel locker %s on %s",
+                point.n,
+                url,
+            )
+            return None
 
-        return None if match is None else match.group(1)
+        return match.group(1)
 
     async def get_parcel_locker_air_data(
         self, locker_code: str, locker_id: str
@@ -171,11 +221,11 @@ async def get_parcel_locker_air_data(
                 raise InPostAirApiClientSensorsMissingError(
                     "Air sensors are not available"
                 ) from e
-            raise InPostAirApiClientError("Something really wrong happened!") from e
-        except:
-            raise
+            raise InPostAirApiClientError(
+                f"Request for air data of parcel locker {locker_code} failed with status {e.status}"
+            ) from e
 
-        return from_dict(ParcelLockerAirDataResponse, await response.json())
+        return from_dict(ParcelLockerAirDataResponse, response.json())
 
 
 class InPostAirApiClientError(Exception):

custom_components/inpost_air/config_flow.py

@@ -1,7 +1,7 @@
 """Config flow for InPost Air integration."""
 
 from __future__ import annotations
-from dataclasses import dataclass
+from dataclasses import asdict, dataclass
 
 import logging
 from typing import Any
@@ -48,7 +48,13 @@ async def validate_input(hass: HomeAssistant, data: dict[str, Any]) -> InPostAir
 
     try:
         parcel_locker_id = await api_client.find_parcel_locker_id(parcel_locker)
+
+        if parcel_locker_id is None:
+            raise ParcelLockerWithoutAirData
+
         await api_client.get_parcel_locker_air_data(parcel_locker.n, parcel_locker_id)
+    except ParcelLockerWithoutAirData:
+        raise
     except Exception as exc:
         raise ParcelLockerWithoutAirData from exc
 
@@ -79,7 +85,7 @@ async def async_step_user(
             else:
                 return self.async_create_entry(
                     title=f"Parcel locker {parcel_locker.n}",
-                    data={"parcel_locker": parcel_locker},
+                    data={"parcel_locker": asdict(parcel_locker)},
                 )
 
         parcel_lockers = [

custom_components/inpost_air/coordinator.py

@@ -7,10 +7,15 @@
 import re
 
 from homeassistant.core import HomeAssistant
+from homeassistant.exceptions import ConfigEntryError
 from homeassistant.helpers.update_coordinator import DataUpdateCoordinator, UpdateFailed
 
 from .models import ParcelLocker
-from .api import InPostAirApiClientError, InPostApi
+from .api import (
+    InPostAirApiClientError,
+    InPostAirApiClientSensorsMissingError,
+    InPostApi,
+)
 from .const import Entities
 
 _LOGGER = logging.getLogger(__name__)
@@ -90,7 +95,11 @@ async def _async_update_data(self):
                 return {
                     x.name: x for line in data.air_sensors if (x := create_value(line))
                 }
+        except InPostAirApiClientSensorsMissingError as err:
+            # Nothing to poll for - retrying will not help, so let the config
+            # entry fail permanently instead of ending up in a retry loop.
+            raise ConfigEntryError(err) from err
         except InPostAirApiClientError as err:
             raise UpdateFailed(err) from err
         except Exception as err:
-            raise UpdateFailed("Error communicating with API") from err
+            raise UpdateFailed(f"Error communicating with API: {err}") from err