Skip to content

[13.x] Fix multipart requests losing attached stream contents on retry - #61728

Merged
taylorotwell merged 1 commit into
laravel:13.xfrom
mehdishakki:fix/retry-multipart-stream-reuse
Sep 25, 2026
Merged

taylorotwell merged 1 commit into
laravel:13.xfrom
mehdishakki:fix/retry-multipart-stream-reuse

Conversation

@mehdishakki

@mehdishakki mehdishakki commented Sep 25, 2026 •

Copy link
Copy Markdown
Contributor

Problem

attach() accepts a stream resource and retry() is documented to retry connection failures, but combining them is broken (#61684):

Http::retry(3, 0)
    ->attach('file', fopen($path, 'rb'), 'hello.txt')
    ->post('http://127.0.0.1:1/upload');
  • Attempt 1 sends the file bytes, then fails.
  • Attempt 2 sends the part without the file contents — the resource is already at EOF.
  • Attempt 3 throws InvalidArgumentException: Invalid resource type: resource (closed) instead of the documented ConnectionException.

Worse, when the retry count is low enough that the closed-resource state is never reached, the failure is silent: the request "succeeds" on a retry while the uploaded file is empty.

Root cause

PendingRequest::send() builds $options['multipart'] once, but Guzzle converts that option into a new MultipartStream on every attempt. Each MultipartStream wraps the same raw PHP resource in a fresh GuzzleHttp\Psr7\Stream:

  1. The first attempt reads the resource to EOF, so subsequent attempts build parts from an exhausted stream.
  2. When a previous attempt's Stream wrapper is garbage-collected, its destructor closes the underlying resource, so a later attempt fails Utils::streamFor() with Invalid resource type: resource (closed).

Fix

Two small, contained changes in PendingRequest:

  1. Wrap attached resources in a PSR-7 stream once, in send() (wrapMultipartStreams()). Guzzle's MultipartStream reuses a passed StreamInterface as-is instead of creating a new wrapper per attempt, so no attempt-scoped destructor ever closes the caller's resource.
  2. Rewind seekable multipart streams before each retry attempt (rewindMultipartStreams()), in both the synchronous retry loop and makePromise() for async requests, so every attempt sends the full part contents.

Non-seekable streams (sockets, pipes) are left untouched — they cannot be replayed, which matches Guzzle's own retry semantics. No method signatures change, and single-attempt requests behave exactly as before (the wrapping stream is destroyed at the same point the raw resource used to be closed).

As a side benefit, Request::hasFile('file', $contents) now also works in Http::fake() assertions for resource-backed attachments, since the recorded part holds a stringable StreamInterface rather than a raw resource.

Tests

  • testRetriedMultipartRequestsResendAttachedStreamContents — sync retry: every attempt's raw multipart payload contains the attached bytes; the request succeeds once the connection recovers.
  • testAsyncRetriedMultipartRequestsResendAttachedStreamContents — the same guarantee for Http::async().
  • testExhaustedRetriesWithAttachedStreamThrowConnectionException — when every attempt fails to connect, the caller receives the documented ConnectionException (not InvalidArgumentException), and every attempt still carried the full file contents.

All three fail before this change (the second attempt's payload is missing the attached bytes) and pass after it. The full tests/Http suite passes.

Fixes #61684

@taylorotwell
taylorotwell merged commit b5e0572 into laravel:13.x Sep 25, 2026
62 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.

Http::retry() with attach(fopen()) drops the file, then throws InvalidArgumentException

2 participants