Keep method and body when retrying a request

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)

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)
 

tests/test_generic.py

@@ -1,10 +1,13 @@
 """Test Plugwise module generic functionality."""
 
+import asyncio
 from unittest.mock import patch
 
+from plugwise.smilecomm import SmileComm
 import pytest
 
 import aiohttp
+from aiohttp import test_utils, web
 
 from .test_init import _LOGGER, TestPlugwise, pw_exceptions
 
@@ -66,3 +69,92 @@ 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 web.Response(text="<rules />")
+
+        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 = test_utils.TestServer(
+            app, port=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 web.HTTPGatewayTimeout()
+            raise web.HTTPAccepted(text="<xml />")
+
+        server = await self._start_rules_server(calls, timeout_once)
+        comm = SmileComm(
+            server.host, "pw", server.port, 10, username="smile", websession=None
+        )
+        data = "<rules><rule id='abc'></rule></rules>"
+        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="<rules />"
+                )
+        finally:
+            await comm.close_connection()
+            await server.close()
+        assert calls
+        assert all(call == ("PUT", "<rules />") 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="<rules />"
+                )
+        finally:
+            await comm.close_connection()
+            await server.close()
+        assert not calls