Skip to content

Commit d50fbf6

Browse files
bug symfony#47879 [HttpClient] Fix buffering after calling AsyncContext::passthru() (nicolas-grekas, lubo13)
This PR was merged into the 5.4 branch. Discussion ---------- [HttpClient] Fix buffering after calling AsyncContext::passthru() | Q | A | ------------- | --- | Branch? | 5.4 | Bug fix? | yes | New feature? | no | Deprecations? | no | Tickets | - | License | MIT | Doc PR | - Hi, guys, I think there is an issue with the RetryableHttpClient, let me explain to you what I've seen. 1. Request is executed into AsyncResponse::__construct 2. The initializer Closure executes AsyncResponse::passthru, there the AsyncContext and a new stream are created 3. 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. 4. This leads the response body stream to be unseekable because the ```$r->openBuffer()``` is not called. The content is left with a value ```null``` and when ```AsyncResponse->toStream()``` is called from ```Psr18Client::110``` the content with value null is bound to the ```content of StreamWrapper```. 5. 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)``` onto ```Psr18Client::121``` 6. The 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. Commits ------- b559f80 [HttpClient] Add test case for seeking into the content of RetryableHttpClient responses c1da0eb [HttpClient] Fix buffering after calling AsyncContext::passthru()
2 parents af6a777 + b559f80 commit d50fbf6

File tree

2 files changed

+25
-1
lines changed

2 files changed

+25
-1
lines changed

src/Symfony/Component/HttpClient/Response/AsyncContext.php

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -181,9 +181,15 @@ public function replaceResponse(ResponseInterface $response): ResponseInterface
181181

182182
/**
183183
* Replaces or removes the chunk filter iterator.
184+
*
185+
* @param ?callable(ChunkInterface, self): ?\Iterator $passthru
184186
*/
185187
public function passthru(callable $passthru = null): void
186188
{
187-
$this->passthru = $passthru;
189+
$this->passthru = $passthru ?? static function ($chunk, $context) {
190+
$context->passthru = null;
191+
192+
yield $chunk;
193+
};
188194
}
189195
}

src/Symfony/Component/HttpClient/Tests/RetryableHttpClientTest.php

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -225,4 +225,22 @@ public function log($level, $message, array $context = []): void
225225
$this->assertNotNull($delay);
226226
$this->assertSame((int) ($retryAfter * 1000), $delay);
227227
}
228+
229+
public function testRetryOnErrorAssertContent()
230+
{
231+
$client = new RetryableHttpClient(
232+
new MockHttpClient([
233+
new MockResponse('', ['http_code' => 500]),
234+
new MockResponse('Test out content', ['http_code' => 200]),
235+
]),
236+
new GenericRetryStrategy([500], 0),
237+
1
238+
);
239+
240+
$response = $client->request('GET', 'http://example.com/foo-bar');
241+
242+
self::assertSame(200, $response->getStatusCode());
243+
self::assertSame('Test out content', $response->getContent());
244+
self::assertSame('Test out content', $response->getContent(), 'Content should be buffered');
245+
}
228246
}

0 commit comments

Comments
 (0)