Skip to content

Commit 7fa7946

Browse files
tescherm-openaitescherm-openai
authored
fix(client): honor Retry-After delays up to two minutes (#3555)
## Summary - honor positive, finite `Retry-After` delays up to 120 seconds - treat longer server-directed delays as non-retryable instead of substituting short exponential backoff - keep exponential backoff for zero, negative, non-finite, missing, or malformed values - cover sync and async clients with numeric seconds, milliseconds, and HTTP-date boundaries ## Motivation The Python SDK currently ignores `Retry-After` values above 60 seconds and falls back to a much shorter exponential delay. Historical reset-header data shows that common minute-scale waits can land just above 60 seconds, while extending the ceiling beyond two minutes adds little coverage before waits jump to hours or days. ## Developer impact Clients with automatic retries enabled will honor server-directed delays through two minutes. If `Retry-After` exceeds two minutes, the SDK surfaces the API error instead of blocking a synchronous worker for an extended period or retrying before the requested time. Retry attempts remain bounded by `max_retries`. ## Validation - `.venv/bin/ruff format --check src/openai/_constants.py src/openai/_base_client.py tests/test_client.py` - `.venv/bin/ruff check src/openai/_constants.py src/openai/_base_client.py tests/test_client.py` - `.venv/bin/pytest -q tests/test_client.py` (198 passed, 2 skipped)
1 parent 3844843 commit 7fa7946

3 files changed

Lines changed: 111 additions & 9 deletions

File tree

‎src/openai/_base_client.py‎

Lines changed: 20 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22

33
import sys
44
import json
5+
import math
56
import time
67
import uuid
78
import email
@@ -86,6 +87,7 @@
8687
DEFAULT_MAX_RETRIES,
8788
INITIAL_RETRY_DELAY,
8889
RAW_RESPONSE_HEADER,
90+
MAX_RETRY_AFTER_DELAY,
8991
OVERRIDE_CAST_TO_HEADER,
9092
DEFAULT_CONNECTION_LIMITS,
9193
)
@@ -781,11 +783,15 @@ def _parse_retry_after_header(self, response_headers: Optional[httpx.Headers] =
781783
pass
782784

783785
# Last, try parsing `retry-after` as a date.
784-
retry_date_tuple = email.utils.parsedate_tz(retry_header)
785-
if retry_date_tuple is None:
786+
try:
787+
retry_date_tuple = email.utils.parsedate_tz(retry_header)
788+
if retry_date_tuple is None:
789+
return None
790+
791+
retry_date = email.utils.mktime_tz(retry_date_tuple)
792+
except (TypeError, ValueError, OverflowError, OSError):
786793
return None
787794

788-
retry_date = email.utils.mktime_tz(retry_date_tuple)
789795
return float(retry_date - time.time())
790796

791797
def _calculate_retry_timeout(
@@ -796,9 +802,9 @@ def _calculate_retry_timeout(
796802
) -> float:
797803
max_retries = options.get_max_retries(self.max_retries)
798804

799-
# If the API asks us to wait a certain amount of time (and it's a reasonable amount), just do what it says.
805+
# Honor server-directed delays up to two minutes.
800806
retry_after = self._parse_retry_after_header(response_headers)
801-
if retry_after is not None and 0 < retry_after <= 60:
807+
if retry_after is not None and math.isfinite(retry_after) and 0 < retry_after <= MAX_RETRY_AFTER_DELAY:
802808
return retry_after
803809

804810
# Also cap retry count to 1000 to avoid any potential overflows with `pow`
@@ -813,6 +819,15 @@ def _calculate_retry_timeout(
813819
return timeout if timeout >= 0 else 0
814820

815821
def _should_retry(self, response: httpx.Response) -> bool:
822+
retry_after = self._parse_retry_after_header(response.headers)
823+
if retry_after is not None and math.isfinite(retry_after) and retry_after > MAX_RETRY_AFTER_DELAY:
824+
log.debug(
825+
"Not retrying because `Retry-After` of %s seconds exceeds the maximum of %s seconds",
826+
retry_after,
827+
MAX_RETRY_AFTER_DELAY,
828+
)
829+
return False
830+
816831
# Note: this is not a standard header
817832
should_retry_header = response.headers.get("x-should-retry")
818833

‎src/openai/_constants.py‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,3 +12,4 @@
1212

1313
INITIAL_RETRY_DELAY = 0.5
1414
MAX_RETRY_DELAY = 8.0
15+
MAX_RETRY_AFTER_DELAY = 2 * 60

‎tests/test_client.py‎

Lines changed: 90 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1083,14 +1083,21 @@ class Model(BaseModel):
10831083
[3, "0", 0.5],
10841084
[3, "-10", 0.5],
10851085
[3, "60", 60],
1086-
[3, "61", 0.5],
1086+
[3, "61", 61],
1087+
[3, "120", 120],
1088+
[3, "121", 0.5],
10871089
[3, "Fri, 29 Sep 2023 16:26:57 GMT", 20],
10881090
[3, "Fri, 29 Sep 2023 16:26:37 GMT", 0.5],
10891091
[3, "Fri, 29 Sep 2023 16:26:27 GMT", 0.5],
10901092
[3, "Fri, 29 Sep 2023 16:27:37 GMT", 60],
1091-
[3, "Fri, 29 Sep 2023 16:27:38 GMT", 0.5],
1093+
[3, "Fri, 29 Sep 2023 16:27:38 GMT", 61],
1094+
[3, "Fri, 29 Sep 2023 16:28:37 GMT", 120],
1095+
[3, "Fri, 29 Sep 2023 16:28:38 GMT", 0.5],
10921096
[3, "99999999999999999999999999999999999", 0.5],
1097+
[3, "inf", 0.5],
1098+
[3, "nan", 0.5],
10931099
[3, "Zun, 29 Sep 2023 16:26:27 GMT", 0.5],
1100+
[3, "Fri, 29 Sep 100000 16:26:57 GMT", 0.5],
10941101
[3, "", 0.5],
10951102
[2, "", 0.5 * 2.0],
10961103
[1, "", 0.5 * 4.0],
@@ -1106,6 +1113,48 @@ def test_parse_retry_after_header(
11061113
calculated = client._calculate_retry_timeout(remaining_retries, options, headers)
11071114
assert calculated == pytest.approx(timeout, 0.5 * 0.875) # pyright: ignore[reportUnknownMemberType]
11081115

1116+
@pytest.mark.parametrize(
1117+
"headers,should_retry",
1118+
[
1119+
[{"retry-after": "120"}, True],
1120+
[{"retry-after": "121"}, False],
1121+
[{"retry-after-ms": "120000"}, True],
1122+
[{"retry-after-ms": "120001"}, False],
1123+
[{"retry-after": "Fri, 29 Sep 2023 16:28:37 GMT"}, True],
1124+
[{"retry-after": "Fri, 29 Sep 2023 16:28:38 GMT"}, False],
1125+
],
1126+
)
1127+
@mock.patch("time.time", mock.MagicMock(return_value=1696004797))
1128+
def test_retry_after_max_delay(self, headers: dict[str, str], should_retry: bool, client: OpenAI) -> None:
1129+
response = httpx.Response(429, headers=headers)
1130+
assert client._should_retry(response) is should_retry
1131+
1132+
@pytest.mark.respx(base_url=base_url)
1133+
def test_does_not_retry_retry_after_above_max(self, respx_mock: MockRouter, client: OpenAI) -> None:
1134+
route = respx_mock.get("/foo").mock(
1135+
return_value=httpx.Response(429, headers={"retry-after": "121"}, json={"error": {}})
1136+
)
1137+
1138+
with pytest.raises(APIStatusError):
1139+
client.get("/foo", cast_to=httpx.Response)
1140+
1141+
assert route.call_count == 1
1142+
1143+
@pytest.mark.respx(base_url=base_url)
1144+
def test_invalid_retry_after_date_does_not_mask_status_error(self, respx_mock: MockRouter, client: OpenAI) -> None:
1145+
route = respx_mock.get("/foo").mock(
1146+
return_value=httpx.Response(
1147+
400,
1148+
headers={"retry-after": "Fri, 29 Sep 100000 16:26:57 GMT"},
1149+
json={"error": {}},
1150+
)
1151+
)
1152+
1153+
with pytest.raises(APIStatusError):
1154+
client.get("/foo", cast_to=httpx.Response)
1155+
1156+
assert route.call_count == 1
1157+
11091158
@mock.patch("openai._base_client.BaseClient._calculate_retry_timeout", _low_retry_timeout)
11101159
@pytest.mark.respx(base_url=base_url)
11111160
def test_retrying_timeout_errors_doesnt_leak(self, respx_mock: MockRouter, client: OpenAI) -> None:
@@ -2339,14 +2388,21 @@ class Model(BaseModel):
23392388
[3, "0", 0.5],
23402389
[3, "-10", 0.5],
23412390
[3, "60", 60],
2342-
[3, "61", 0.5],
2391+
[3, "61", 61],
2392+
[3, "120", 120],
2393+
[3, "121", 0.5],
23432394
[3, "Fri, 29 Sep 2023 16:26:57 GMT", 20],
23442395
[3, "Fri, 29 Sep 2023 16:26:37 GMT", 0.5],
23452396
[3, "Fri, 29 Sep 2023 16:26:27 GMT", 0.5],
23462397
[3, "Fri, 29 Sep 2023 16:27:37 GMT", 60],
2347-
[3, "Fri, 29 Sep 2023 16:27:38 GMT", 0.5],
2398+
[3, "Fri, 29 Sep 2023 16:27:38 GMT", 61],
2399+
[3, "Fri, 29 Sep 2023 16:28:37 GMT", 120],
2400+
[3, "Fri, 29 Sep 2023 16:28:38 GMT", 0.5],
23482401
[3, "99999999999999999999999999999999999", 0.5],
2402+
[3, "inf", 0.5],
2403+
[3, "nan", 0.5],
23492404
[3, "Zun, 29 Sep 2023 16:26:27 GMT", 0.5],
2405+
[3, "Fri, 29 Sep 100000 16:26:57 GMT", 0.5],
23502406
[3, "", 0.5],
23512407
[2, "", 0.5 * 2.0],
23522408
[1, "", 0.5 * 4.0],
@@ -2362,6 +2418,36 @@ async def test_parse_retry_after_header(
23622418
calculated = async_client._calculate_retry_timeout(remaining_retries, options, headers)
23632419
assert calculated == pytest.approx(timeout, 0.5 * 0.875) # pyright: ignore[reportUnknownMemberType]
23642420

2421+
@pytest.mark.respx(base_url=base_url)
2422+
async def test_does_not_retry_retry_after_above_max(
2423+
self, respx_mock: MockRouter, async_client: AsyncOpenAI
2424+
) -> None:
2425+
route = respx_mock.get("/foo").mock(
2426+
return_value=httpx.Response(429, headers={"retry-after": "121"}, json={"error": {}})
2427+
)
2428+
2429+
with pytest.raises(APIStatusError):
2430+
await async_client.get("/foo", cast_to=httpx.Response)
2431+
2432+
assert route.call_count == 1
2433+
2434+
@pytest.mark.respx(base_url=base_url)
2435+
async def test_invalid_retry_after_date_does_not_mask_status_error(
2436+
self, respx_mock: MockRouter, async_client: AsyncOpenAI
2437+
) -> None:
2438+
route = respx_mock.get("/foo").mock(
2439+
return_value=httpx.Response(
2440+
400,
2441+
headers={"retry-after": "Fri, 29 Sep 100000 16:26:57 GMT"},
2442+
json={"error": {}},
2443+
)
2444+
)
2445+
2446+
with pytest.raises(APIStatusError):
2447+
await async_client.get("/foo", cast_to=httpx.Response)
2448+
2449+
assert route.call_count == 1
2450+
23652451
@mock.patch("openai._base_client.BaseClient._calculate_retry_timeout", _low_retry_timeout)
23662452
@pytest.mark.respx(base_url=base_url)
23672453
async def test_retrying_timeout_errors_doesnt_leak(self, respx_mock: MockRouter, async_client: AsyncOpenAI) -> None:

0 commit comments

Comments
 (0)