Skip to content

fix(client): handle multipart and streaming bodies on paid 402 retry#166

Merged
brendanjryan merged 1 commit into
tempoxyz:mainfrom
bennytimz:fix/client-retry-body-multipart
Jul 10, 2026
Merged

fix(client): handle multipart and streaming bodies on paid 402 retry#166
brendanjryan merged 1 commit into
tempoxyz:mainfrom
bennytimz:fix/client-retry-body-multipart

Conversation

@bennytimz

@bennytimz bennytimz commented Jul 3, 2026

Copy link
Copy Markdown
Contributor

Summary

This PR complements #156 (bytes/content= body replay) and addresses the two body-type gaps that #156 does not cover.

What #156 fixed

PR #156 by @Tleaoo fixes the core bug: PaymentTransport was reusing the already-consumed request.stream on 402 retry, so any POST/PUT/PATCH body was silently dropped. The fix is await request.aread() before dispatch and content=request.content (instead of stream=request.stream) on retry.

What this PR adds on top

1. Multipart / files= bodies (tested)

aread() works for MultipartStream because it implements both SyncByteStream and AsyncByteStream. The full multipart-encoded bytes are buffered before the first dispatch, and the paid retry sends the identical payload.

New tests:

  • test_paid_retry_replays_multipart_bodyfiles= upload, verifies body and filename are present in the retry
  • test_paid_retry_replays_put_body — PUT with a bytes body
  • test_paid_retry_replays_patch_body — PATCH with a bytes body

Also includes test_paid_retry_replays_request_body (same as #156's test) so this branch is self-contained.

2. Async generator bodies — explicit PaymentError

content=async_gen() in httpx produces an AsyncIteratorByteStream which is AsyncByteStream but not SyncByteStream. Unlike MultipartStream and ByteStream, an async generator:

  • may be infinite or tied to a one-shot I/O source
  • may already be partially consumed before the transport receives it
  • silently buffering it would break the streaming contract and could OOM

PaymentTransport.handle_async_request now detects this case before any I/O and raises PaymentError with a clear message directing callers to use a buffered body instead.

New test:

  • test_paid_retry_raises_for_streaming_body — verifies PaymentError is raised immediately with no requests sent

Detection logic

if isinstance(request.stream, httpx.AsyncByteStream) and not isinstance(
    request.stream, httpx.SyncByteStream
):
    raise PaymentError("Streaming request bodies (async generators) are not supported ...")

ByteStream and MultipartStream are both Sync+Async so they pass through. Only AsyncIteratorByteStream (async generators) is async-only and is caught.

Test plan

  • All 28 tests/test_client.py tests pass (confirmed locally)
  • No regressions in the rest of the test suite
  • Multipart body is identical byte-for-byte on the initial and retry requests
  • Async generator body raises PaymentError before any network I/O

@brendanjryan

Copy link
Copy Markdown
Collaborator

LGTM -- thanks for the fix

@brendanjryan

Copy link
Copy Markdown
Collaborator

@bennytimz - can you add a changelog?

Complements PR tempoxyz#156 (bytes/content= body replay) with two additions:

1. Multipart / files= bodies: aread() is called before the first dispatch,
   buffering the MultipartStream into request._content. The paid retry uses
   content=request.content so the full multipart payload is replayed. Tests:
   test_paid_retry_replays_multipart_body, test_paid_retry_replays_put_body,
   test_paid_retry_replays_patch_body.

2. Async generator bodies: an AsyncByteStream that is not also a SyncByteStream
   (i.e. AsyncIteratorByteStream from content=async_gen()) cannot be safely
   buffered for replay — the generator may be infinite, already partially
   consumed, or tied to a one-shot I/O source. PaymentTransport now raises
   PaymentError immediately with a clear message directing callers to use a
   buffered body instead. Test: test_paid_retry_raises_for_streaming_body.

Also includes the core fix from tempoxyz#156: await request.aread() before dispatch
and content=request.content (not stream=request.stream) on retry, so that
all finite body types are correctly replayed.
@bennytimz bennytimz force-pushed the fix/client-retry-body-multipart branch from d207c01 to 654d076 Compare July 7, 2026 20:21
@bennytimz

Copy link
Copy Markdown
Contributor Author

Added, thanks for the review @brendanjryan

bennytimz added a commit to bennytimz/pympp that referenced this pull request Jul 7, 2026
Adds RetryPolicy and wires it into PaymentTransport and Client.

RetryPolicy (src/mpp/client/retry.py):
  - max_attempts=3 with backoff_delays=[0.5, 1.0] seconds by default
  - Retries on status codes in retryable_status_codes (500/502/503/504)
  - Optionally retries on httpx.ConnectError / httpx.RemoteProtocolError
  - Exported from mpp and mpp.client for easy import

PaymentTransport:
  - New retry_policy: RetryPolicy | None = None parameter
  - _dispatch() helper runs the retry loop before 402 handling so that
    transient 5xx errors are resolved before the payment flow begins
  - asyncio.sleep provides the configurable backoff between attempts
  - Each retry attempt logs a warning with attempt number and URL
  - Async-generator bodies (guarded by PR tempoxyz#166) propagate before the
    retry loop, so no unsafe body replay can occur

Client:
  - retry_policy defaults to RetryPolicy() (retry on by default)
  - Pass retry_policy=None to get raw no-retry behaviour

Tests (tests/test_client.py):
  - test_5xx_retried_with_backoff: 503x2 then 200; verifies sleep(0.5) and sleep(1.0)
  - test_5xx_exhausted_returns_last_response: max_attempts=2, always 503
  - test_connect_error_retried: ConnectError then 200
  - test_retry_disabled_when_policy_is_none: PaymentTransport with retry_policy=None
  - test_non_retryable_5xx_not_retried: 501 passes through in one attempt
  - test_client_retry_disabled_when_policy_is_none: Client(retry_policy=None) via HTTPXMock
@brendanjryan brendanjryan merged commit a5789c1 into tempoxyz:main Jul 10, 2026
13 checks passed
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.

2 participants