Skip to content

Commit

Permalink
Don't log full exception on connection timeout, fixes #47
Browse files Browse the repository at this point in the history
  • Loading branch information
CJNE committed Sep 27, 2021
1 parent d9ccba0 commit c3d9671
Show file tree
Hide file tree
Showing 6 changed files with 73 additions and 4 deletions.
1 change: 1 addition & 0 deletions .envrc
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
layout python3
5 changes: 5 additions & 0 deletions custom_components/sunspec/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
from homeassistant.helpers.update_coordinator import DataUpdateCoordinator
from homeassistant.helpers.update_coordinator import UpdateFailed

from .api import ConnectionTimeoutError
from .api import SunSpecApiClient
from .const import CONF_ENABLED_MODELS
from .const import CONF_HOST
Expand Down Expand Up @@ -128,6 +129,10 @@ async def _async_update_data(self):
data[model_id] = await self.api.async_get_data(model_id)
self.api.close()
return data
except ConnectionTimeoutError as exception:
_LOGGER.warning("SunSpec modbus timeout")
self.api.reconnect()
raise UpdateFailed() from exception
except Exception as exception:
_LOGGER.warning(exception)
self.api.reconnect()
Expand Down
17 changes: 16 additions & 1 deletion custom_components/sunspec/api.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,13 +4,23 @@

import sunspec2.modbus.client as modbus_client
from homeassistant.core import HomeAssistant
from sunspec2.modbus.client import SunSpecModbusClientException
from sunspec2.modbus.client import SunSpecModbusClientTimeout

TIMEOUT = 60


_LOGGER: logging.Logger = logging.getLogger(__package__)


class ConnectionTimeoutError(Exception):
pass


class ConnectionError(Exception):
pass


class SunSpecModelWrapper:
def __init__(self, models) -> None:
"""Sunspec model wrapper"""
Expand Down Expand Up @@ -83,7 +93,12 @@ def async_get_client(self):
return self._hass.async_add_executor_job(self.get_client)

async def async_get_data(self, model_id) -> SunSpecModelWrapper:
return await self.read(model_id)
try:
return await self.read(model_id)
except SunSpecModbusClientTimeout as timeout_error:
raise ConnectionTimeoutError() from timeout_error
except SunSpecModbusClientException as connect_error:
raise ConnectionError() from connect_error

async def read(self, model_id) -> SunSpecModelWrapper:
return await self._hass.async_add_executor_job(self.read_model, model_id)
Expand Down
13 changes: 13 additions & 0 deletions tests/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@

import pytest
import sunspec2.file.client as modbus_client
from custom_components.sunspec.api import ConnectionTimeoutError

pytest_plugins = "pytest_homeassistant_custom_component"

Expand Down Expand Up @@ -89,3 +90,15 @@ def error_get_data_fixture():
side_effect=Exception,
):
yield


# In this fixture, we are forcing calls to async_get_data to raise an Exception. This is useful
# for exception handling.
@pytest.fixture
def timeout_error_on_get_data():
"""Simulate timeout error when retrieving data from API."""
with patch(
"custom_components.sunspec.SunSpecApiClient.async_get_data",
side_effect=ConnectionTimeoutError,
):
yield
30 changes: 27 additions & 3 deletions tests/test_api.py
Original file line number Diff line number Diff line change
@@ -1,8 +1,10 @@
"""Tests for SunSpec api."""
import pytest
from custom_components.sunspec.api import (
SunSpecApiClient,
)
from custom_components.sunspec.api import ConnectionError
from custom_components.sunspec.api import ConnectionTimeoutError
from custom_components.sunspec.api import SunSpecApiClient
from sunspec2.modbus.client import SunSpecModbusClientException
from sunspec2.modbus.client import SunSpecModbusClientTimeout


async def test_api(hass, sunspec_client_mock):
Expand Down Expand Up @@ -76,3 +78,25 @@ async def test_modbus_connect_fail(hass, mocker):

with pytest.raises(Exception):
api.modbus_connect()


async def test_read_model_timeout(hass, mocker):
mocker.patch(
"custom_components.sunspec.api.SunSpecApiClient.read_model",
side_effect=SunSpecModbusClientTimeout,
)
api = SunSpecApiClient(host="test", port=123, slave_id=1, hass=hass)

with pytest.raises(ConnectionTimeoutError):
await api.async_get_data(1)


async def test_read_model_error(hass, mocker):
mocker.patch(
"custom_components.sunspec.api.SunSpecApiClient.read_model",
side_effect=SunSpecModbusClientException,
)
api = SunSpecApiClient(host="test", port=123, slave_id=1, hass=hass)

with pytest.raises(ConnectionError):
await api.async_get_data(1)
11 changes: 11 additions & 0 deletions tests/test_init.py
Original file line number Diff line number Diff line change
Expand Up @@ -55,3 +55,14 @@ async def test_setup_entry_exception(hass, error_on_get_data):
# an error.
with pytest.raises(ConfigEntryNotReady):
assert await async_setup_entry(hass, config_entry)


async def test_fetch_data_timeout(hass, timeout_error_on_get_data):
"""Test ConfigEntryNotReady when API raises an exception during entry setup."""
config_entry = MockConfigEntry(domain=DOMAIN, data=MOCK_CONFIG, entry_id="test")

# In this case we are testing the condition where async_setup_entry raises
# ConfigEntryNotReady using the `error_on_get_data` fixture which simulates
# an error.
with pytest.raises(ConfigEntryNotReady):
assert await async_setup_entry(hass, config_entry)

0 comments on commit c3d9671

Please sign in to comment.