Fix RecursionError in Husqvarna Automower coordinator (#123085)
* reach maximum recursion depth exceeded in tests
* second background task
* Update homeassistant/components/husqvarna_automower/coordinator.py
Co-authored-by: Martin Hjelmare <marhje52@gmail.com>
* Update homeassistant/components/husqvarna_automower/coordinator.py
Co-authored-by: Martin Hjelmare <marhje52@gmail.com>
* test
* modify test
* tests
* use correct exception
* reset mock
* use recursion_limit
* remove unneeded ticks
* test TimeoutException
* set lower recursionlimit
* remove not that important comment and move the other
* test that we connect and listen successfully
* Simulate hass shutting down
* skip testing against the recursion limit
* Update homeassistant/components/husqvarna_automower/coordinator.py
Co-authored-by: Martin Hjelmare <marhje52@gmail.com>
* mock
* Remove comment
* Revert "mock"
This reverts commit e8ddaea3d7
.
* Move patch to decorator
* Make execution of patched methods predictable
* Parametrize test, make mocked start_listening block
* Apply suggestions from code review
---------
Co-authored-by: Martin Hjelmare <marhje52@gmail.com>
Co-authored-by: Erik <erik@montnemery.com>
This commit is contained in:
parent
5cce369ce8
commit
827875473b
3 changed files with 92 additions and 27 deletions
|
@ -8,6 +8,7 @@ from aioautomower.exceptions import (
|
||||||
ApiException,
|
ApiException,
|
||||||
AuthException,
|
AuthException,
|
||||||
HusqvarnaWSServerHandshakeError,
|
HusqvarnaWSServerHandshakeError,
|
||||||
|
TimeoutException,
|
||||||
)
|
)
|
||||||
from aioautomower.model import MowerAttributes
|
from aioautomower.model import MowerAttributes
|
||||||
from aioautomower.session import AutomowerSession
|
from aioautomower.session import AutomowerSession
|
||||||
|
@ -22,6 +23,7 @@ from .const import DOMAIN
|
||||||
_LOGGER = logging.getLogger(__name__)
|
_LOGGER = logging.getLogger(__name__)
|
||||||
MAX_WS_RECONNECT_TIME = 600
|
MAX_WS_RECONNECT_TIME = 600
|
||||||
SCAN_INTERVAL = timedelta(minutes=8)
|
SCAN_INTERVAL = timedelta(minutes=8)
|
||||||
|
DEFAULT_RECONNECT_TIME = 2 # Define a default reconnect time
|
||||||
|
|
||||||
|
|
||||||
class AutomowerDataUpdateCoordinator(DataUpdateCoordinator[dict[str, MowerAttributes]]):
|
class AutomowerDataUpdateCoordinator(DataUpdateCoordinator[dict[str, MowerAttributes]]):
|
||||||
|
@ -40,8 +42,8 @@ class AutomowerDataUpdateCoordinator(DataUpdateCoordinator[dict[str, MowerAttrib
|
||||||
update_interval=SCAN_INTERVAL,
|
update_interval=SCAN_INTERVAL,
|
||||||
)
|
)
|
||||||
self.api = api
|
self.api = api
|
||||||
|
|
||||||
self.ws_connected: bool = False
|
self.ws_connected: bool = False
|
||||||
|
self.reconnect_time = DEFAULT_RECONNECT_TIME
|
||||||
|
|
||||||
async def _async_update_data(self) -> dict[str, MowerAttributes]:
|
async def _async_update_data(self) -> dict[str, MowerAttributes]:
|
||||||
"""Subscribe for websocket and poll data from the API."""
|
"""Subscribe for websocket and poll data from the API."""
|
||||||
|
@ -66,24 +68,28 @@ class AutomowerDataUpdateCoordinator(DataUpdateCoordinator[dict[str, MowerAttrib
|
||||||
hass: HomeAssistant,
|
hass: HomeAssistant,
|
||||||
entry: ConfigEntry,
|
entry: ConfigEntry,
|
||||||
automower_client: AutomowerSession,
|
automower_client: AutomowerSession,
|
||||||
reconnect_time: int = 2,
|
|
||||||
) -> None:
|
) -> None:
|
||||||
"""Listen with the client."""
|
"""Listen with the client."""
|
||||||
try:
|
try:
|
||||||
await automower_client.auth.websocket_connect()
|
await automower_client.auth.websocket_connect()
|
||||||
reconnect_time = 2
|
# Reset reconnect time after successful connection
|
||||||
|
self.reconnect_time = DEFAULT_RECONNECT_TIME
|
||||||
await automower_client.start_listening()
|
await automower_client.start_listening()
|
||||||
except HusqvarnaWSServerHandshakeError as err:
|
except HusqvarnaWSServerHandshakeError as err:
|
||||||
_LOGGER.debug(
|
_LOGGER.debug(
|
||||||
"Failed to connect to websocket. Trying to reconnect: %s", err
|
"Failed to connect to websocket. Trying to reconnect: %s",
|
||||||
|
err,
|
||||||
|
)
|
||||||
|
except TimeoutException as err:
|
||||||
|
_LOGGER.debug(
|
||||||
|
"Failed to listen to websocket. Trying to reconnect: %s",
|
||||||
|
err,
|
||||||
)
|
)
|
||||||
|
|
||||||
if not hass.is_stopping:
|
if not hass.is_stopping:
|
||||||
await asyncio.sleep(reconnect_time)
|
await asyncio.sleep(self.reconnect_time)
|
||||||
reconnect_time = min(reconnect_time * 2, MAX_WS_RECONNECT_TIME)
|
self.reconnect_time = min(self.reconnect_time * 2, MAX_WS_RECONNECT_TIME)
|
||||||
await self.client_listen(
|
entry.async_create_background_task(
|
||||||
hass=hass,
|
hass,
|
||||||
entry=entry,
|
self.client_listen(hass, entry, automower_client),
|
||||||
automower_client=automower_client,
|
"reconnect_task",
|
||||||
reconnect_time=reconnect_time,
|
|
||||||
)
|
)
|
||||||
|
|
|
@ -1,5 +1,6 @@
|
||||||
"""Test helpers for Husqvarna Automower."""
|
"""Test helpers for Husqvarna Automower."""
|
||||||
|
|
||||||
|
import asyncio
|
||||||
from collections.abc import Generator
|
from collections.abc import Generator
|
||||||
import time
|
import time
|
||||||
from unittest.mock import AsyncMock, patch
|
from unittest.mock import AsyncMock, patch
|
||||||
|
@ -101,10 +102,17 @@ async def setup_credentials(hass: HomeAssistant) -> None:
|
||||||
def mock_automower_client(values) -> Generator[AsyncMock]:
|
def mock_automower_client(values) -> Generator[AsyncMock]:
|
||||||
"""Mock a Husqvarna Automower client."""
|
"""Mock a Husqvarna Automower client."""
|
||||||
|
|
||||||
|
async def listen() -> None:
|
||||||
|
"""Mock listen."""
|
||||||
|
listen_block = asyncio.Event()
|
||||||
|
await listen_block.wait()
|
||||||
|
pytest.fail("Listen was not cancelled!")
|
||||||
|
|
||||||
mock = AsyncMock(spec=AutomowerSession)
|
mock = AsyncMock(spec=AutomowerSession)
|
||||||
mock.auth = AsyncMock(side_effect=ClientWebSocketResponse)
|
mock.auth = AsyncMock(side_effect=ClientWebSocketResponse)
|
||||||
mock.commands = AsyncMock(spec_set=_MowerCommands)
|
mock.commands = AsyncMock(spec_set=_MowerCommands)
|
||||||
mock.get_status.return_value = values
|
mock.get_status.return_value = values
|
||||||
|
mock.start_listening = AsyncMock(side_effect=listen)
|
||||||
|
|
||||||
with patch(
|
with patch(
|
||||||
"homeassistant.components.husqvarna_automower.AutomowerSession",
|
"homeassistant.components.husqvarna_automower.AutomowerSession",
|
||||||
|
|
|
@ -1,14 +1,16 @@
|
||||||
"""Tests for init module."""
|
"""Tests for init module."""
|
||||||
|
|
||||||
from datetime import datetime, timedelta
|
from asyncio import Event
|
||||||
|
from datetime import datetime
|
||||||
import http
|
import http
|
||||||
import time
|
import time
|
||||||
from unittest.mock import AsyncMock
|
from unittest.mock import AsyncMock, patch
|
||||||
|
|
||||||
from aioautomower.exceptions import (
|
from aioautomower.exceptions import (
|
||||||
ApiException,
|
ApiException,
|
||||||
AuthException,
|
AuthException,
|
||||||
HusqvarnaWSServerHandshakeError,
|
HusqvarnaWSServerHandshakeError,
|
||||||
|
TimeoutException,
|
||||||
)
|
)
|
||||||
from aioautomower.model import MowerAttributes, WorkArea
|
from aioautomower.model import MowerAttributes, WorkArea
|
||||||
from freezegun.api import FrozenDateTimeFactory
|
from freezegun.api import FrozenDateTimeFactory
|
||||||
|
@ -127,28 +129,77 @@ async def test_update_failed(
|
||||||
assert entry.state is entry_state
|
assert entry.state is entry_state
|
||||||
|
|
||||||
|
|
||||||
|
@patch(
|
||||||
|
"homeassistant.components.husqvarna_automower.coordinator.DEFAULT_RECONNECT_TIME", 0
|
||||||
|
)
|
||||||
|
@pytest.mark.parametrize(
|
||||||
|
("method_path", "exception", "error_msg"),
|
||||||
|
[
|
||||||
|
(
|
||||||
|
["auth", "websocket_connect"],
|
||||||
|
HusqvarnaWSServerHandshakeError,
|
||||||
|
"Failed to connect to websocket.",
|
||||||
|
),
|
||||||
|
(
|
||||||
|
["start_listening"],
|
||||||
|
TimeoutException,
|
||||||
|
"Failed to listen to websocket.",
|
||||||
|
),
|
||||||
|
],
|
||||||
|
)
|
||||||
async def test_websocket_not_available(
|
async def test_websocket_not_available(
|
||||||
hass: HomeAssistant,
|
hass: HomeAssistant,
|
||||||
mock_automower_client: AsyncMock,
|
mock_automower_client: AsyncMock,
|
||||||
mock_config_entry: MockConfigEntry,
|
mock_config_entry: MockConfigEntry,
|
||||||
caplog: pytest.LogCaptureFixture,
|
caplog: pytest.LogCaptureFixture,
|
||||||
freezer: FrozenDateTimeFactory,
|
freezer: FrozenDateTimeFactory,
|
||||||
|
method_path: list[str],
|
||||||
|
exception: type[Exception],
|
||||||
|
error_msg: str,
|
||||||
) -> None:
|
) -> None:
|
||||||
"""Test trying reload the websocket."""
|
"""Test trying to reload the websocket."""
|
||||||
mock_automower_client.start_listening.side_effect = HusqvarnaWSServerHandshakeError(
|
calls = []
|
||||||
"Boom"
|
mock_called = Event()
|
||||||
)
|
mock_stall = Event()
|
||||||
|
|
||||||
|
async def mock_function():
|
||||||
|
mock_called.set()
|
||||||
|
await mock_stall.wait()
|
||||||
|
# Raise the first time the method is awaited
|
||||||
|
if not calls:
|
||||||
|
calls.append(None)
|
||||||
|
raise exception("Boom")
|
||||||
|
if mock_side_effect:
|
||||||
|
await mock_side_effect()
|
||||||
|
|
||||||
|
# Find the method to mock
|
||||||
|
mock = mock_automower_client
|
||||||
|
for itm in method_path:
|
||||||
|
mock = getattr(mock, itm)
|
||||||
|
mock_side_effect = mock.side_effect
|
||||||
|
mock.side_effect = mock_function
|
||||||
|
|
||||||
|
# Setup integration and verify log error message
|
||||||
await setup_integration(hass, mock_config_entry)
|
await setup_integration(hass, mock_config_entry)
|
||||||
assert "Failed to connect to websocket. Trying to reconnect: Boom" in caplog.text
|
await mock_called.wait()
|
||||||
assert mock_automower_client.auth.websocket_connect.call_count == 1
|
mock_called.clear()
|
||||||
assert mock_automower_client.start_listening.call_count == 1
|
# Allow the exception to be raised
|
||||||
assert mock_config_entry.state is ConfigEntryState.LOADED
|
mock_stall.set()
|
||||||
freezer.tick(timedelta(seconds=2))
|
assert mock.call_count == 1
|
||||||
async_fire_time_changed(hass)
|
|
||||||
await hass.async_block_till_done()
|
await hass.async_block_till_done()
|
||||||
assert mock_automower_client.auth.websocket_connect.call_count == 2
|
assert f"{error_msg} Trying to reconnect: Boom" in caplog.text
|
||||||
assert mock_automower_client.start_listening.call_count == 2
|
|
||||||
assert mock_config_entry.state is ConfigEntryState.LOADED
|
# Simulate a successful connection
|
||||||
|
caplog.clear()
|
||||||
|
await mock_called.wait()
|
||||||
|
mock_called.clear()
|
||||||
|
await hass.async_block_till_done()
|
||||||
|
assert mock.call_count == 2
|
||||||
|
assert "Trying to reconnect: Boom" not in caplog.text
|
||||||
|
|
||||||
|
# Simulate hass shutting down
|
||||||
|
await hass.async_stop()
|
||||||
|
assert mock.call_count == 2
|
||||||
|
|
||||||
|
|
||||||
async def test_device_info(
|
async def test_device_info(
|
||||||
|
|
Loading…
Add table
Reference in a new issue