Skip to content

Commit

Permalink
Handle API errors more gracefully
Browse files Browse the repository at this point in the history
  • Loading branch information
masaccio committed Oct 26, 2023
1 parent f7102fd commit 5794a93
Show file tree
Hide file tree
Showing 6 changed files with 66 additions and 55 deletions.
4 changes: 4 additions & 0 deletions Changelog.md
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,10 @@
🪲 indicates bug fixes
🚀 indicates new features or improvements

## v1.6.1

🪲 Fixes error handling for Kingspan API problems [issue-9](https://github.com/masaccio/ha-kingspan-watchman-sensit/issues/9).

## v1.6.0

🚀 Configuration option added for debugging Kingspan connections. When enabled, very verbose logs are generated for the connection to the Kingspan internet service. The logs include username and password.
Expand Down
12 changes: 4 additions & 8 deletions custom_components/kingspan_watchman_sensit/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
"""
import asyncio
import logging
from asyncio import TimeoutError
from datetime import timedelta

from homeassistant.config_entries import ConfigEntry
Expand Down Expand Up @@ -55,9 +56,7 @@ async def async_setup_entry(hass: HomeAssistant, entry: ConfigEntry):
raise ConfigEntryAuthFailed("Credentials invalid") from e
except TimeoutError as e:
_LOGGER.debug("Credentials check for username '%s' timed out: %s", username, e)
raise ConfigEntryNotReady(
"Timed out while connecting to Kingspan service"
) from e
raise ConfigEntryNotReady("Timed out while connecting to Kingspan service") from e

coordinator = SENSiTDataUpdateCoordinator(
hass,
Expand All @@ -76,9 +75,7 @@ async def async_setup_entry(hass: HomeAssistant, entry: ConfigEntry):
for platform in PLATFORMS:
if entry.options.get(platform, True): # pragma: no branch
coordinator.platforms.append(platform)
hass.async_add_job(
hass.config_entries.async_forward_entry_setup(entry, platform)
)
hass.async_add_job(hass.config_entries.async_forward_entry_setup(entry, platform))

entry.add_update_listener(async_reload_entry)
return True
Expand Down Expand Up @@ -107,8 +104,7 @@ async def update(self):
"""Update data via API."""
try:
return await self.api.async_get_data()
except Exception as e:
_LOGGER.debug("API update failed: %s", e)
except APIError as e:
raise UpdateFailed() from e


Expand Down
15 changes: 10 additions & 5 deletions custom_components/kingspan_watchman_sensit/api.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
"""Sample API Client."""
import logging
from asyncio import TimeoutError
from datetime import datetime, timedelta, timezone

from async_timeout import timeout
Expand Down Expand Up @@ -41,11 +40,17 @@ async def async_get_data(self) -> dict:
async with timeout(API_TIMEOUT):
return await self._get_tank_data()
except APIError as e:
_LOGGER.error("API error logging in as %s: %s", self._username, str(e))
except TimeoutError:
_LOGGER.error("Timeout error logging in as %s", self._username)
msg = f"API error logging in as {self._username}: {e}"
_LOGGER.error(msg)
raise APIError(msg) from e
except TimeoutError as e:
msg = f"Timeout error logging in as {self._username}: {e}"
_LOGGER.error(msg)
raise APIError(msg) from e
except Exception as e: # pylint: disable=broad-except
_LOGGER.error("Unhandled error logging in as %s: %s", self._username, e)
msg = f"Unhandled error logging in as {self._username}: {e}"
_LOGGER.error(msg)
raise APIError(msg) from e

async def check_credentials(self) -> bool:
"""Login to check credentials"""
Expand Down
26 changes: 7 additions & 19 deletions custom_components/kingspan_watchman_sensit/config_flow.py
Original file line number Diff line number Diff line change
Expand Up @@ -44,16 +44,10 @@ async def async_step_user(self, user_input=None):
user_input[CONF_USERNAME], user_input[CONF_PASSWORD]
)
if valid:
_LOGGER.debug(
"login succeeded for username '%s'", user_input[CONF_USERNAME]
)
return self.async_create_entry(
title=user_input[CONF_USERNAME], data=user_input
)
_LOGGER.debug("login succeeded for username '%s'", user_input[CONF_USERNAME])
return self.async_create_entry(title=user_input[CONF_USERNAME], data=user_input)
else:
_LOGGER.debug(
"login failed for username '%s'", user_input[CONF_USERNAME]
)
_LOGGER.debug("login failed for username '%s'", user_input[CONF_USERNAME])
self._errors["base"] = "auth"

return await self._show_config_form(user_input)
Expand All @@ -64,9 +58,7 @@ async def async_step_reauth(self, user_input: Dict[str, str] = None) -> FlowResu
"""Perform reauth upon an API authentication error."""
return await self.async_step_reauth_confirm()

async def async_step_reauth_confirm(
self, user_input: Dict[str, str] = None
) -> FlowResult:
async def async_step_reauth_confirm(self, user_input: Dict[str, str] = None) -> FlowResult:
if user_input is None:
return self.async_show_form(step_id="reauth_confirm")
return await self.async_step_user()
Expand Down Expand Up @@ -95,7 +87,7 @@ async def _test_credentials(self, username, password):
"""Return true if credentials is valid."""
try:
client = SENSiTApiClient(username, password)
await client.async_get_data()
_ = await client.async_get_data()
return True
except Exception: # pylint: disable=broad-except
pass
Expand All @@ -119,9 +111,7 @@ async def async_step_init(self, user_input: dict | None = None) -> FlowResult:
"Config options: %s",
", ".join([f"{k}='{v}'" for k, v in user_input.items()]),
)
return self.async_create_entry(
title=self.config_entry.title, data=user_input
)
return self.async_create_entry(title=self.config_entry.title, data=user_input)

options = {
vol.Optional(
Expand All @@ -132,9 +122,7 @@ async def async_step_init(self, user_input: dict | None = None) -> FlowResult:
): cv.positive_int,
vol.Optional(
CONF_USAGE_WINDOW,
default=self.config_entry.options.get(
CONF_USAGE_WINDOW, DEFAULT_USAGE_WINDOW
),
default=self.config_entry.options.get(CONF_USAGE_WINDOW, DEFAULT_USAGE_WINDOW),
): cv.positive_int,
vol.Optional(
CONF_KINGSPAN_DEBUG,
Expand Down
26 changes: 6 additions & 20 deletions tests/test_api.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
"""Tests for Kingspan Watchman SENSiT api."""
import asyncio
import pytest
from datetime import datetime, timezone

import pandas as pd
Expand All @@ -16,7 +17,7 @@
)


async def test_api(hass, mock_sensor_client, mocker, caplog):
async def test_api(hass, mock_sensor_client, mocker):
"""Test API calls."""
api = SENSiTApiClient("test", "test", 14)
tank_data = await api.async_get_data()
Expand All @@ -27,29 +28,14 @@ async def test_api(hass, mock_sensor_client, mocker, caplog):
assert tank_data[0].capacity == MOCK_TANK_CAPACITY
history = pd.DataFrame(tank_data[0].history)
local_tzinfo = datetime.now(timezone.utc).astimezone().tzinfo
assert tank_data[0].last_read == history.iloc[-1].reading_date.replace(
tzinfo=local_tzinfo
)
assert tank_data[0].last_read == history.iloc[-1].reading_date.replace(tzinfo=local_tzinfo)
assert round(tank_data[0].usage_rate, 2) == 96.67
assert tank_data[0].forecast_empty == 10

caplog.clear()
mocker.patch(MOCK_GET_DATA_METHOD, side_effect=asyncio.TimeoutError)
_ = await api.async_get_data()
assert len(caplog.record_tuples) == 1
assert "Timeout error logging in" in caplog.record_tuples[0][2]

caplog.clear()
mocker.patch(MOCK_GET_DATA_METHOD, side_effect=APIError("api-test error"))
_ = await api.async_get_data()
assert len(caplog.record_tuples) == 1
assert "API error logging in as test: api-test" in caplog.record_tuples[0][2]

caplog.clear()
mocker.patch(MOCK_GET_DATA_METHOD, side_effect=Exception())
_ = await api.async_get_data()
assert len(caplog.record_tuples) == 1
assert "Unhandled error" in caplog.record_tuples[0][2]
with pytest.raises(APIError) as e:
_ = await api.async_get_data()
assert "Timeout error logging in" in str(e)


async def test_api_filtering(hass, mock_sensor_client, mocker):
Expand Down
38 changes: 35 additions & 3 deletions tests/test_sensor.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,17 +2,22 @@
from datetime import datetime, timezone

import pytest
import asyncio
from custom_components.kingspan_watchman_sensit import async_unload_entry
from custom_components.kingspan_watchman_sensit.const import DOMAIN
from homeassistant.const import ATTR_ICON
from homeassistant.helpers import device_registry as dr
from pytest_homeassistant_custom_component.common import MockConfigEntry
from homeassistant.helpers.update_coordinator import UpdateFailed
from unittest.mock import patch, AsyncMock
from connectsensor import APIError

from .const import (
MOCK_CONFIG,
MOCK_TANK_CAPACITY,
MOCK_TANK_LEVEL,
MOCK_TANK_NAME,
MOCK_GET_DATA_METHOD,
HistoryType,
)

Expand Down Expand Up @@ -59,6 +64,35 @@ async def test_sensor(hass, mock_sensor_client):
assert await async_unload_entry(hass, config_entry)


async def test_sensor_exceptions(hass, mock_sensor_client, mocker, caplog):
config_entry = MockConfigEntry(domain=DOMAIN, data=MOCK_CONFIG)

config_entry.add_to_hass(hass)
await hass.config_entries.async_setup(config_entry.entry_id)
await hass.async_block_till_done()

caplog.clear()
mocker.patch(MOCK_GET_DATA_METHOD, side_effect=asyncio.TimeoutError)
with pytest.raises(UpdateFailed) as e:
await hass.data[DOMAIN][config_entry.entry_id].update()
assert len(caplog.record_tuples) == 1
assert "Timeout error logging in" in caplog.record_tuples[0][2]

caplog.clear()
mocker.patch(MOCK_GET_DATA_METHOD, side_effect=APIError("api-test error"))
with pytest.raises(UpdateFailed) as e:
await hass.data[DOMAIN][config_entry.entry_id].update()
assert len(caplog.record_tuples) == 1
assert "API error logging in as test@example.com" in caplog.record_tuples[0][2]

caplog.clear()
mocker.patch(MOCK_GET_DATA_METHOD, side_effect=Exception())
with pytest.raises(UpdateFailed) as e:
await hass.data[DOMAIN][config_entry.entry_id].update()
assert len(caplog.record_tuples) == 1
assert "Unhandled error" in caplog.record_tuples[0][2]


@pytest.mark.parametrize(
"mock_sensor_client", [[MOCK_TANK_LEVEL, HistoryType.DECREASING, 2]], indirect=True
)
Expand Down Expand Up @@ -111,9 +145,7 @@ async def test_sensor_icon_empty(hass, mock_sensor_client):
assert await async_unload_entry(hass, config_entry)


@pytest.mark.parametrize(
"mock_sensor_client", [[MOCK_TANK_CAPACITY * 0.3]], indirect=True
)
@pytest.mark.parametrize("mock_sensor_client", [[MOCK_TANK_CAPACITY * 0.3]], indirect=True)
async def test_sensor_icon_low(hass, mock_sensor_client):
"""Test sensor."""
config_entry = MockConfigEntry(domain=DOMAIN, data=MOCK_CONFIG)
Expand Down

0 comments on commit 5794a93

Please sign in to comment.