Repository: esphome/aioesphomeapi (language: python)

Issue:
Remove extra timeouts 
both start and finish have an extra safety.   I have audited the code and we aren’t missing any timeouts anymore. Since we control the code path end to end here we should remove the extra safeties when we have sufficient coverage 

https://github.com/esphome/aioesphomeapi/blob/cafdccaf6c3cb50171d5ef8e9277c0b87b2e7dee/aioesphomeapi/connection.py#L501


Discussion / hints:

Fix patch:
diff --git a/aioesphomeapi/connection.py b/aioesphomeapi/connection.py
index 23e12ad3..57329cb7 100644
--- a/aioesphomeapi/connection.py
+++ b/aioesphomeapi/connection.py
@@ -87,9 +87,6 @@
 # to reboot and connect to the network/WiFi.
 TCP_CONNECT_TIMEOUT = 60.0
 
-# The maximum time for the whole connect process to complete
-CONNECT_AND_SETUP_TIMEOUT = 120.0
-
 # How long to wait for an existing connection to finish being
 # setup when requesting a disconnect so we can try to disconnect
 # gracefully without closing the socket out from under the
@@ -500,11 +497,7 @@ async def start_connection(self) -> None:
         )
         self._start_connect_task = start_connect_task
         try:
-            # Allow 2 minutes for connect and setup; this is only as a last measure
-            # to protect from issues if some part of the connect process mistakenly
-            # does not have a timeout
-            async with asyncio_timeout(CONNECT_AND_SETUP_TIMEOUT):
-                await start_connect_task
+            await start_connect_task
         except (Exception, CancelledError) as ex:
             # If the task was cancelled, we need to clean up the connection
             # and raise the CancelledError as APIConnectionError
@@ -554,11 +547,7 @@ async def finish_connection(self, *, login: bool) -> None:
         )
         self._finish_connect_task = finish_connect_task
         try:
-            # Allow 2 minutes for connect and setup; this is only as a last measure
-            # to protect from issues if some part of the connect process mistakenly
-            # does not have a timeout
-            async with asyncio_timeout(CONNECT_AND_SETUP_TIMEOUT):
-                await self._finish_connect_task
+            await self._finish_connect_task
         except (Exception, CancelledError) as ex:
             # If the task was cancelled, we need to clean up the connection
             # and raise the CancelledError as APIConnectionError


Test patch:
diff --git a/tests/test_connection.py b/tests/test_connection.py
index eb96aab1..20b3154b 100644
--- a/tests/test_connection.py
+++ b/tests/test_connection.py
@@ -333,21 +333,19 @@ async def test_start_connection_times_out(
     """Test handling of start connection timing out."""
     loop = asyncio.get_event_loop()
 
-    async def _create_mock_transport_protocol(create_func, **kwargs):
+    async def _mock_socket_connect(*args, **kwargs):
         await asyncio.sleep(500)
 
-    with patch.object(
-        loop, "create_connection", side_effect=_create_mock_transport_protocol
+    with patch.object(loop, "sock_connect", side_effect=_mock_socket_connect), patch(
+        "aioesphomeapi.connection.TCP_CONNECT_TIMEOUT", 0.0
     ):
         connect_task = asyncio.create_task(connect(conn, login=False))
         await asyncio.sleep(0)
 
-    async_fire_time_changed(utcnow() + timedelta(seconds=200))
-    await asyncio.sleep(0)
+        async_fire_time_changed(utcnow() + timedelta(seconds=200))
+        await asyncio.sleep(0)
 
-    with pytest.raises(
-        APIConnectionError, match="Error while starting connection: TimeoutError"
-    ):
+    with pytest.raises(APIConnectionError, match="Timeout while connecting"):
         await connect_task
 
     async_fire_time_changed(utcnow() + timedelta(seconds=600))
@@ -444,9 +442,7 @@ def on_msg(msg):
     async_fire_time_changed(utcnow() + timedelta(seconds=200))
     await asyncio.sleep(0)
 
-    with pytest.raises(
-        APIConnectionError, match="Error while finishing connection: TimeoutError"
-    ):
+    with pytest.raises(APIConnectionError, match="Hello timed out"):
         await connect_task
 
     async_fire_time_changed(utcnow() + timedelta(seconds=600))