From 88a45f3d8425a0f2b597f59c36112c5a0260a5b9 Mon Sep 17 00:00:00 2001 From: Mart van Male Date: Sun, 25 Oct 2026 11:11:06 +0100 Subject: [PATCH 1/3] Keep method and body when retrying a request A request that hit a timeout or a 504 was retried without its method and data, so a PUT was resent as a GET. The change was not resent, and the gateway's XML answer to that GET was taken as success. The existing timeout tests passed only because the mock answered the stray GET with 405. --- CHANGELOG.md | 4 ++ plugwise/smilecomm.py | 4 +- tests/test_generic.py | 90 +++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 96 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 17ddaacda..215e2e8a7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,9 @@ # Changelog +## Ongoing + +- Fix setting changes after a timeout or a 504 error. The retry sent a read instead of the change, so the change was reported as done even when the gateway never got it. + ## v1.14.8 - Fix for issue [#933](https://github.com/plugwise/python-plugwise/issues/933) via PR [#934](https://github.com/plugwise/python-plugwise/pull/934) diff --git a/plugwise/smilecomm.py b/plugwise/smilecomm.py index e770ec7d2..471b7b7eb 100644 --- a/plugwise/smilecomm.py +++ b/plugwise/smilecomm.py @@ -97,7 +97,7 @@ async def _request( exc, ) raise ConnectionFailedError from exc - return await self._request(command, retry - 1) + return await self._request(command, retry - 1, method=method, data=data) if resp.status == 504: if retry < 1: @@ -108,7 +108,7 @@ async def _request( "504 Gateway Timeout", ) raise ConnectionFailedError - return await self._request(command, retry - 1) + return await self._request(command, retry - 1, method=method, data=data) return await self._request_validate(resp, method) diff --git a/tests/test_generic.py b/tests/test_generic.py index 4a6af75c0..f69488d8b 100644 --- a/tests/test_generic.py +++ b/tests/test_generic.py @@ -1,7 +1,9 @@ """Test Plugwise module generic functionality.""" +import asyncio from unittest.mock import patch +from plugwise.smilecomm import SmileComm import pytest import aiohttp @@ -66,3 +68,91 @@ async def test_connect_timeout(self, timeout_test): except pw_exceptions.PlugwiseException: setup_result = True assert setup_result + + @staticmethod + async def _start_rules_server(calls, put_handler): + """Start a mock Smile that records the method and body of each request.""" + + async def put(request): + calls.append(("PUT", await request.text())) + return await put_handler(request, calls) + + async def get(request): + calls.append(("GET", await request.text())) + return aiohttp.web.Response(text="") + + app = aiohttp.web.Application() + if put_handler is not None: + app.router.add_route("PUT", "/core/rules{tail:.*}", put) + app.router.add_route("GET", "/core/rules{tail:.*}", get) + server = aiohttp.test_utils.TestServer( + app, port=aiohttp.test_utils.unused_port(), scheme="http", host="127.0.0.1" + ) + await server.start_server() + return server + + @pytest.mark.asyncio + async def test_put_retried_as_put_after_504(self): + """Test that a retried PUT keeps its method and body.""" + calls = [] + + async def timeout_once(request, calls): + if len(calls) == 1: + raise aiohttp.web.HTTPGatewayTimeout() + raise aiohttp.web.HTTPAccepted(text="") + + server = await self._start_rules_server(calls, timeout_once) + comm = SmileComm( + server.host, "pw", server.port, 10, username="smile", websession=None + ) + data = "" + try: + # pylint: disable-next=protected-access + await comm._request("/core/rules;id=abc", method="put", data=data) + finally: + await comm.close_connection() + await server.close() + assert calls == [("PUT", data), ("PUT", data)] + + @pytest.mark.asyncio + async def test_put_dropped_raises(self): + """Test that a PUT failing on every retry raises instead of turning into a GET.""" + calls = [] + + async def drop_connection(request, calls): + request.transport.close() + await asyncio.sleep(0.1) + + server = await self._start_rules_server(calls, drop_connection) + comm = SmileComm( + server.host, "pw", server.port, 10, username="smile", websession=None + ) + try: + with pytest.raises(pw_exceptions.ConnectionFailedError): + # pylint: disable-next=protected-access + await comm._request( + "/core/rules;id=abc", method="put", data="" + ) + finally: + await comm.close_connection() + await server.close() + assert calls and all(call == ("PUT", "") for call in calls) + + @pytest.mark.asyncio + async def test_put_not_allowed_raises(self): + """Test that a 405 on PUT raises.""" + calls = [] + server = await self._start_rules_server(calls, None) + comm = SmileComm( + server.host, "pw", server.port, 10, username="smile", websession=None + ) + try: + with pytest.raises(pw_exceptions.ConnectionFailedError): + # pylint: disable-next=protected-access + await comm._request( + "/core/rules;id=abc", method="put", data="" + ) + finally: + await comm.close_connection() + await server.close() + assert not calls From e7b284cbab4668667477d6d897d459b84538c002 Mon Sep 17 00:00:00 2001 From: Mart van Male Date: Sun, 25 Oct 2026 13:35:58 +0100 Subject: [PATCH 2/3] Import the aiohttp submodules the retry tests use --- tests/test_generic.py | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/tests/test_generic.py b/tests/test_generic.py index f69488d8b..516bb891a 100644 --- a/tests/test_generic.py +++ b/tests/test_generic.py @@ -7,6 +7,7 @@ import pytest import aiohttp +from aiohttp import test_utils, web from .test_init import _LOGGER, TestPlugwise, pw_exceptions @@ -79,14 +80,14 @@ async def put(request): async def get(request): calls.append(("GET", await request.text())) - return aiohttp.web.Response(text="") + return web.Response(text="") - app = aiohttp.web.Application() + app = web.Application() if put_handler is not None: app.router.add_route("PUT", "/core/rules{tail:.*}", put) app.router.add_route("GET", "/core/rules{tail:.*}", get) - server = aiohttp.test_utils.TestServer( - app, port=aiohttp.test_utils.unused_port(), scheme="http", host="127.0.0.1" + server = test_utils.TestServer( + app, port=test_utils.unused_port(), scheme="http", host="127.0.0.1" ) await server.start_server() return server @@ -98,8 +99,8 @@ async def test_put_retried_as_put_after_504(self): async def timeout_once(request, calls): if len(calls) == 1: - raise aiohttp.web.HTTPGatewayTimeout() - raise aiohttp.web.HTTPAccepted(text="") + raise web.HTTPGatewayTimeout() + raise web.HTTPAccepted(text="") server = await self._start_rules_server(calls, timeout_once) comm = SmileComm( From 62f1ea2ad3c99f298019bb117c9c4802922795ed Mon Sep 17 00:00:00 2001 From: Bouwe Westerdijk <11290930+bouwew@users.noreply.github.com> Date: Sun, 25 Oct 2026 17:23:13 +0100 Subject: [PATCH 3/3] Refactor assertion for calls in test_generic.py --- tests/test_generic.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/test_generic.py b/tests/test_generic.py index 516bb891a..2cd1ad451 100644 --- a/tests/test_generic.py +++ b/tests/test_generic.py @@ -137,7 +137,8 @@ async def drop_connection(request, calls): finally: await comm.close_connection() await server.close() - assert calls and all(call == ("PUT", "") for call in calls) + assert calls + assert all(call == ("PUT", "") for call in calls) @pytest.mark.asyncio async def test_put_not_allowed_raises(self):