diff --git a/haystack/utils/requests_utils.py b/haystack/utils/requests_utils.py index c5270cc055a..050228bda2f 100644 --- a/haystack/utils/requests_utils.py +++ b/haystack/utils/requests_utils.py @@ -58,6 +58,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(), @@ -67,7 +70,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) @@ -168,6 +170,9 @@ async def example_5xx(): 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(), @@ -177,7 +182,6 @@ async def example_5xx(): 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) diff --git a/releasenotes/notes/fix-request-with-retry-timeout-on-retry-2e1437ea8cbba7ad.yaml b/releasenotes/notes/fix-request-with-retry-timeout-on-retry-2e1437ea8cbba7ad.yaml new file mode 100644 index 00000000000..04af94f40bf --- /dev/null +++ b/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. diff --git a/test/utils/test_requests_utils.py b/test/utils/test_requests_utils.py index 9bf621268ae..c011ec964bf 100644 --- a/test/utils/test_requests_utils.py +++ b/test/utils/test_requests_utils.py @@ -121,6 +121,30 @@ def raise_for_status(): 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 @@ def raise_for_status(): 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]