[HttpClient] Fix buffering after calling AsyncContext::passthru() #47879
Conversation
|
Hey! I see that this is your first PR. That is great! Welcome! Symfony has a contribution guide which I suggest you to read. In short:
Review the GitHub status checks of your pull request and try to solve the reported issues. If some tests are failing, try to see if they are failing because of this change. When two Symfony core team members approve this change, it will be merged and you will become an official Symfony contributor! I am going to sit back now and wait for the reviews. Cheers! Carsonbot |
|
The attached patch doesn't look correct to me because it'd mean yielding a $chunk that should be skipped, since it belongs to a response that should be retried instead. Can you reproduce your scenario in a test case so that we can figure out another fix maybe? |
|
Yes, sure it's not the correct one, it's just a dirty hack to try it out if we open the buffer do we receive the seekable stream? |
|
Not yet sorry. The test case would help a lot! |
|
@nicolas-grekas the test has been added, I will waiting for your response |
8cc9e10 to
b1d03d1
Compare
b1d03d1 to
b559f80
Compare
|
Thank you @lubo13. |
|
@nicolas-grekas Thank you, could we apply this fix to 6.1 with a new bugfix version? Sorry, but I could find the tag, if you already have created such please share it. |
|
It's already merged up. Releases happen at the end of the month. |
|
Okay, Thank you |
Hi, guys, I think there is an issue with the RetryableHttpClient, let me explain to you what I've seen.
Request is executed into AsyncResponse::__construct
The initializer Closure executes AsyncResponse::passthru, there the AsyncContext and a new stream are created
The next step is yielding from passthruStream and there the stream is checked -
$r->stream->valid(). Here the stream is not a valid iterator, because the last retry attempt has already been made and the generator is closed.This leads the response body stream to be unseekable because the
$r->openBuffer()is not called. The content is left with a valuenulland whenAsyncResponse->toStream()is called fromPsr18Client::110the content with value null is bound to thecontent of StreamWrapper.After that this newly generated resource is passed to the
Psr17Factory->createStreamFromResource()and the resulted (unseekable body) stream is passed to the$response->withBody($body)ontoPsr18Client::121The result is that the Response body stream is not seekable and when I execute two times
(string) $response->getBody()after the first attempt the body is an empty string.Let me know what you think.
Thank you.