From efa4db52432726853c282f177e24f06cb30d795d Mon Sep 17 00:00:00 2001 From: Neil Sleightholm Date: Mon, 28 Sep 2026 09:22:26 +0100 Subject: [PATCH] fix: return False on set_state timeout instead of raising _execute_state_change is documented to return False on failure, but a Hive API timeout propagated as asyncio.TimeoutError, and a failed request (for which _call_endpoint returns {}) raised KeyError on resp["original"]. In Home Assistant this aborts the whole multi-entity service call (e.g. climate.set_hvac_mode on several TRVs) and bypasses continue_on_error, since neither is a HomeAssistantError. Catch the timeout and read the status with .get() so both cases log and return False as documented. Polling's own timeout handling is unchanged. Co-Authored-By: Claude Opus 5.5 --- src/helper/device_handler_base.py | 19 ++++++++++++++----- tests/unit/test_base_handler.py | 19 ++++++++++++++----- 2 files changed, 28 insertions(+), 10 deletions(-) diff --git a/src/helper/device_handler_base.py b/src/helper/device_handler_base.py index 97a30184..7c674ed6 100644 --- a/src/helper/device_handler_base.py +++ b/src/helper/device_handler_base.py @@ -1,5 +1,6 @@ """Shared base class for all Hive device handlers.""" +import asyncio import logging from typing import Any @@ -39,16 +40,24 @@ async def _execute_state_change(self, device: Device, **state_kwargs) -> bool: return False await self.session.hive_refresh_tokens() data = self.session.data.products[device.hive_id] - resp = await self.session.api.set_state( - data["type"], device.hive_id, **state_kwargs - ) - if resp["original"] == HTTP_OK: + try: + resp = await self.session.api.set_state( + data["type"], device.hive_id, **state_kwargs + ) + except asyncio.TimeoutError: + # Honour the documented False-on-failure contract so one slow + # device doesn't abort a multi-entity service call. + _LOGGER.warning( + "_execute_state_change - set_state timed out for %s", device.ha_name + ) + return False + if resp.get("original") == HTTP_OK: await self.session.get_devices(device.hive_id) return True _LOGGER.error( "_execute_state_change - set_state failed for %s: HTTP %s", device.ha_name, - resp["original"], + resp.get("original"), ) return False diff --git a/tests/unit/test_base_handler.py b/tests/unit/test_base_handler.py index 57200358..fcb6de92 100644 --- a/tests/unit/test_base_handler.py +++ b/tests/unit/test_base_handler.py @@ -2,9 +2,9 @@ # pylint: disable=protected-access,too-few-public-methods,attribute-defined-outside-init +import asyncio from unittest.mock import AsyncMock, MagicMock -import pytest from apyhiveapi.helper.device_handler_base import BaseDeviceHandler from apyhiveapi.helper.hivedataclasses import Device from apyhiveapi.helper.map import Map @@ -167,11 +167,20 @@ async def test_non_200_returns_false(self): assert result is False session.get_devices.assert_not_called() - async def test_malformed_set_state_response_raises_key_error(self): - """KeyError propagates when set_state response is missing 'original' key.""" + async def test_malformed_set_state_response_returns_false(self): + """Returns False when set_state response is missing 'original' key.""" session = _make_session({"prod-1": {"type": "heating"}}) session.api.set_state = AsyncMock(return_value={"parsed": {}}) h = _make_handler(session) d = _make_device() - with pytest.raises(KeyError): - await h._execute_state_change(d, mode="MANUAL") + assert await h._execute_state_change(d, mode="MANUAL") is False + session.get_devices.assert_not_called() + + async def test_set_state_timeout_returns_false(self): + """Returns False instead of raising when set_state times out.""" + session = _make_session({"prod-1": {"type": "heating"}}) + session.api.set_state = AsyncMock(side_effect=asyncio.TimeoutError) + h = _make_handler(session) + d = _make_device() + assert await h._execute_state_change(d, mode="MANUAL") is False + session.get_devices.assert_not_called()