fix: honor custom timeout on every attempt in request_with_retry (#11998)

Co-authored-by: Sebastian Husch Lee <[email protected]>

haystack/utils/requests_utils.py

@@ -61,6 +61,9 @@ def request_with_retry(
     if status_codes_to_retry is None:
         status_codes_to_retry = [408, 418, 429, 503]
 
+    # Pop `timeout` once, before the retry loop.
+    timeout = kwargs.pop("timeout", 10)
+
     @retry(
         reraise=True,
         wait=wait_exponential(),
@@ -70,7 +73,6 @@ def request_with_retry(
         after=after_log(logger, logging.DEBUG),
     )
     def run() -> httpx.Response:
-        timeout = kwargs.pop("timeout", 10)
         with httpx.Client() as client:
             res = client.request(**kwargs, timeout=timeout)
 
@@ -171,6 +173,9 @@ async def async_request_with_retry(
     if status_codes_to_retry is None:
         status_codes_to_retry = [408, 418, 429, 503]
 
+    # Pop `timeout` once, before the retry loop.
+    timeout = kwargs.pop("timeout", 10)
+
     @retry(
         reraise=True,
         wait=wait_exponential(),
@@ -180,7 +185,6 @@ async def async_request_with_retry(
         after=after_log(logger, logging.DEBUG),
     )
     async def run() -> httpx.Response:
-        timeout = kwargs.pop("timeout", 10)
         async with httpx.AsyncClient() as client:
             res = await client.request(**kwargs, timeout=timeout)
 

releasenotes/notes/fix-request-with-retry-timeout-on-retry-2e1437ea8cbba7ad.yaml

@@ -0,0 +1,6 @@
+---
+fixes:
+  - |
+    ``request_with_retry`` and ``async_request_with_retry`` now honor a custom ``timeout`` on every
+    retry attempt. Previously the ``timeout`` was consumed on the first attempt and any subsequent
+    retry silently fell back to the default of 10 seconds.

test/utils/test_requests_utils.py

@@ -121,6 +121,30 @@ class TestRequestWithRetry:
                 assert mock_request.call_count == 2
                 mock_sleep.assert_called()
 
+    def test_request_with_retry_custom_timeout_preserved_across_retries(self):
+        """A custom timeout must be honored on every attempt, not just the first one.
+
+        Regression test: ``timeout`` used to be popped from the shared ``kwargs`` inside the
+        retried inner function, so after the first attempt it was gone and every subsequent
+        retry silently fell back to the default of 10 seconds.
+        """
+        with patch("time.sleep", return_value=None):
+            success_response = httpx.Response(status_code=200, request=httpx.Request("GET", "https://example.com"))
+
+            with patch("httpx.Client.request") as mock_request:
+                # First attempt fails with a retryable error, second attempt succeeds.
+                mock_request.side_effect = [
+                    httpx.RequestError("boom", request=httpx.Request("GET", "https://example.com")),
+                    success_response,
+                ]
+
+                response = request_with_retry(method="GET", url="https://example.com", attempts=2, timeout=5)
+
+                assert response == success_response
+                assert mock_request.call_count == 2
+                # Both the initial attempt and the retry must use the caller's timeout, not the default.
+                assert [call.kwargs["timeout"] for call in mock_request.call_args_list] == [5, 5]
+
 
 class TestAsyncRequestWithRetry:
     @pytest.mark.asyncio
@@ -234,3 +258,30 @@ class TestAsyncRequestWithRetry:
                 assert response == success_response
                 assert mock_request.call_count == 2
                 mock_sleep.assert_called()
+
+    @pytest.mark.asyncio
+    async def test_async_request_with_retry_custom_timeout_preserved_across_retries(self):
+        """A custom timeout must be honored on every attempt, not just the first one.
+
+        Regression test: ``timeout`` used to be popped from the shared ``kwargs`` inside the
+        retried inner function, so after the first attempt it was gone and every subsequent
+        retry silently fell back to the default of 10 seconds.
+        """
+        with patch("asyncio.sleep", return_value=None):
+            success_response = httpx.Response(status_code=200, request=httpx.Request("GET", "https://example.com"))
+
+            with patch("httpx.AsyncClient.request") as mock_request:
+                # First attempt fails with a retryable error, second attempt succeeds.
+                mock_request.side_effect = [
+                    httpx.RequestError("boom", request=httpx.Request("GET", "https://example.com")),
+                    success_response,
+                ]
+
+                response = await async_request_with_retry(
+                    method="GET", url="https://example.com", attempts=2, timeout=5
+                )
+
+                assert response == success_response
+                assert mock_request.call_count == 2
+                # Both the initial attempt and the retry must use the caller's timeout, not the default.
+                assert [call.kwargs["timeout"] for call in mock_request.call_args_list] == [5, 5]