Skip to content

Commit 2e66085

Browse files
jderussenicolas-grekas
authored andcommitted
Fix content swallowed by AsyncClient initializer
1 parent dda8496 commit 2e66085

File tree

4 files changed

+82
-8
lines changed

4 files changed

+82
-8
lines changed

Response/AsyncResponse.php

Lines changed: 35 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
namespace Symfony\Component\HttpClient\Response;
1313

1414
use Symfony\Component\HttpClient\Chunk\ErrorChunk;
15+
use Symfony\Component\HttpClient\Chunk\FirstChunk;
1516
use Symfony\Component\HttpClient\Chunk\LastChunk;
1617
use Symfony\Component\HttpClient\Exception\TransportException;
1718
use Symfony\Contracts\HttpClient\ChunkInterface;
@@ -34,6 +35,7 @@ final class AsyncResponse implements ResponseInterface, StreamableInterface
3435
private $response;
3536
private $info = ['canceled' => false];
3637
private $passthru;
38+
private $stream;
3739
private $lastYielded = false;
3840

3941
/**
@@ -226,6 +228,19 @@ public static function stream(iterable $responses, float $timeout = null, string
226228

227229
$asyncMap[$r->response] = $r;
228230
$wrappedResponses[] = $r->response;
231+
232+
if ($r->stream) {
233+
yield from self::passthruStream($response = $r->response, $r, new FirstChunk(), $asyncMap);
234+
235+
if (!isset($asyncMap[$response])) {
236+
array_pop($wrappedResponses);
237+
}
238+
239+
if ($r->response !== $response && !isset($asyncMap[$r->response])) {
240+
$asyncMap[$r->response] = $r;
241+
$wrappedResponses[] = $r->response;
242+
}
243+
}
229244
}
230245

231246
if (!$client) {
@@ -286,6 +301,7 @@ public static function stream(iterable $responses, float $timeout = null, string
286301

287302
private static function passthru(HttpClientInterface $client, self $r, ChunkInterface $chunk, \SplObjectStorage $asyncMap = null): \Generator
288303
{
304+
$r->stream = null;
289305
$response = $r->response;
290306
$context = new AsyncContext($r->passthru, $client, $r->response, $r->info, $r->content, $r->offset);
291307
if (null === $stream = ($r->passthru)($chunk, $context)) {
@@ -295,32 +311,39 @@ private static function passthru(HttpClientInterface $client, self $r, ChunkInte
295311

296312
return;
297313
}
298-
$chunk = null;
299314

300315
if (!$stream instanceof \Iterator) {
301316
throw new \LogicException(sprintf('A chunk passthru must return an "Iterator", "%s" returned.', get_debug_type($stream)));
302317
}
318+
$r->stream = $stream;
319+
320+
yield from self::passthruStream($response, $r, null, $asyncMap);
321+
}
303322

323+
private static function passthruStream(ResponseInterface $response, self $r, ?ChunkInterface $chunk, ?\SplObjectStorage $asyncMap): \Generator
324+
{
304325
while (true) {
305326
try {
306-
if (null !== $chunk) {
307-
$stream->next();
327+
if (null !== $chunk && $r->stream) {
328+
$r->stream->next();
308329
}
309330

310-
if (!$stream->valid()) {
331+
if (!$r->stream || !$r->stream->valid() || !$r->stream) {
332+
$r->stream = null;
311333
break;
312334
}
313335
} catch (\Throwable $e) {
336+
unset($asyncMap[$response]);
337+
$r->stream = null;
314338
$r->info['error'] = $e->getMessage();
315339
$r->response->cancel();
316340

317341
yield $r => $chunk = new ErrorChunk($r->offset, $e);
318342
$chunk->didThrow() ?: $chunk->getContent();
319-
unset($asyncMap[$response]);
320343
break;
321344
}
322345

323-
$chunk = $stream->current();
346+
$chunk = $r->stream->current();
324347

325348
if (!$chunk instanceof ChunkInterface) {
326349
throw new \LogicException(sprintf('A chunk passthru must yield instances of "%s", "%s" yielded.', ChunkInterface::class, get_debug_type($chunk)));
@@ -356,6 +379,12 @@ private static function passthru(HttpClientInterface $client, self $r, ChunkInte
356379
}
357380
}
358381

382+
if (null !== $chunk->getError() || $chunk->isLast()) {
383+
$stream = $r->stream;
384+
$r->stream = null;
385+
unset($asyncMap[$response]);
386+
}
387+
359388
if (null === $chunk->getError()) {
360389
$r->offset += \strlen($content);
361390

@@ -387,7 +416,6 @@ private static function passthru(HttpClientInterface $client, self $r, ChunkInte
387416
$chunk->didThrow() ?: $chunk->getContent();
388417
}
389418

390-
unset($asyncMap[$response]);
391419
break;
392420
}
393421
}

RetryableHttpClient.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -127,7 +127,7 @@ public function request(string $method, string $url, array $options = []): Respo
127127

128128
$context->getResponse()->cancel();
129129

130-
$delay = $this->getDelayFromHeader($context->getHeaders()) ?? $this->strategy->getDelay($context, $chunk instanceof LastChunk ? $content : null, $exception);
130+
$delay = $this->getDelayFromHeader($context->getHeaders()) ?? $this->strategy->getDelay($context, $chunk->isLast() ? $content : null, $exception);
131131
++$retryCount;
132132

133133
$this->logger->info('Try #{count} after {delay}ms'.($exception ? ': '.$exception->getMessage() : ', status code: '.$context->getStatusCode()), [

Tests/AsyncDecoratorTraitTest.php

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -282,4 +282,25 @@ public function testInfoPassToDecorator()
282282
$this->assertSame('test', $lastInfo['foo']);
283283
$this->assertArrayHasKey('previous_info', $lastInfo);
284284
}
285+
286+
public function testMultipleYieldInInitializer()
287+
{
288+
$first = null;
289+
$client = $this->getHttpClient(__FUNCTION__, function (ChunkInterface $chunk, AsyncContext $context) use (&$first) {
290+
if ($chunk->isFirst()) {
291+
$first = $chunk;
292+
293+
return;
294+
}
295+
$context->passthru();
296+
yield $first;
297+
yield $context->createChunk('injectedFoo');
298+
yield $chunk;
299+
});
300+
301+
$response = $client->request('GET', 'http://localhost:8057/404', ['timeout' => 0.1]);
302+
303+
$this->assertSame(404, $response->getStatusCode());
304+
$this->assertStringContainsString('injectedFoo', $response->getContent(false));
305+
}
285306
}

Tests/RetryableHttpClientTest.php

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -68,6 +68,31 @@ public function shouldRetry(AsyncContext $context, ?string $responseContent, ?Tr
6868
self::assertSame(200, $response->getStatusCode());
6969
}
7070

71+
public function testRetryWithBodyKeepContent()
72+
{
73+
$client = new RetryableHttpClient(
74+
new MockHttpClient([
75+
new MockResponse('my bad', ['http_code' => 400]),
76+
]),
77+
new class([400], 0) extends GenericRetryStrategy {
78+
public function shouldRetry(AsyncContext $context, ?string $responseContent, ?TransportExceptionInterface $exception): ?bool
79+
{
80+
if (null === $responseContent) {
81+
return null;
82+
}
83+
84+
return 'my bad' !== $responseContent;
85+
}
86+
},
87+
1
88+
);
89+
90+
$response = $client->request('GET', 'http://example.com/foo-bar');
91+
92+
self::assertSame(400, $response->getStatusCode());
93+
self::assertSame('my bad', $response->getContent(false));
94+
}
95+
7196
public function testRetryWithBodyInvalid()
7297
{
7398
$client = new RetryableHttpClient(

0 commit comments

Comments
 (0)