Skip to content

Commit

Permalink
Update fitbit error handling (#101304)
Browse files Browse the repository at this point in the history
* Update fitbit error handling

* Update exceptions to inherit HomeAssistantError and add reason code

* Revert config flow exception mapping hack
  • Loading branch information
allenporter committed Oct 6, 2023
1 parent 1d31def commit c7d533d
Show file tree
Hide file tree
Showing 8 changed files with 203 additions and 49 deletions.
12 changes: 4 additions & 8 deletions homeassistant/components/fitbit/__init__.py
Original file line number Diff line number Diff line change
@@ -1,8 +1,5 @@
"""The fitbit component."""

from http import HTTPStatus

import aiohttp

from homeassistant.config_entries import ConfigEntry
from homeassistant.const import Platform
Expand All @@ -12,6 +9,7 @@

from . import api
from .const import DOMAIN
from .exceptions import FitbitApiException, FitbitAuthException

PLATFORMS: list[Platform] = [Platform.SENSOR]

Expand All @@ -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
Expand Down
29 changes: 21 additions & 8 deletions homeassistant/components/fitbit/api.py
Original file line number Diff line number Diff line change
@@ -1,17 +1,20 @@
"""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
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__)
Expand All @@ -20,6 +23,9 @@
CONF_EXPIRES_AT = "expires_at"


_T = TypeVar("_T")


class FitbitApi(ABC):
"""Fitbit client library wrapper base class.
Expand Down Expand Up @@ -58,9 +64,7 @@ async def async_get_user_profile(self) -> FitbitProfile:
"""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(
Expand Down Expand Up @@ -95,9 +99,7 @@ async def async_get_unit_system(self) -> FitbitUnitSystem:
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(
Expand All @@ -120,12 +122,23 @@ async def async_get_latest_time_series(self, resource_type: str) -> dict[str, An
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."""
Expand Down
47 changes: 31 additions & 16 deletions homeassistant/components/fitbit/application_credentials.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,12 @@
"""

import base64
from http import HTTPStatus
import logging
from typing import Any, cast

import aiohttp

from homeassistant.components.application_credentials import (
AuthImplementation,
AuthorizationServer,
Expand All @@ -18,6 +21,7 @@
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__)

Expand All @@ -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
Expand Down
8 changes: 5 additions & 3 deletions homeassistant/components/fitbit/config_flow.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,15 +4,14 @@
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
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__)

Expand Down Expand Up @@ -60,7 +59,10 @@ async def async_oauth_create_entry(self, data: dict[str, Any]) -> FlowResult:
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")

Expand Down
14 changes: 14 additions & 0 deletions homeassistant/components/fitbit/exceptions.py
Original file line number Diff line number Diff line change
@@ -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."""
23 changes: 17 additions & 6 deletions homeassistant/components/fitbit/sensor.py
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@
FITBIT_DEFAULT_RESOURCES,
FitbitUnitSystem,
)
from .exceptions import FitbitApiException
from .model import FitbitDevice

_LOGGER: Final = logging.getLogger(__name__)
Expand Down Expand Up @@ -707,12 +708,22 @@ async def async_update(self) -> None:
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)
25 changes: 23 additions & 2 deletions tests/components/fitbit/test_config_flow.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -88,13 +89,30 @@ 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,
aioclient_mock: AiohttpClientMocker,
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(
Expand Down Expand Up @@ -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(
Expand Down
Loading

0 comments on commit c7d533d

Please sign in to comment.