Skip to content

test: pin down core HTTP retry behaviour - #850

Open
kraenhansen wants to merge 1 commit into
mainfrom
kh/test-http-retry-behavior
Open

test: pin down core HTTP retry behaviour#850
kraenhansen wants to merge 1 commit into
mainfrom
kh/test-http-retry-behavior

Conversation

@kraenhansen

Copy link
Copy Markdown
Member

A user reported a regression between v2.58.0 and v2.59.0: they cancel an in-flight upload (async task cancellation → connection teardown), typically after the body is sent but before the response is read, and believed the SDK now treats that cancellation as a retryable error and re-sends it.

This adds characterisation tests for the retry policy in core/http_client.py so the two separable questions in that report are pinned down independently.

Cancellation is not retried

_should_retry() takes an httpx.Response and only inspects response.status_code. There is no try/except anywhere around the HTTP call — the only exception handling in the generated raw clients is JSONDecodeError/ValidationError for response parsing. So any exception escapes before a retry decision is reached.

Covered: a faithful reproduction of the reported scenario (transport reads the full body, then hangs; the awaiting task is cancelled), CancelledError raised inside the transport, and ReadError / WriteError / RemoteProtocolError / ConnectError / ReadTimeout. All assert exactly one attempt.

Default retry count changed 0 → 2

This is the real behaviour change, and it explains the duplicate uploads they saw. The pre-v2.59.0 code was effectively dead:

retries: int = 2                                     # param default
max_retries = request_options.get("max_retries", 0)  # default 0
if max_retries > retries:                            # 0 > 2 → never True

Effective retries were max(0, max_retries - 2), so even an explicit max_retries=2 did nothing. It is now retries: int = 0, max_retries defaulting to 2, and retries < max_retries. This arrived via the Fern generator bump 4.42.04.64.1 in #817, not a hand-written change.

test_retry_resends_the_full_request_body pins the cost that makes the default matter: a 503 re-sends the entire body three times. _should_retry keys purely off status code with no method awareness, so this applies to 10+ multipart POST upload endpoints. Raised with Fern separately, along with a request for a client-wide max_retries (it currently only exists on RequestOptions).

No source changes — this only documents current behaviour, and deliberately asserts the 0 → 2 default so a future generator bump that changes it again fails loudly.

Test plan

  • pytest tests/test_retry_behavior.py — 27 passed
  • Bisected against the v2.58.0 http_client.py: the 14 cancellation / transport-error / non-retryable tests pass unchanged, the 13 retry-count tests fail. Confirms cancellation handling is identical across versions and only the retry count regressed.
  • mypy tests/test_retry_behavior.py — clean
  • Backoff is monkeypatched to 1 ms, so the suite runs in ~0.2 s

🤖 Generated with Claude Code

Characterises the default retry policy in core/http_client.py after a user
reported a regression between v2.58.0 and v2.59.0, claiming cancelled
in-flight requests were being retried.

Splits the report into its two separable questions: whether cancellation is
retried (it is not) and how many times a retryable response is retried
(zero before, two after).

Co-Authored-By: Claude <noreply@anthropic.com>
@kraenhansen
kraenhansen marked this pull request as ready for review August 20, 2026 08:50
@cursor

cursor Bot commented Aug 20, 2026

Copy link
Copy Markdown

PR Summary

Cursor Bugbot is generating a summary for commit 7937286. Configure here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant