Skip to content

Commit 24fadc4

Browse files
authored
fix: don't retry an API error that declares itself non-retryable (#358)
## What The client stops retrying an API error whose response body says it can't succeed on a retry. Today every 5xx is retried until the retry budget runs out, even when the server has already declared that resending the same request will fail the same way. With the platform's retry settings (20s initial, 90s cap, 300s budget): ``` Before: POST → 503 {"retryable": false} → wait 20s → 503 → … → 6 attempts, ~320s, then ServerError After: POST → 503 {"retryable": false} → ServerError (1 attempt, same body) ``` | Error body | Before | After | |---|---|---| | `{"detail": …, "retryable": false, …}` | retried | **returned at once** | | `{"detail": …, "retryable": true, …}` | retried | retried | | `{"detail": "Internal server error"}` (no verdict) | retried | retried | | non-JSON, `"retryable": "false"` (string), unparsable | retried | retried | The same check covers the split-PDF path. A chunk that gets a declared non-retryable 5xx is sent once instead of using up its own per-chunk retry budget. ## Why The Unstructured API publishes its own verdict next to `detail` on errors it raises deliberately: `error_type`, `audience`, `retryable` and `error_reason`. An OCR (Tesseract) timeout, for example, answers 503 with `retryable: false`. The same image re-sent to the same time budget times out again. The retry loop matches on status before any caller can read the body, so no caller can act on that verdict. In production, one Transform record received 12 HTTP 500s across two dispatches over 670s. Attempt-6 retry markers are 98.6% as common as attempt-1 markers, so these failures do not recover on retry. The retry loop is the one place that both whole-document requests and split-PDF chunk requests pass through. A caller-side hook can't reach the split-PDF chunks, because the split hook sends them through its own HTTP client. The check only ever *stops* a retry. A declared `retryable: true` never makes a status retryable that wasn't already. `retries.py` already carries hand edits (#342). `gen.yaml` has `persistentEdits` enabled. ## Notes for consumers - No API change. Only a JSON object body with the boolean `"retryable": false` changes behaviour. An unread streamed response is never consumed; it keeps status-based retries. - Retry-After is still not honored. This PR doesn't change that. ## Validation - Unit suite: 371 passed, 1 xfailed. `make lint` is clean (pylint 10/10, mypy). - New tests cover sync and async retries, exact and range status codes, the split-PDF chunk path in buffered and cached modes, and public `partition` / `partition_async` through a mocked transport. - Two rounds of Codex review. The finding from round 1 (an uncaught `RecursionError` on a deeply nested body) is fixed. <!-- This is an auto-generated description by cubic. --> <a href="https://cubic.dev/pr/Unstructured-IO/unstructured-python-client/pull/358?utm_source=github" target="_blank" rel="noopener noreferrer" data-no-image-dialog="true"><picture><source media="(prefers-color-scheme: dark)" srcset="https://www.cubic.dev/buttons/review-in-cubic-dark.svg"><source media="(prefers-color-scheme: light)" srcset="https://www.cubic.dev/buttons/review-in-cubic-light.svg"><img alt="View guided diff" src="https://www.cubic.dev/buttons/review-in-cubic-light.svg"></picture></a> <!-- End of auto-generated description by cubic. -->
1 parent ba25d25 commit 24fadc4

5 files changed

Lines changed: 228 additions & 29 deletions

File tree

‎CHANGELOG.md‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,8 @@
1+
## 0.46.4
2+
3+
### Fixes
4+
* Return API errors immediately when their JSON body declares `"retryable": false`, instead of retrying them until the retry budget expires. Applies to sync and async requests, including split-PDF chunks. Other errors keep the existing status-based retry behavior.
5+
16
## 0.46.3
27

38
### Fixes

‎_test_unstructured_client/unit/test_retries.py‎

Lines changed: 160 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,8 @@
55
import httpx
66
import pytest
77

8+
from unstructured_client import UnstructuredClient
9+
from unstructured_client.models.errors import ServerError
810
from unstructured_client.utils import retries as retries_module
911
from unstructured_client.utils.retries import (
1012
BackoffStrategy,
@@ -555,3 +557,161 @@ async def func():
555557
with pytest.raises(ValueError, match="permanent"):
556558
asyncio.run(retry_async(func, retries_config))
557559
assert call_count == 1
560+
561+
562+
def _send_with_retry(respond, config, async_mode, *, stream=False):
563+
transport = httpx.MockTransport(respond)
564+
if async_mode:
565+
async def run():
566+
async with httpx.AsyncClient(transport=transport) as client:
567+
return await retry_async(
568+
lambda: client.send(client.build_request("GET", "http://test.invalid"), stream=stream),
569+
config,
570+
)
571+
return asyncio.run(run())
572+
with httpx.Client(transport=transport) as client:
573+
return retry(
574+
lambda: client.send(client.build_request("GET", "http://test.invalid"), stream=stream),
575+
config,
576+
)
577+
578+
579+
@pytest.mark.parametrize("async_mode", [False, True])
580+
@pytest.mark.parametrize(
581+
"body,content_type,stop",
582+
[
583+
(b'{"detail":"unsupported","retryable":false}', "application/json", True),
584+
(b'{"retryable":false}', "application/problem+json; charset=utf-8", True),
585+
(b'{"retryable":true}', "application/json", False),
586+
(b'{"detail":"Internal server error"}', "application/json", False),
587+
(b'{"retryable":"false"}', "application/json", False),
588+
(b'{"retryable":0}', "application/json", False),
589+
(b'{"retryable":null}', "application/json", False),
590+
(b'[{"retryable":false}]', "application/json", False),
591+
(b'false', "application/json", False),
592+
(b'{broken', "application/json", False),
593+
pytest.param(
594+
b'{"retryable":false,"detail":' + b"[" * 100_000 + b"]" * 100_000 + b"}",
595+
"application/json",
596+
False,
597+
id="deeply-nested",
598+
),
599+
(b'upstream error', "text/plain", False),
600+
(b'{"retryable":false}', "text/plain", True),
601+
(b'{"retryable":false}', "", True),
602+
],
603+
)
604+
def test_declared_retry_verdict(
605+
fake_clock, async_mode, body, content_type, stop
606+
):
607+
status_code = 503
608+
attempts = []
609+
610+
def respond(request):
611+
attempts.append(request)
612+
if len(attempts) == 1:
613+
return httpx.Response(status_code, content=body, headers={"Content-Type": content_type})
614+
return httpx.Response(200, json=[])
615+
616+
config = _retries(status_codes=["5xx"], initial_interval=1, max_interval=1)
617+
result = _send_with_retry(respond, config, async_mode)
618+
619+
assert len(attempts) == (1 if stop else 2)
620+
assert result.status_code == (status_code if stop else 200)
621+
if stop:
622+
assert result.content == body
623+
624+
625+
@pytest.mark.parametrize("async_mode", [False, True])
626+
@pytest.mark.parametrize("verdict", [False, True])
627+
def test_declared_verdict_does_not_retry_non_matching_status(fake_clock, async_mode, verdict):
628+
attempts = []
629+
630+
def respond(request):
631+
attempts.append(request)
632+
return httpx.Response(400, json={"retryable": verdict})
633+
634+
config = _retries(status_codes=["5xx"])
635+
response = _send_with_retry(respond, config, async_mode)
636+
637+
assert response.status_code == 400
638+
assert len(attempts) == 1
639+
640+
641+
@pytest.mark.parametrize("async_mode", [False, True])
642+
def test_unread_stream_is_not_consumed(fake_clock, async_mode):
643+
class SyncStream(httpx.SyncByteStream):
644+
def __iter__(self):
645+
pytest.fail("retry inspected an unread stream")
646+
yield b""
647+
648+
class AsyncStream(httpx.AsyncByteStream):
649+
async def __aiter__(self):
650+
pytest.fail("retry inspected an unread stream")
651+
yield b""
652+
653+
attempts = []
654+
655+
def respond(request):
656+
attempts.append(request)
657+
if len(attempts) == 1:
658+
return httpx.Response(
659+
503, headers={"Content-Type": "application/json"},
660+
stream=AsyncStream() if async_mode else SyncStream(),
661+
)
662+
return httpx.Response(200)
663+
664+
config = _retries(status_codes=["5xx"])
665+
result = _send_with_retry(respond, config, async_mode, stream=True)
666+
667+
assert result.status_code == 200
668+
assert len(attempts) == 2
669+
670+
671+
@pytest.mark.parametrize("async_mode", [False, True])
672+
@pytest.mark.parametrize("status_code", [500, 503])
673+
def test_public_partition_preserves_declared_error(fake_clock, async_mode, status_code):
674+
body = {"detail": "unsupported document", "retryable": False}
675+
attempts = []
676+
677+
def respond(request):
678+
attempts.append(request)
679+
return httpx.Response(status_code, json=body)
680+
681+
async def run():
682+
transport = httpx.MockTransport(respond)
683+
with httpx.Client(transport=transport) as client:
684+
async with httpx.AsyncClient(transport=transport) as async_client:
685+
sdk = UnstructuredClient(
686+
server_url="http://test.invalid", client=client, async_client=async_client
687+
)
688+
request = {"partition_parameters": {
689+
"files": {"content": b"hello", "file_name": "test.txt"},
690+
"split_pdf_page": False,
691+
}}
692+
config = _retries(initial_interval=1, max_interval=1, max_elapsed_time=2).config
693+
with pytest.raises(ServerError) as exc:
694+
if async_mode:
695+
await sdk.general.partition_async(request=request, retries=config)
696+
else:
697+
sdk.general.partition(request=request, retries=config)
698+
assert exc.value.raw_response.json() == body
699+
assert "unsupported document" in str(exc.value)
700+
701+
asyncio.run(run())
702+
assert len(attempts) == 1
703+
704+
705+
@pytest.mark.parametrize("async_mode", [False, True])
706+
@pytest.mark.parametrize("status_code", [500, 503])
707+
@pytest.mark.parametrize("status_codes", [["5xx"], ["500", "503"]])
708+
def test_declared_verdict_stops_matching_status(fake_clock, async_mode, status_code, status_codes):
709+
attempts = []
710+
711+
def respond(request):
712+
attempts.append(request)
713+
return httpx.Response(status_code, json={"retryable": False})
714+
715+
response = _send_with_retry(respond, _retries(status_codes=status_codes), async_mode)
716+
assert response.status_code == status_code
717+
assert len(attempts) == 1

‎_test_unstructured_client/unit/test_split_pdf_hook.py‎

Lines changed: 36 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2186,4 +2186,39 @@ def _chunk_paths_side_effect(*args, **kwargs):
21862186
assert hook.cache_tmp_data_feature == {}
21872187
assert hook.cache_tmp_data_dir == {}
21882188
tempdir.cleanup.assert_called_once()
2189-
executor.shutdown.assert_not_called()
2189+
executor.shutdown.assert_not_called()
2190+
2191+
2192+
@pytest.mark.asyncio
2193+
@pytest.mark.parametrize("cached", [False, True])
2194+
@pytest.mark.parametrize("status_code", [500, 503])
2195+
async def test_declared_non_retryable_chunk_sent_once(tmp_path, cached, status_code):
2196+
body = {"detail": "unsupported chunk", "retryable": False}
2197+
attempts = []
2198+
2199+
def respond(request):
2200+
attempts.append(request)
2201+
return httpx.Response(status_code, json=body)
2202+
2203+
async with httpx.AsyncClient(transport=httpx.MockTransport(respond)) as client:
2204+
response = await SplitPdfHook().call_api_partial(
2205+
pdf_chunk_request=httpx.Request(
2206+
"POST",
2207+
"http://test.invalid/general/v0/general",
2208+
headers={"Accept": "application/x-ndjson" if cached else "application/json"},
2209+
content=b"pdf chunk",
2210+
),
2211+
pdf_chunk_file=io.BytesIO(b"pdf chunk"),
2212+
limiter=asyncio.Semaphore(1),
2213+
_operation_id="declared-error",
2214+
chunk_index=0,
2215+
page_number=1,
2216+
async_client=client,
2217+
retry_config=RetryConfig("backoff", BackoffStrategy(1, 1, 1, 2), True),
2218+
cache_tmp_data_feature=cached,
2219+
temp_dir_path=str(tmp_path),
2220+
)
2221+
assert len(attempts) == 1
2222+
assert response.status_code == status_code
2223+
assert response.json() == body
2224+
assert list(tmp_path.iterdir()) == []

‎src/unstructured_client/_version.py‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,10 +3,10 @@
33
import importlib.metadata
44

55
__title__: str = "unstructured-client"
6-
__version__: str = "0.46.3"
6+
__version__: str = "0.46.4"
77
__openapi_doc_version__: str = "1.2.31"
88
__gen_version__: str = "2.680.0"
9-
__user_agent__: str = "speakeasy-sdk/python 0.46.3 2.680.0 1.2.31 unstructured-client"
9+
__user_agent__: str = "speakeasy-sdk/python 0.46.4 2.680.0 1.2.31 unstructured-client"
1010

1111
try:
1212
if __package__ is not None:

‎src/unstructured_client/utils/retries.py‎

Lines changed: 25 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -87,6 +87,27 @@ def __init__(self, inner: Exception):
8787
self.inner = inner
8888

8989

90+
def _declares_non_retryable(response: httpx.Response) -> bool:
91+
# Inspect buffered error bodies without consuming unread response streams.
92+
try:
93+
body = response.json()
94+
except (ValueError, RecursionError, httpx.ResponseNotRead):
95+
return False
96+
return isinstance(body, dict) and body.get("retryable") is False
97+
98+
99+
def _is_retry_status(response: httpx.Response, status_codes: List[str]) -> bool:
100+
for code in status_codes:
101+
if "X" in code.upper():
102+
code_range = int(code[0])
103+
status_major = response.status_code / 100
104+
if code_range <= status_major < code_range + 1:
105+
return True
106+
elif response.status_code == int(code):
107+
return True
108+
return False
109+
110+
90111
def retry(func, retries: Retries):
91112
if retries.config.strategy == "backoff":
92113

@@ -95,19 +116,8 @@ def do_request() -> httpx.Response:
95116
try:
96117
res = func()
97118

98-
for code in retries.status_codes:
99-
if "X" in code.upper():
100-
code_range = int(code[0])
101-
102-
status_major = res.status_code / 100
103-
104-
if code_range <= status_major < code_range + 1:
105-
raise TemporaryError(res)
106-
else:
107-
parsed_code = int(code)
108-
109-
if res.status_code == parsed_code:
110-
raise TemporaryError(res)
119+
if _is_retry_status(res, retries.status_codes) and not _declares_non_retryable(res):
120+
raise TemporaryError(res)
111121
except httpx.TransportError as exception:
112122
if retries.config.retry_connection_errors:
113123
raise
@@ -141,19 +151,8 @@ async def do_request() -> httpx.Response:
141151
try:
142152
res = await func()
143153

144-
for code in retries.status_codes:
145-
if "X" in code.upper():
146-
code_range = int(code[0])
147-
148-
status_major = res.status_code / 100
149-
150-
if code_range <= status_major < code_range + 1:
151-
raise TemporaryError(res)
152-
else:
153-
parsed_code = int(code)
154-
155-
if res.status_code == parsed_code:
156-
raise TemporaryError(res)
154+
if _is_retry_status(res, retries.status_codes) and not _declares_non_retryable(res):
155+
raise TemporaryError(res)
157156
except httpx.TransportError as exception:
158157
if retries.config.retry_connection_errors:
159158
raise

0 commit comments

Comments
 (0)