Repository navigation
[13.x] Fix multipart requests losing attached stream contents on retry - #61728
Merged
taylorotwell merged 1 commit intoSep 25, 2026
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
attach()accepts a stream resource andretry()is documented to retry connection failures, but combining them is broken (#61684):InvalidArgumentException: Invalid resource type: resource (closed)instead of the documentedConnectionException.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 newMultipartStreamon every attempt. EachMultipartStreamwraps the same raw PHP resource in a freshGuzzleHttp\Psr7\Stream:Streamwrapper is garbage-collected, its destructor closes the underlying resource, so a later attempt failsUtils::streamFor()withInvalid resource type: resource (closed).Fix
Two small, contained changes in
PendingRequest:send()(wrapMultipartStreams()). Guzzle'sMultipartStreamreuses a passedStreamInterfaceas-is instead of creating a new wrapper per attempt, so no attempt-scoped destructor ever closes the caller's resource.rewindMultipartStreams()), in both the synchronous retry loop andmakePromise()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 inHttp::fake()assertions for resource-backed attachments, since the recorded part holds a stringableStreamInterfacerather 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 forHttp::async().testExhaustedRetriesWithAttachedStreamThrowConnectionException— when every attempt fails to connect, the caller receives the documentedConnectionException(notInvalidArgumentException), 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/Httpsuite passes.Fixes #61684