Improve internet/API error handling for BMW (#90274)

* Improve internet/API error handling

* Switch to library exceptions for HTTP status errors

* Remove duplicate logging after reconnect

* Raise UpdateFailed instead of custom log handling

---------

Co-authored-by: rikroe <rikroe@users.noreply.github.com>
Co-authored-by: G Johansson <goran.johansson@shiftit.se>
This commit is contained in:
rikroe 2023-05-06 10:00:37 +02:00 committed by GitHub
parent 1af1f4db0c
commit 9a65a89aa4
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 60 additions and 28 deletions

View File

@ -6,7 +6,8 @@ from typing import Any
from bimmer_connected.api.authentication import MyBMWAuthentication
from bimmer_connected.api.regions import get_region_from_name
from httpx import HTTPError
from bimmer_connected.models import MyBMWAPIError, MyBMWAuthError
from httpx import RequestError
import voluptuous as vol
from homeassistant import config_entries, core, exceptions
@ -41,7 +42,9 @@ async def validate_input(
try:
await auth.login()
except HTTPError as ex:
except MyBMWAuthError as ex:
raise InvalidAuth from ex
except (MyBMWAPIError, RequestError) as ex:
raise CannotConnect from ex
# Return info that you want to store in the config entry.
@ -80,6 +83,8 @@ class BMWConfigFlow(config_entries.ConfigFlow, domain=DOMAIN):
}
except CannotConnect:
errors["base"] = "cannot_connect"
except InvalidAuth:
errors["base"] = "invalid_auth"
if info:
if self._reauth_entry:
@ -160,3 +165,7 @@ class BMWOptionsFlow(config_entries.OptionsFlowWithConfigEntry):
class CannotConnect(exceptions.HomeAssistantError):
"""Error to indicate we cannot connect."""
class InvalidAuth(exceptions.HomeAssistantError):
"""Error to indicate there is invalid auth."""

View File

@ -6,8 +6,8 @@ import logging
from bimmer_connected.account import MyBMWAccount
from bimmer_connected.api.regions import get_region_from_name
from bimmer_connected.models import GPSPosition
from httpx import HTTPError, HTTPStatusError, TimeoutException
from bimmer_connected.models import GPSPosition, MyBMWAPIError, MyBMWAuthError
from httpx import RequestError
from homeassistant.config_entries import ConfigEntry
from homeassistant.const import CONF_PASSWORD, CONF_REGION, CONF_USERNAME
@ -56,20 +56,12 @@ class BMWDataUpdateCoordinator(DataUpdateCoordinator[None]):
try:
await self.account.get_vehicles()
except (HTTPError, HTTPStatusError, TimeoutException) as err:
if isinstance(err, HTTPStatusError) and err.response.status_code == 429:
# Increase scan interval to not jump to not bring up the issue next time
self.update_interval = timedelta(
seconds=DEFAULT_SCAN_INTERVAL_SECONDS * 3
)
if isinstance(err, HTTPStatusError) and err.response.status_code in (
401,
403,
):
# Clear refresh token only and trigger reauth
self._update_config_entry_refresh_token(None)
raise ConfigEntryAuthFailed(str(err)) from err
raise UpdateFailed(f"Error communicating with BMW API: {err}") from err
except MyBMWAuthError as err:
# Clear refresh token and trigger reauth
self._update_config_entry_refresh_token(None)
raise ConfigEntryAuthFailed(err) from err
except (MyBMWAPIError, RequestError) as err:
raise UpdateFailed(err) from err
if self.account.refresh_token != old_refresh_token:
self._update_config_entry_refresh_token(self.account.refresh_token)
@ -79,9 +71,6 @@ class BMWDataUpdateCoordinator(DataUpdateCoordinator[None]):
self.account.refresh_token,
)
# Reset scan interval after successful update
self.update_interval = timedelta(seconds=DEFAULT_SCAN_INTERVAL_SECONDS)
def _update_config_entry_refresh_token(self, refresh_token: str | None) -> None:
"""Update or delete the refresh_token in the Config Entry."""
data = {

View File

@ -3,7 +3,8 @@ from copy import deepcopy
from unittest.mock import patch
from bimmer_connected.api.authentication import MyBMWAuthentication
from httpx import HTTPError
from bimmer_connected.models import MyBMWAPIError, MyBMWAuthError
from httpx import RequestError
from homeassistant import config_entries, data_entry_flow
from homeassistant.components.bmw_connected_drive.config_flow import DOMAIN
@ -37,15 +38,48 @@ async def test_show_form(hass: HomeAssistant) -> None:
assert result["step_id"] == "user"
async def test_connection_error(hass: HomeAssistant) -> None:
"""Test we show user form on BMW connected drive connection error."""
def _mock_get_oauth_token(*args, **kwargs):
pass
async def test_authentication_error(hass: HomeAssistant) -> None:
"""Test we show user form on MyBMW authentication error."""
with patch(
"bimmer_connected.api.authentication.MyBMWAuthentication.login",
side_effect=HTTPError("login failure"),
side_effect=MyBMWAuthError("Login failed"),
):
result = await hass.config_entries.flow.async_init(
DOMAIN,
context={"source": config_entries.SOURCE_USER},
data=FIXTURE_USER_INPUT,
)
assert result["type"] == data_entry_flow.FlowResultType.FORM
assert result["step_id"] == "user"
assert result["errors"] == {"base": "invalid_auth"}
async def test_connection_error(hass: HomeAssistant) -> None:
"""Test we show user form on MyBMW API error."""
with patch(
"bimmer_connected.api.authentication.MyBMWAuthentication.login",
side_effect=RequestError("Connection reset"),
):
result = await hass.config_entries.flow.async_init(
DOMAIN,
context={"source": config_entries.SOURCE_USER},
data=FIXTURE_USER_INPUT,
)
assert result["type"] == data_entry_flow.FlowResultType.FORM
assert result["step_id"] == "user"
assert result["errors"] == {"base": "cannot_connect"}
async def test_api_error(hass: HomeAssistant) -> None:
"""Test we show user form on general connection error."""
with patch(
"bimmer_connected.api.authentication.MyBMWAuthentication.login",
side_effect=MyBMWAPIError("400 Bad Request"),
):
result = await hass.config_entries.flow.async_init(
DOMAIN,