diff --git a/homeassistant/components/fitbit/__init__.py b/homeassistant/components/fitbit/__init__.py index 522754de5d6..acf3014fb33 100644 --- a/homeassistant/components/fitbit/__init__.py +++ b/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: - raise ConfigEntryAuthFailed from err - raise ConfigEntryNotReady from err - except aiohttp.ClientError as err: + except FitbitAuthException as err: + raise ConfigEntryAuthFailed from err + except FitbitApiException as err: raise ConfigEntryNotReady from err hass.data[DOMAIN][entry.entry_id] = fitbit_api diff --git a/homeassistant/components/fitbit/api.py b/homeassistant/components/fitbit/api.py index 9ebfbcf7188..dab64724e1c 100644 --- a/homeassistant/components/fitbit/api.py +++ b/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.""" diff --git a/homeassistant/components/fitbit/application_credentials.py b/homeassistant/components/fitbit/application_credentials.py index 95a7cf799bf..e66b9ca9014 100644 --- a/homeassistant/components/fitbit/application_credentials.py +++ b/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 = { - "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()) + return await self._post( + { + "grant_type": "authorization_code", + "code": external_data["code"], + "redirect_uri": external_data["state"]["redirect_uri"], + } + ) async def _token_request(self, data: dict) -> dict: """Make a token request.""" + return await self._post( + { + **data, + CONF_CLIENT_ID: self.client_id, + CONF_CLIENT_SECRET: self.client_secret, + } + ) + + async def _post(self, data: dict[str, Any]) -> dict[str, Any]: session = async_get_clientsession(self.hass) - body = { - **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) - resp.raise_for_status() + 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 diff --git a/homeassistant/components/fitbit/config_flow.py b/homeassistant/components/fitbit/config_flow.py index ff9cf6cd17c..ee2340e7587 100644 --- a/homeassistant/components/fitbit/config_flow.py +++ b/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") diff --git a/homeassistant/components/fitbit/exceptions.py b/homeassistant/components/fitbit/exceptions.py new file mode 100644 index 00000000000..82ac53d5f99 --- /dev/null +++ b/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.""" diff --git a/homeassistant/components/fitbit/sensor.py b/homeassistant/components/fitbit/sensor.py index 313106a0d0f..c7c5e3258ed 100644 --- a/homeassistant/components/fitbit/sensor.py +++ b/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 - registered_devs: list[FitbitDevice] = await self.api.async_get_devices() - self.device = next( - device for device in registered_devs if device.id == device_id - ) - self._attr_native_value = self.device.battery + 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) diff --git a/tests/components/fitbit/test_config_flow.py b/tests/components/fitbit/test_config_flow.py index df4bae89b47..e6ab39aff59 100644 --- a/tests/components/fitbit/test_config_flow.py +++ b/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( diff --git a/tests/components/fitbit/test_sensor.py b/tests/components/fitbit/test_sensor.py index 9e2089b959c..2eb29db43de 100644 --- a/tests/components/fitbit/test_sensor.py +++ b/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"