diff --git a/src/openai/_base_client.py b/src/openai/_base_client.py index 3d7dda5afd..10d7b9f7ca 100644 --- a/src/openai/_base_client.py +++ b/src/openai/_base_client.py @@ -2,6 +2,7 @@ import sys import json +import math import time import uuid import email @@ -86,6 +87,7 @@ DEFAULT_MAX_RETRIES, INITIAL_RETRY_DELAY, RAW_RESPONSE_HEADER, + MAX_RETRY_AFTER_DELAY, OVERRIDE_CAST_TO_HEADER, DEFAULT_CONNECTION_LIMITS, ) @@ -781,11 +783,15 @@ def _parse_retry_after_header(self, response_headers: Optional[httpx.Headers] = pass # Last, try parsing `retry-after` as a date. - retry_date_tuple = email.utils.parsedate_tz(retry_header) - if retry_date_tuple is None: + try: + retry_date_tuple = email.utils.parsedate_tz(retry_header) + if retry_date_tuple is None: + return None + + retry_date = email.utils.mktime_tz(retry_date_tuple) + except (TypeError, ValueError, OverflowError, OSError): return None - retry_date = email.utils.mktime_tz(retry_date_tuple) return float(retry_date - time.time()) def _calculate_retry_timeout( @@ -796,9 +802,9 @@ def _calculate_retry_timeout( ) -> float: max_retries = options.get_max_retries(self.max_retries) - # If the API asks us to wait a certain amount of time (and it's a reasonable amount), just do what it says. + # Honor server-directed delays up to two minutes. retry_after = self._parse_retry_after_header(response_headers) - if retry_after is not None and 0 < retry_after <= 60: + if retry_after is not None and math.isfinite(retry_after) and 0 < retry_after <= MAX_RETRY_AFTER_DELAY: return retry_after # Also cap retry count to 1000 to avoid any potential overflows with `pow` @@ -813,6 +819,15 @@ def _calculate_retry_timeout( return timeout if timeout >= 0 else 0 def _should_retry(self, response: httpx.Response) -> bool: + retry_after = self._parse_retry_after_header(response.headers) + if retry_after is not None and math.isfinite(retry_after) and retry_after > MAX_RETRY_AFTER_DELAY: + log.debug( + "Not retrying because `Retry-After` of %s seconds exceeds the maximum of %s seconds", + retry_after, + MAX_RETRY_AFTER_DELAY, + ) + return False + # Note: this is not a standard header should_retry_header = response.headers.get("x-should-retry") diff --git a/src/openai/_constants.py b/src/openai/_constants.py index 7029dc72b0..fd73c6485d 100644 --- a/src/openai/_constants.py +++ b/src/openai/_constants.py @@ -12,3 +12,4 @@ INITIAL_RETRY_DELAY = 0.5 MAX_RETRY_DELAY = 8.0 +MAX_RETRY_AFTER_DELAY = 2 * 60 diff --git a/tests/test_client.py b/tests/test_client.py index bdbc2ce26b..33a5b1c224 100644 --- a/tests/test_client.py +++ b/tests/test_client.py @@ -1083,14 +1083,21 @@ class Model(BaseModel): [3, "0", 0.5], [3, "-10", 0.5], [3, "60", 60], - [3, "61", 0.5], + [3, "61", 61], + [3, "120", 120], + [3, "121", 0.5], [3, "Fri, 29 Sep 2023 16:26:57 GMT", 20], [3, "Fri, 29 Sep 2023 16:26:37 GMT", 0.5], [3, "Fri, 29 Sep 2023 16:26:27 GMT", 0.5], [3, "Fri, 29 Sep 2023 16:27:37 GMT", 60], - [3, "Fri, 29 Sep 2023 16:27:38 GMT", 0.5], + [3, "Fri, 29 Sep 2023 16:27:38 GMT", 61], + [3, "Fri, 29 Sep 2023 16:28:37 GMT", 120], + [3, "Fri, 29 Sep 2023 16:28:38 GMT", 0.5], [3, "99999999999999999999999999999999999", 0.5], + [3, "inf", 0.5], + [3, "nan", 0.5], [3, "Zun, 29 Sep 2023 16:26:27 GMT", 0.5], + [3, "Fri, 29 Sep 100000 16:26:57 GMT", 0.5], [3, "", 0.5], [2, "", 0.5 * 2.0], [1, "", 0.5 * 4.0], @@ -1106,6 +1113,48 @@ def test_parse_retry_after_header( calculated = client._calculate_retry_timeout(remaining_retries, options, headers) assert calculated == pytest.approx(timeout, 0.5 * 0.875) # pyright: ignore[reportUnknownMemberType] + @pytest.mark.parametrize( + "headers,should_retry", + [ + [{"retry-after": "120"}, True], + [{"retry-after": "121"}, False], + [{"retry-after-ms": "120000"}, True], + [{"retry-after-ms": "120001"}, False], + [{"retry-after": "Fri, 29 Sep 2023 16:28:37 GMT"}, True], + [{"retry-after": "Fri, 29 Sep 2023 16:28:38 GMT"}, False], + ], + ) + @mock.patch("time.time", mock.MagicMock(return_value=1696004797)) + def test_retry_after_max_delay(self, headers: dict[str, str], should_retry: bool, client: OpenAI) -> None: + response = httpx.Response(429, headers=headers) + assert client._should_retry(response) is should_retry + + @pytest.mark.respx(base_url=base_url) + def test_does_not_retry_retry_after_above_max(self, respx_mock: MockRouter, client: OpenAI) -> None: + route = respx_mock.get("/foo").mock( + return_value=httpx.Response(429, headers={"retry-after": "121"}, json={"error": {}}) + ) + + with pytest.raises(APIStatusError): + client.get("/foo", cast_to=httpx.Response) + + assert route.call_count == 1 + + @pytest.mark.respx(base_url=base_url) + def test_invalid_retry_after_date_does_not_mask_status_error(self, respx_mock: MockRouter, client: OpenAI) -> None: + route = respx_mock.get("/foo").mock( + return_value=httpx.Response( + 400, + headers={"retry-after": "Fri, 29 Sep 100000 16:26:57 GMT"}, + json={"error": {}}, + ) + ) + + with pytest.raises(APIStatusError): + client.get("/foo", cast_to=httpx.Response) + + assert route.call_count == 1 + @mock.patch("openai._base_client.BaseClient._calculate_retry_timeout", _low_retry_timeout) @pytest.mark.respx(base_url=base_url) def test_retrying_timeout_errors_doesnt_leak(self, respx_mock: MockRouter, client: OpenAI) -> None: @@ -2339,14 +2388,21 @@ class Model(BaseModel): [3, "0", 0.5], [3, "-10", 0.5], [3, "60", 60], - [3, "61", 0.5], + [3, "61", 61], + [3, "120", 120], + [3, "121", 0.5], [3, "Fri, 29 Sep 2023 16:26:57 GMT", 20], [3, "Fri, 29 Sep 2023 16:26:37 GMT", 0.5], [3, "Fri, 29 Sep 2023 16:26:27 GMT", 0.5], [3, "Fri, 29 Sep 2023 16:27:37 GMT", 60], - [3, "Fri, 29 Sep 2023 16:27:38 GMT", 0.5], + [3, "Fri, 29 Sep 2023 16:27:38 GMT", 61], + [3, "Fri, 29 Sep 2023 16:28:37 GMT", 120], + [3, "Fri, 29 Sep 2023 16:28:38 GMT", 0.5], [3, "99999999999999999999999999999999999", 0.5], + [3, "inf", 0.5], + [3, "nan", 0.5], [3, "Zun, 29 Sep 2023 16:26:27 GMT", 0.5], + [3, "Fri, 29 Sep 100000 16:26:57 GMT", 0.5], [3, "", 0.5], [2, "", 0.5 * 2.0], [1, "", 0.5 * 4.0], @@ -2362,6 +2418,36 @@ async def test_parse_retry_after_header( calculated = async_client._calculate_retry_timeout(remaining_retries, options, headers) assert calculated == pytest.approx(timeout, 0.5 * 0.875) # pyright: ignore[reportUnknownMemberType] + @pytest.mark.respx(base_url=base_url) + async def test_does_not_retry_retry_after_above_max( + self, respx_mock: MockRouter, async_client: AsyncOpenAI + ) -> None: + route = respx_mock.get("/foo").mock( + return_value=httpx.Response(429, headers={"retry-after": "121"}, json={"error": {}}) + ) + + with pytest.raises(APIStatusError): + await async_client.get("/foo", cast_to=httpx.Response) + + assert route.call_count == 1 + + @pytest.mark.respx(base_url=base_url) + async def test_invalid_retry_after_date_does_not_mask_status_error( + self, respx_mock: MockRouter, async_client: AsyncOpenAI + ) -> None: + route = respx_mock.get("/foo").mock( + return_value=httpx.Response( + 400, + headers={"retry-after": "Fri, 29 Sep 100000 16:26:57 GMT"}, + json={"error": {}}, + ) + ) + + with pytest.raises(APIStatusError): + await async_client.get("/foo", cast_to=httpx.Response) + + assert route.call_count == 1 + @mock.patch("openai._base_client.BaseClient._calculate_retry_timeout", _low_retry_timeout) @pytest.mark.respx(base_url=base_url) async def test_retrying_timeout_errors_doesnt_leak(self, respx_mock: MockRouter, async_client: AsyncOpenAI) -> None: