bug #38378 [HttpClient] Fix exception triggered with AsyncResponse (jderusse)

nicolas-grekas · nicolas-grekas · commit 1f7b8fdf7bd5 · 2020-10-02T15:57:56.000+02:00
This PR was merged into the 5.2-dev branch. Discussion ---------- [HttpClient] Fix exception triggered with AsyncResponse | Q | A | ------------- | --- | Branch? | master | Bug fix? | yes | New feature? | no | Deprecations? | no | Tickets | #38289 (comment) | License | MIT | Doc PR | / When a request is replayed with the AsyncResponse and the 2nd request failed, users get an exception even when they pass `$throw=false`. In fact, there are 2 exceptions: 1. Cannot get the content of the response twice: buffering is disabled. https://github.com/symfony/symfony/blob/73ca97c97ab4c4fc9a7bca5fb4339fca22e0cfa1/src/Symfony/Component/HttpClient/Response/CommonResponseTrait.php#L51-L68 Because CommonResponseTrait::$content is null and `self::stream` yield only the LastChunk. The content is stays null 2. HTTP/2 429 returned for "https://httpbin.org/status/429" https://github.com/symfony/symfony/blob/73ca97c97ab4c4fc9a7bca5fb4339fca22e0cfa1/src/Symfony/Component/HttpClient/Response/AsyncResponse.php#L209-L225 Because on the second request passthru is null, it didn't disable the the exception thrown on destruct for the wrapped response. This Commits ------- 3aedb51 [HttpClient] Fix exception triggered with AsyncResponse
diff --git a/src/Symfony/Component/HttpClient/Response/AsyncResponse.php b/src/Symfony/Component/HttpClient/Response/AsyncResponse.php
@@ -206,6 +206,11 @@ public static function stream(iterable $responses, float $timeout = null, string
             foreach ($client->stream($wrappedResponses, $timeout) as $response => $chunk) {
                 $r = $asyncMap[$response];
 
+                if (null === $chunk->getError() && $chunk->isFirst()) {
+                    // Ensure no exception is thrown on destruct for the wrapped response
+                    $r->response->getStatusCode();
+                }
+
                 if (!$r->passthru) {
                     if (null !== $chunk->getError() || $chunk->isLast()) {
                         unset($asyncMap[$response]);
@@ -219,11 +224,6 @@ public static function stream(iterable $responses, float $timeout = null, string
                     continue;
                 }
 
-                if (null === $chunk->getError() && $chunk->isFirst()) {
-                    // Ensure no exception is thrown on destruct for the wrapped response
-                    $r->response->getStatusCode();
-                }
-
                 foreach (self::passthru($r->client, $r, $chunk, $asyncMap) as $chunk) {
                     yield $r => $chunk;
                 }
diff --git a/src/Symfony/Component/HttpClient/Tests/AsyncDecoratorTraitTest.php b/src/Symfony/Component/HttpClient/Tests/AsyncDecoratorTraitTest.php
@@ -62,6 +62,22 @@ public function testRetry404()
         $this->assertSame(200, $response->getStatusCode());
     }
 
+    public function testRetry404WithThrow()
+    {
+        $client = $this->getHttpClient(__FUNCTION__, function (ChunkInterface $chunk, AsyncContext $context) {
+            $this->assertTrue($chunk->isFirst());
+            $this->assertSame(404, $context->getStatusCode());
+            $context->getResponse()->cancel();
+            $context->replaceRequest('GET', 'http://localhost:8057/404');
+            $context->passthru();
+        });
+
+        $response = $client->request('GET', 'http://localhost:8057/404');
+
+        $this->expectException(ClientExceptionInterface::class);
+        $response->getContent(true);
+    }
+
     public function testRetryTransportError()
     {
         $client = $this->getHttpClient(__FUNCTION__, function (ChunkInterface $chunk, AsyncContext $context) {
