Update fitbit error handling (#101304)

* Update fitbit error handling

* Update exceptions to inherit HomeAssistantError and add reason code

* Revert config flow exception mapping hack

homeassistant/components/fitbit/__init__.py

@@ -1,8 +1,5 @@
 """The fitbit component."""
 
-from http import HTTPStatus
-
-import aiohttp
 
 from homeassistant.config_entries import ConfigEntry
 from homeassistant.const import Platform
@@ -12,6 +9,7 @@ from homeassistant.helpers import config_entry_oauth2_flow
 
 from . import api
 from .const import DOMAIN
+from .exceptions import FitbitApiException, FitbitAuthException
 
 PLATFORMS: list[Platform] = [Platform.SENSOR]
 
@@ -31,11 +29,9 @@ async def async_setup_entry(hass: HomeAssistant, entry: ConfigEntry) -> bool:
     )
     try:
         await fitbit_api.async_get_access_token()
-    except aiohttp.ClientResponseError as err:
-        if err.status == HTTPStatus.UNAUTHORIZED:
+    except FitbitAuthException as err:
         raise ConfigEntryAuthFailed from err
-        raise ConfigEntryNotReady from err
-    except aiohttp.ClientError as err:
+    except FitbitApiException as err:
         raise ConfigEntryNotReady from err
 
     hass.data[DOMAIN][entry.entry_id] = fitbit_api

homeassistant/components/fitbit/api.py

@@ -1,10 +1,12 @@
 """API for fitbit bound to Home Assistant OAuth."""
 
 from abc import ABC, abstractmethod
+from collections.abc import Callable
 import logging
-from typing import Any, cast
+from typing import Any, TypeVar, cast
 
 from fitbit import Fitbit
+from fitbit.exceptions import HTTPException, HTTPUnauthorized
 
 from homeassistant.const import CONF_ACCESS_TOKEN
 from homeassistant.core import HomeAssistant
@@ -12,6 +14,7 @@ from homeassistant.helpers import config_entry_oauth2_flow
 from homeassistant.util.unit_system import METRIC_SYSTEM
 
 from .const import FitbitUnitSystem
+from .exceptions import FitbitApiException, FitbitAuthException
 from .model import FitbitDevice, FitbitProfile
 
 _LOGGER = logging.getLogger(__name__)
@@ -20,6 +23,9 @@ CONF_REFRESH_TOKEN = "refresh_token"
 CONF_EXPIRES_AT = "expires_at"
 
 
+_T = TypeVar("_T")
+
+
 class FitbitApi(ABC):
     """Fitbit client library wrapper base class.
 
@@ -58,9 +64,7 @@ class FitbitApi(ABC):
         """Return the user profile from the API."""
         if self._profile is None:
             client = await self._async_get_client()
-            response: dict[str, Any] = await self._hass.async_add_executor_job(
-                client.user_profile_get
-            )
+            response: dict[str, Any] = await self._run(client.user_profile_get)
             _LOGGER.debug("user_profile_get=%s", response)
             profile = response["user"]
             self._profile = FitbitProfile(
@@ -95,9 +99,7 @@ class FitbitApi(ABC):
     async def async_get_devices(self) -> list[FitbitDevice]:
         """Return available devices."""
         client = await self._async_get_client()
-        devices: list[dict[str, str]] = await self._hass.async_add_executor_job(
-            client.get_devices
-        )
+        devices: list[dict[str, str]] = await self._run(client.get_devices)
         _LOGGER.debug("get_devices=%s", devices)
         return [
             FitbitDevice(
@@ -120,12 +122,23 @@ class FitbitApi(ABC):
         def _time_series() -> dict[str, Any]:
             return cast(dict[str, Any], client.time_series(resource_type, period="7d"))
 
-        response: dict[str, Any] = await self._hass.async_add_executor_job(_time_series)
+        response: dict[str, Any] = await self._run(_time_series)
         _LOGGER.debug("time_series(%s)=%s", resource_type, response)
         key = resource_type.replace("/", "-")
         dated_results: list[dict[str, Any]] = response[key]
         return dated_results[-1]
 
+    async def _run(self, func: Callable[[], _T]) -> _T:
+        """Run client command."""
+        try:
+            return await self._hass.async_add_executor_job(func)
+        except HTTPUnauthorized as err:
+            _LOGGER.debug("Unauthorized error from fitbit API: %s", err)
+            raise FitbitAuthException from err
+        except HTTPException as err:
+            _LOGGER.debug("Error from fitbit API: %s", err)
+            raise FitbitApiException from err
+
 
 class OAuthFitbitApi(FitbitApi):
     """Provide fitbit authentication tied to an OAuth2 based config entry."""

homeassistant/components/fitbit/application_credentials.py

@@ -5,9 +5,12 @@ details on Fitbit authorization.
 """
 
 import base64
