From 211e7cf9f60ce7b46416dc1fc5d3bb2075053788 Mon Sep 17 00:00:00 2001
From: aggieNick02 <nick@pcpartpicker.com>
Date: Fri, 27 Sep 2024 14:53:58 -0500
Subject: [PATCH] Fix cases in Session.request method where an exception could
 lead to exiting the method without the created socket being owned by a
 response or closed.  This would then cause future calls to get a socket with
 the same parameters to fail because one already existed and hadn't been
 closed.  The PR
 https://github.com/adafruit/Adafruit_CircuitPython_Requests/pull/72 fixed
 this issue for one case where an exception could be raised. This generalizes
 that in an attempt to fix it in other cases that could still be hit.

---
 adafruit_requests.py | 30 ++++++++++++++++++++----------
 1 file changed, 20 insertions(+), 10 deletions(-)

diff --git a/adafruit_requests.py b/adafruit_requests.py
index f0c9e65..427b9e1 100644
--- a/adafruit_requests.py
+++ b/adafruit_requests.py
@@ -622,6 +622,10 @@ def request(  # noqa: PLR0912,PLR0913,PLR0915 Too many branches,Too many argumen
 
         # We may fail to send the request if the socket we got is closed already. So, try a second
         # time in that case.
+        # Note that the loop below actually tries a second time in other failure cases too,
+        # namely timeout and no data from socket. This was not covered in the stated intent of the
+        # commit that introduced the loop, but removing the retry from those cases could prove
+        # problematic to callers that now depend on that resiliency.
         retry_count = 0
         last_exc = None
         while retry_count < 2:
@@ -643,17 +647,23 @@ def request(  # noqa: PLR0912,PLR0913,PLR0915 Too many branches,Too many argumen
             if ok:
                 # Read the H of "HTTP/1.1" to make sure the socket is alive. send can appear to work
                 # even when the socket is closed.
-                if hasattr(socket, "recv"):
-                    result = socket.recv(1)
-                else:
-                    result = bytearray(1)
-                    try:
+                # Both recv/recv_into can raise OSError; when that happens, we need to call
+                # _connection_manager.close_socket(socket) or future calls to _connection_manager.get_socket()
+                # for the same parameter set will fail
+                try:
+                    if hasattr(socket, "recv"):
+                        result = socket.recv(1)
+                    else:
+                        result = bytearray(1)
                         socket.recv_into(result)
-                    except OSError:
-                        pass
-                if result == b"H":
-                    # Things seem to be ok so break with socket set.
-                    break
+                    if result == b"H":
+                        # Things seem to be ok so break with socket set.
+                        break
+                    else:
+                        raise RuntimeError("no data from socket")
+                except (OSError, RuntimeError) as exc:
+                    last_exc = exc
+                    pass
             self._connection_manager.close_socket(socket)
             socket = None