From 321a31d9f52cefe2ba4685e47462438be8643349 Mon Sep 17 00:00:00 2001 From: Mick Vleeshouwer Date: Mon, 22 Jun 2026 14:52:00 +0000 Subject: [PATCH 1/4] fix: retry transient connection errors on all HTTP requests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Previously only fetch_events was decorated with @retry_on_connection_failure, so a transient TimeoutError or ClientConnectorError raised from any other method — including the command path (execute_action_group) and all setup/state/refresh calls — propagated raw on the first occurrence. Centralize the retry on the _get/_post/_put/_delete helpers so every request gets uniform transient-connection retry, and drop the now redundant decorator from fetch_events. Fixes #2147 --- pyoverkiz/client.py | 5 ++++- tests/test_client.py | 52 ++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 56 insertions(+), 1 deletion(-) diff --git a/pyoverkiz/client.py b/pyoverkiz/client.py index 836761a9..35560b67 100644 --- a/pyoverkiz/client.py +++ b/pyoverkiz/client.py @@ -476,7 +476,6 @@ async def register_event_listener(self) -> str: @retry_on_concurrent_requests @retry_on_auth_error @retry_on_listener_error - @retry_on_connection_failure async def fetch_events(self) -> list[Event]: """Fetch new events from a registered event listener. Fetched events are removed. @@ -1021,6 +1020,7 @@ async def deactivate_developer_mode(self, gateway_id: str) -> None: """ await self._delete(f"setup/gateways/{gateway_id}/developerMode") + @retry_on_connection_failure async def _get(self, path: str) -> Any: """Make a GET request to the OverKiz API.""" await self._refresh_token_if_expired() @@ -1032,6 +1032,7 @@ async def _get(self, path: str) -> Any: ) as response: return await self._parse_response(response) + @retry_on_connection_failure async def _post( self, path: str, @@ -1050,6 +1051,7 @@ async def _post( ) as response: return await self._parse_response(response) + @retry_on_connection_failure async def _put(self, path: str, payload: dict[str, Any] | None = None) -> Any: """Make a PUT request to the OverKiz API.""" await self._refresh_token_if_expired() @@ -1062,6 +1064,7 @@ async def _put(self, path: str, payload: dict[str, Any] | None = None) -> Any: ) as response: return await self._parse_response(response) + @retry_on_connection_failure async def _delete(self, path: str) -> None: """Make a DELETE request to the OverKiz API.""" await self._refresh_token_if_expired() diff --git a/tests/test_client.py b/tests/test_client.py index 4956b713..ed35b75b 100644 --- a/tests/test_client.py +++ b/tests/test_client.py @@ -68,6 +68,58 @@ async def test_get_devices_basic(self, client: OverkizClient): devices = await client.get_devices() assert len(devices) == 23 + @pytest.mark.asyncio + async def test_backoff_retries_command_on_connection_failure( + self, client: OverkizClient + ) -> None: + """Ensure the command path retries transient connection failures. + + Regression test for #2147: a transient ``TimeoutError`` raised on the + execute command path must be retried instead of propagating raw on the + first occurrence. + """ + resp = MockResponse(json.dumps({"execId": "exec-1"})) + + with ( + patch("backoff._async.asyncio.sleep", new=AsyncMock()) as sleep_mock, + patch.object( + aiohttp.ClientSession, + "post", + side_effect=[TimeoutError("timed out"), resp], + ) as post_mock, + ): + exec_id = await client.execute_action_group( + actions=[Action(device_url="io://1234-5678-9012/12345678")], + ) + + assert exec_id == "exec-1" + assert post_mock.call_count == 2 + assert sleep_mock.await_count == 1 + + @pytest.mark.asyncio + async def test_backoff_retries_get_on_connection_failure( + self, client: OverkizClient + ) -> None: + """Ensure GET requests retry transient ``ClientConnectorError`` failures.""" + resp = MockResponse(json.dumps({"protocolVersion": "1"})) + connection_error = aiohttp.ClientConnectorError( + connection_key=MagicMock(), os_error=OSError("unreachable") + ) + + with ( + patch("backoff._async.asyncio.sleep", new=AsyncMock()) as sleep_mock, + patch.object( + aiohttp.ClientSession, + "get", + side_effect=[connection_error, resp], + ) as get_mock, + ): + result = await client.get_api_version() + + assert result == "1" + assert get_mock.call_count == 2 + assert sleep_mock.await_count == 1 + @pytest.mark.parametrize( ("fixture_name", "event_length"), [ From c8eac89c439fd7a4f26958445822e7362dae4add Mon Sep 17 00:00:00 2001 From: Mick Vleeshouwer Date: Mon, 22 Jun 2026 14:57:55 +0000 Subject: [PATCH 2/4] fix: lower connection-failure retry to 3 tries / 30s budget Tighten retry_on_connection_failure from 5 tries / 120s to 3 tries / 30s so a flaky connection gives up faster (~3s worst-case sleep) instead of blocking a command or poll for up to ~15s. Add a test covering the give-up-after-max-tries path. --- pyoverkiz/client.py | 4 ++-- tests/test_client.py | 23 +++++++++++++++++++++++ 2 files changed, 25 insertions(+), 2 deletions(-) diff --git a/pyoverkiz/client.py b/pyoverkiz/client.py index 35560b67..1c5fcfda 100644 --- a/pyoverkiz/client.py +++ b/pyoverkiz/client.py @@ -102,8 +102,8 @@ async def refresh_listener(invocation: Details) -> None: retry_on_connection_failure = backoff.on_exception( backoff.expo, (TimeoutError, ClientConnectorError), - max_tries=5, - max_time=120, + max_tries=3, + max_time=30, jitter=backoff.full_jitter, logger=_LOGGER, ) diff --git a/tests/test_client.py b/tests/test_client.py index ed35b75b..7d8e89e8 100644 --- a/tests/test_client.py +++ b/tests/test_client.py @@ -120,6 +120,29 @@ async def test_backoff_retries_get_on_connection_failure( assert get_mock.call_count == 2 assert sleep_mock.await_count == 1 + @pytest.mark.asyncio + async def test_backoff_gives_up_after_max_tries_on_connection_failure( + self, client: OverkizClient + ) -> None: + """Ensure connection failures are re-raised after max_tries attempts. + + ``retry_on_connection_failure`` is capped at 3 attempts (1 initial + 2 + retries); a persistent failure must surface rather than retry forever. + """ + with ( + patch("backoff._async.asyncio.sleep", new=AsyncMock()) as sleep_mock, + patch.object( + aiohttp.ClientSession, + "get", + side_effect=TimeoutError("timed out"), + ) as get_mock, + pytest.raises(TimeoutError), + ): + await client.get_api_version() + + assert get_mock.call_count == 3 + assert sleep_mock.await_count == 2 + @pytest.mark.parametrize( ("fixture_name", "event_length"), [ From ff4b5046ba9a37c54138e47bdcc9475137923d50 Mon Sep 17 00:00:00 2001 From: Mick Vleeshouwer Date: Mon, 22 Jun 2026 15:01:33 +0000 Subject: [PATCH 3/4] test: drop redundant GET retry test The give-up-after-max-tries test already exercises _get through the decorator, so the GET retry-once test added no coverage beyond the command-path (_post) regression test. --- tests/test_client.py | 24 ------------------------ 1 file changed, 24 deletions(-) diff --git a/tests/test_client.py b/tests/test_client.py index 7d8e89e8..ff1548aa 100644 --- a/tests/test_client.py +++ b/tests/test_client.py @@ -96,30 +96,6 @@ async def test_backoff_retries_command_on_connection_failure( assert post_mock.call_count == 2 assert sleep_mock.await_count == 1 - @pytest.mark.asyncio - async def test_backoff_retries_get_on_connection_failure( - self, client: OverkizClient - ) -> None: - """Ensure GET requests retry transient ``ClientConnectorError`` failures.""" - resp = MockResponse(json.dumps({"protocolVersion": "1"})) - connection_error = aiohttp.ClientConnectorError( - connection_key=MagicMock(), os_error=OSError("unreachable") - ) - - with ( - patch("backoff._async.asyncio.sleep", new=AsyncMock()) as sleep_mock, - patch.object( - aiohttp.ClientSession, - "get", - side_effect=[connection_error, resp], - ) as get_mock, - ): - result = await client.get_api_version() - - assert result == "1" - assert get_mock.call_count == 2 - assert sleep_mock.await_count == 1 - @pytest.mark.asyncio async def test_backoff_gives_up_after_max_tries_on_connection_failure( self, client: OverkizClient From 061194f94e452d2f34df2bf1c15efc13142bfd09 Mon Sep 17 00:00:00 2001 From: Mick Vleeshouwer Date: Mon, 22 Jun 2026 15:03:39 +0000 Subject: [PATCH 4/4] test: simplify connection-failure test docstrings --- tests/test_client.py | 13 ++----------- 1 file changed, 2 insertions(+), 11 deletions(-) diff --git a/tests/test_client.py b/tests/test_client.py index ff1548aa..d6556b58 100644 --- a/tests/test_client.py +++ b/tests/test_client.py @@ -72,12 +72,7 @@ async def test_get_devices_basic(self, client: OverkizClient): async def test_backoff_retries_command_on_connection_failure( self, client: OverkizClient ) -> None: - """Ensure the command path retries transient connection failures. - - Regression test for #2147: a transient ``TimeoutError`` raised on the - execute command path must be retried instead of propagating raw on the - first occurrence. - """ + """Ensure the command path retries a transient connection failure.""" resp = MockResponse(json.dumps({"execId": "exec-1"})) with ( @@ -100,11 +95,7 @@ async def test_backoff_retries_command_on_connection_failure( async def test_backoff_gives_up_after_max_tries_on_connection_failure( self, client: OverkizClient ) -> None: - """Ensure connection failures are re-raised after max_tries attempts. - - ``retry_on_connection_failure`` is capped at 3 attempts (1 initial + 2 - retries); a persistent failure must surface rather than retry forever. - """ + """Ensure a persistent connection failure is re-raised after 3 attempts.""" with ( patch("backoff._async.asyncio.sleep", new=AsyncMock()) as sleep_mock, patch.object(