bug #38520 [HttpClient] Fix nesteed stream in AsyncResponse (jderusse)

nicolas-grekas · nicolas-grekas · commit 2a70f6884733 · 2020-10-12T09:57:06.000+02:00
This PR was merged into the 5.x branch. Discussion ---------- [HttpClient] Fix nesteed stream in AsyncResponse | Q | A | ------------- | --- | Branch? | 5.x | Bug fix? | yes | New feature? | no | Deprecations? | no | Tickets | #38509 | License | MIT | Doc PR | / When streaming twice (streaming inside streaming) an AsycResponse the second stream will yield the LastChunk, but the first Stream won't have access to it and ends with an exception `A chunk passthru must yield an "isLast()" chunk` This PR adds a state in AsyncRsponse to remember if the lastChunk has been yielded. Reproducer: ```php // a Simple Client that return an AsyncResponse $client = new class(HttpClient::create()) implements HttpClientInterface { use AsyncDecoratorTrait; private $client; public function __construct(HttpClientInterface $client) { $this->client = $client; } public function request(string $method, string $url, array $options = []): ResponseInterface { return new AsyncResponse($this->client, $method, $url, $options, null); } }; $response = $client->request('GET', 'https://httpbin.org/status/200'); foreach ($client->stream($response) as $chunk) { // will end in a FirstChunk <== bug foreach ($client->stream($response) as $chunk) { // This will correctly handle the LastChunk } } ``` Commits ------- 8ba54c1 Fix nesteed stream in AsyncResponse
diff --git a/src/Symfony/Component/HttpClient/Response/AsyncResponse.php b/src/Symfony/Component/HttpClient/Response/AsyncResponse.php
@@ -34,6 +34,7 @@ final class AsyncResponse implements ResponseInterface, StreamableInterface
     private $response;
     private $info = ['canceled' => false];
     private $passthru;
+    private $lastYielded = false;
 
     /**
      * @param ?callable(ChunkInterface, AsyncContext): ?\Iterator $passthru
@@ -255,7 +256,10 @@ public static function stream(iterable $responses, float $timeout = null, string
                 }
             }
 
-            if (null === $chunk->getError() && !$chunk->isLast() && $r->response === $response && null !== $r->client) {
+            if (null === $chunk->getError() && $chunk->isLast()) {
+                $r->lastYielded = true;
+            }
+            if (null === $chunk->getError() && !$r->lastYielded && $r->response === $response && null !== $r->client) {
                 throw new \LogicException('A chunk passthru must yield an "isLast()" chunk before ending a stream.');
             }
 
diff --git a/src/Symfony/Component/HttpClient/Tests/AsyncDecoratorTraitTest.php b/src/Symfony/Component/HttpClient/Tests/AsyncDecoratorTraitTest.php
@@ -230,4 +230,27 @@ public function testRetryTimeout()
 
         $this->assertSame(200, $response->getStatusCode());
     }
+
+    public function testRecurciveStream()
+    {
+        $client = new class(parent::getHttpClient(__FUNCTION__)) implements HttpClientInterface {
+            use AsyncDecoratorTrait;
+
+            public function request(string $method, string $url, array $options = []): ResponseInterface
+            {
+                return new AsyncResponse($this->client, $method, $url, $options);
+            }
+        };
+
+        $response = $client->request('GET', 'http://localhost:8057/json');
+        $content = '';
+        foreach ($client->stream($response) as $chunk) {
+            $content .= $chunk->getContent();
+            foreach ($client->stream($response) as $chunk) {
+                $content .= $chunk->getContent();
+            }
+        }
+
+        $this->assertSame('{"documents":[{"id":"\/json\/1"},{"id":"\/json\/2"},{"id":"\/json\/3"}]}', $content);
+    }
 }

Original file line number	Diff line number	Diff line change
`@@ -34,6 +34,7 @@ final class AsyncResponse implements ResponseInterface, StreamableInterface`
`34`	`34`	`private $response;`
`35`	`35`	`private $info = ['canceled' => false];`
`36`	`36`	`private $passthru;`
	`37`	`+ private $lastYielded = false;`
`37`	`38`
`38`	`39`	`/**`
`39`	`40`	`* @param ?callable(ChunkInterface, AsyncContext): ?\Iterator $passthru`
`@@ -255,7 +256,10 @@ public static function stream(iterable $responses, float $timeout = null, string`
`255`	`256`	`}`
`256`	`257`	`}`
`257`	`258`
`258`		`- if (null === $chunk->getError() && !$chunk->isLast() && $r->response === $response && null !== $r->client) {`
	`259`	`+ if (null === $chunk->getError() && $chunk->isLast()) {`
	`260`	`+ $r->lastYielded = true;`
	`261`	`+ }`
	`262`	`+ if (null === $chunk->getError() && !$r->lastYielded && $r->response === $response && null !== $r->client) {`
`259`	`263`	`throw new \LogicException('A chunk passthru must yield an "isLast()" chunk before ending a stream.');`
`260`	`264`	`}`
`261`	`265`