+from http import HTTPStatus
 import logging
 from typing import Any, cast
 
+import aiohttp
+
 from homeassistant.components.application_credentials import (
     AuthImplementation,
     AuthorizationServer,
@@ -18,6 +21,7 @@ from homeassistant.helpers import config_entry_oauth2_flow
 from homeassistant.helpers.aiohttp_client import async_get_clientsession
 
 from .const import CONF_CLIENT_ID, CONF_CLIENT_SECRET, OAUTH2_AUTHORIZE, OAUTH2_TOKEN
+from .exceptions import FitbitApiException, FitbitAuthException
 
 _LOGGER = logging.getLogger(__name__)
 
@@ -31,26 +35,37 @@ class FitbitOAuth2Implementation(AuthImplementation):
 
     async def async_resolve_external_data(self, external_data: dict[str, Any]) -> dict:
         """Resolve the authorization code to tokens."""
-        session = async_get_clientsession(self.hass)
-        data = {
+        return await self._post(
+            {
                 "grant_type": "authorization_code",
                 "code": external_data["code"],
                 "redirect_uri": external_data["state"]["redirect_uri"],
             }
-        resp = await session.post(self.token_url, data=data, headers=self._headers)
-        resp.raise_for_status()
-        return cast(dict, await resp.json())
+        )
 
     async def _token_request(self, data: dict) -> dict:
         """Make a token request."""
-        session = async_get_clientsession(self.hass)
-        body = {
+        return await self._post(
+            {
                 **data,
                 CONF_CLIENT_ID: self.client_id,
                 CONF_CLIENT_SECRET: self.client_secret,
             }
-        resp = await session.post(self.token_url, data=body, headers=self._headers)
+        )
+
+    async def _post(self, data: dict[str, Any]) -> dict[str, Any]:
+        session = async_get_clientsession(self.hass)
+        try:
+            resp = await session.post(self.token_url, data=data, headers=self._headers)
             resp.raise_for_status()
+        except aiohttp.ClientResponseError as err:
+            error_body = await resp.text()
+            _LOGGER.debug("Client response error body: %s", error_body)
+            if err.status == HTTPStatus.UNAUTHORIZED:
+                raise FitbitAuthException from err
+            raise FitbitApiException from err
+        except aiohttp.ClientError as err:
+            raise FitbitApiException from err
         return cast(dict, await resp.json())
 
     @property

homeassistant/components/fitbit/config_flow.py

@@ -4,8 +4,6 @@ from collections.abc import Mapping
 import logging
 from typing import Any
 
-from fitbit.exceptions import HTTPException
-
 from homeassistant.config_entries import ConfigEntry
 from homeassistant.const import CONF_TOKEN
 from homeassistant.data_entry_flow import FlowResult
@@ -13,6 +11,7 @@ from homeassistant.helpers import config_entry_oauth2_flow
 
 from . import api
 from .const import DOMAIN, OAUTH_SCOPES
+from .exceptions import FitbitApiException, FitbitAuthException
 
 _LOGGER = logging.getLogger(__name__)
 
@@ -60,7 +59,10 @@ class OAuth2FlowHandler(
         client = api.ConfigFlowFitbitApi(self.hass, data[CONF_TOKEN])
         try:
             profile = await client.async_get_user_profile()
-        except HTTPException as err:
+        except FitbitAuthException as err:
+            _LOGGER.error("Failed to authenticate with Fitbit API: %s", err)
+            return self.async_abort(reason="invalid_access_token")
+        except FitbitApiException as err:
             _LOGGER.error("Failed to fetch user profile for Fitbit API: %s", err)
             return self.async_abort(reason="cannot_connect")
 

homeassistant/components/fitbit/exceptions.py

@@ -0,0 +1,14 @@
+"""Exceptions for fitbit API calls.
+
+These exceptions exist to provide common exceptions for the async and sync client libraries.
+"""
+
+from homeassistant.exceptions import HomeAssistantError
+
+
+class FitbitApiException(HomeAssistantError):
+    """Error talking to the fitbit API."""
+
+
+class FitbitAuthException(FitbitApiException):
+    """Authentication related error talking to the fitbit API."""

homeassistant/components/fitbit/sensor.py

@@ -59,6 +59,7 @@ from .const import (
     FITBIT_DEFAULT_RESOURCES,
     FitbitUnitSystem,
 )
+from .exceptions import FitbitApiException
 from .model import FitbitDevice
 
 _LOGGER: Final = logging.getLogger(__name__)
@@ -707,12 +708,22 @@ class FitbitSensor(SensorEntity):
         resource_type = self.entity_description.key
         if resource_type == "devices/battery" and self.device is not None:
             device_id = self.device.id
+            try:
                 registered_devs: list[FitbitDevice] = await self.api.async_get_devices()
+            except FitbitApiException:
+                self._attr_available = False
+            else:
+                self._attr_available = True
                 self.device = next(
                     device for device in registered_devs if device.id == device_id
                 )
                 self._attr_native_value = self.device.battery
+            return
 
-        else:
+        try:
             result = await self.api.async_get_latest_time_series(resource_type)
+        except FitbitApiException:
+            self._attr_available = False
+        else:
+            self._attr_available = True
             self._attr_native_value = self.entity_description.value_fn(result)

tests/components/fitbit/test_config_flow.py

@@ -2,6 +2,7 @@
 
 from collections.abc import Awaitable, Callable
 from http import HTTPStatus
+from typing import Any
 from unittest.mock import patch
 
 import pytest
@@ -88,6 +89,20 @@ async def test_full_flow(
     }
 
 
+@pytest.mark.parametrize(
+    ("http_status", "json", "error_reason"),
+    [
+        (HTTPStatus.INTERNAL_SERVER_ERROR, None, "cannot_connect"),
+        (HTTPStatus.FORBIDDEN, None, "cannot_connect"),
+        (
+            HTTPStatus.UNAUTHORIZED,
+            {
+                "errors": [{"errorType": "invalid_grant"}],
+            },
+            "invalid_access_token",
+        ),
+    ],
+)
 async def test_api_failure(
     hass: HomeAssistant,
     hass_client_no_auth: ClientSessionGenerator,
@@ -95,6 +110,9 @@ async def test_api_failure(
     current_request_with_host: None,
     requests_mock: Mocker,
     setup_credentials: None,
+    http_status: HTTPStatus,
+    json: Any,
+    error_reason: str,
 ) -> None:
     """Test a failure to fetch the profile during the setup flow."""
     result = await hass.config_entries.flow.async_init(
@@ -126,12 +144,15 @@ async def test_api_failure(
     )
 
     requests_mock.register_uri(
-        "GET", PROFILE_API_URL, status_code=HTTPStatus.INTERNAL_SERVER_ERROR
+        "GET",
+        PROFILE_API_URL,
+        status_code=http_status,
+        json=json,
     )
 
     result = await hass.config_entries.flow.async_configure(result["flow_id"])
     assert result.get("type") == FlowResultType.ABORT
-    assert result.get("reason") == "cannot_connect"
+    assert result.get("reason") == error_reason
 
 
 async def test_config_entry_already_exists(

tests/components/fitbit/test_sensor.py

@@ -2,17 +2,25 @@
 
 
 from collections.abc import Awaitable, Callable
+from http import HTTPStatus
 from typing import Any
 
 import pytest
+from requests_mock.mocker import Mocker
 from syrupy.assertion import SnapshotAssertion
 
 from homeassistant.components.fitbit.const import DOMAIN
 from homeassistant.const import Platform
 from homeassistant.core import HomeAssistant
 from homeassistant.helpers import entity_registry as er
+from homeassistant.helpers.entity_component import async_update_entity
 
-from .conftest import PROFILE_USER_ID, timeseries_response
+from .conftest import (
+    DEVICES_API_URL,
+    PROFILE_USER_ID,
+    TIMESERIES_API_URL_FORMAT,
+    timeseries_response,
+)
 
 DEVICE_RESPONSE_CHARGE_2 = {
     "battery": "Medium",
@@ -359,7 +367,6 @@ async def test_activity_scope_config_entry(
     setup_credentials: None,
     integration_setup: Callable[[], Awaitable[bool]],
     register_timeseries: Callable[[str, dict[str, Any]], None],
-    entity_registry: er.EntityRegistry,
 ) -> None:
     """Test activity sensors are enabled."""
 
@@ -404,7 +411,6 @@ async def test_heartrate_scope_config_entry(
     setup_credentials: None,
     integration_setup: Callable[[], Awaitable[bool]],
     register_timeseries: Callable[[str, dict[str, Any]], None],
-    entity_registry: er.EntityRegistry,
 ) -> None:
     """Test heartrate sensors are enabled."""
 
@@ -429,7 +435,6 @@ async def test_sleep_scope_config_entry(
     setup_credentials: None,
     integration_setup: Callable[[], Awaitable[bool]],
     register_timeseries: Callable[[str, dict[str, Any]], None],
-    entity_registry: er.EntityRegistry,
 ) -> None:
     """Test sleep sensors are enabled."""
 
@@ -471,7 +476,6 @@ async def test_weight_scope_config_entry(
     setup_credentials: None,
     integration_setup: Callable[[], Awaitable[bool]],
     register_timeseries: Callable[[str, dict[str, Any]], None],
-    entity_registry: er.EntityRegistry,
 ) -> None:
     """Test sleep sensors are enabled."""
 
@@ -493,7 +497,6 @@ async def test_settings_scope_config_entry(
     setup_credentials: None,
     integration_setup: Callable[[], Awaitable[bool]],
     register_timeseries: Callable[[str, dict[str, Any]], None],
-    entity_registry: er.EntityRegistry,
 ) -> None:
     """Test heartrate sensors are enabled."""
 
@@ -510,3 +513,82 @@ async def test_settings_scope_config_entry(
     assert [s.entity_id for s in states] == [
         "sensor.charge_2_battery",
     ]
+
+
+@pytest.mark.parametrize(
+    ("scopes"),
+    [(["heartrate"])],
+)
+async def test_sensor_update_failed(
+    hass: HomeAssistant,
+    setup_credentials: None,
+    integration_setup: Callable[[], Awaitable[bool]],
+    requests_mock: Mocker,
+) -> None:
+    """Test a failed sensor update when talking to the API."""
+
+    requests_mock.register_uri(
+        "GET",
+        TIMESERIES_API_URL_FORMAT.format(resource="activities/heart"),
+        status_code=HTTPStatus.INTERNAL_SERVER_ERROR,
+    )
+
+    assert await integration_setup()
+
+    state = hass.states.get("sensor.resting_heart_rate")
+    assert state
+    assert state.state == "unavailable"
+
+
+@pytest.mark.parametrize(
+    ("scopes", "mock_devices"),
+    [(["settings"], None)],
+)
+async def test_device_battery_level_update_failed(
+    hass: HomeAssistant,
+    setup_credentials: None,
+    integration_setup: Callable[[], Awaitable[bool]],
+    requests_mock: Mocker,
+) -> None:
+    """Test API failure for a battery level sensor for devices."""
+
+    requests_mock.register_uri(
+        "GET",
+        DEVICES_API_URL,
+        [
+            {
+                "status_code": HTTPStatus.OK,
+                "json": [DEVICE_RESPONSE_CHARGE_2],
+            },
+            # A second spurious update request on startup
+            {
+                "status_code": HTTPStatus.OK,
+                "json": [DEVICE_RESPONSE_CHARGE_2],
+            },
+            # Fail when requesting an update
+            {
+                "status_code": HTTPStatus.INTERNAL_SERVER_ERROR,
+                "json": {
+                    "errors": [
+                        {
+                            "errorType": "request",
+                            "message": "An error occurred",
+                        }
+                    ]
+                },
+            },
+        ],
+    )
+
+    assert await integration_setup()
+
+    state = hass.states.get("sensor.charge_2_battery")
+    assert state
+    assert state.state == "Medium"
+
+    # Request an update for the entity which will fail
+    await async_update_entity(hass, "sensor.charge_2_battery")
+
+    state = hass.states.get("sensor.charge_2_battery")
+    assert state
+    assert state.state == "unavailable"