[HttpClient] Don't reset timeout counter when initializing requests

nicolas-grekas · nicolas-grekas · commit 8f3bdeb359c9 · 2021-12-11T17:33:30.000+01:00
diff --git a/src/Symfony/Component/HttpClient/Response/CurlResponse.php b/src/Symfony/Component/HttpClient/Response/CurlResponse.php
@@ -148,6 +148,7 @@ public function __construct(CurlClientState $multi, $ch, array $options = null,
         };
 
         // Schedule the request in a non-blocking way
+        $multi->lastTimeout = null;
         $multi->openHandles[$id] = [$ch, $options];
         curl_multi_add_handle($multi->handle, $ch);
 
diff --git a/src/Symfony/Component/HttpClient/Response/NativeResponse.php b/src/Symfony/Component/HttpClient/Response/NativeResponse.php
@@ -183,6 +183,7 @@ private function open(): void
             return;
         }
 
+        $this->multi->lastTimeout = null;
         $this->multi->openHandles[$this->id] = [$h, $this->buffer, $this->onProgress, &$this->remaining, &$this->info];
     }
 
diff --git a/src/Symfony/Component/HttpClient/Response/ResponseTrait.php b/src/Symfony/Component/HttpClient/Response/ResponseTrait.php
@@ -233,15 +233,15 @@ abstract protected static function perform(ClientState $multi, array &$responses
      */
     abstract protected static function select(ClientState $multi, float $timeout): int;
 
-    private static function initialize(self $response, float $timeout = null): void
+    private static function initialize(self $response): void
     {
         if (null !== $response->info['error']) {
             throw new TransportException($response->info['error']);
         }
 
         try {
-            if (($response->initializer)($response, $timeout)) {
-                foreach (self::stream([$response], $timeout) as $chunk) {
+            if (($response->initializer)($response, -0.0)) {
+                foreach (self::stream([$response], -0.0) as $chunk) {
                     if ($chunk->isFirst()) {
                         break;
                     }
@@ -304,7 +304,7 @@ private function doDestruct()
         $this->shouldBuffer = true;
 
         if ($this->initializer && null === $this->info['error']) {
-            self::initialize($this, -0.0);
+            self::initialize($this);
             $this->checkStatusCode();
         }
     }
diff --git a/src/Symfony/Component/HttpClient/Tests/MockHttpClientTest.php b/src/Symfony/Component/HttpClient/Tests/MockHttpClientTest.php
@@ -157,6 +157,7 @@ protected function getHttpClient(string $testCase): HttpClientInterface
                 $this->markTestSkipped('Real transport required');
                 break;
 
+            case 'testTimeoutOnInitialize':
             case 'testTimeoutOnDestruct':
                 $this->markTestSkipped('Real transport required');
                 break;
diff --git a/src/Symfony/Component/HttpClient/Tests/NativeHttpClientTest.php b/src/Symfony/Component/HttpClient/Tests/NativeHttpClientTest.php
@@ -26,6 +26,11 @@ public function testInformationalResponseStream()
         $this->markTestSkipped('NativeHttpClient doesn\'t support informational status codes.');
     }
 
+    public function testTimeoutOnInitialize()
+    {
+        $this->markTestSkipped('NativeHttpClient doesn\'t support opening concurrent requests.');
+    }
+
     public function testTimeoutOnDestruct()
     {
         $this->markTestSkipped('NativeHttpClient doesn\'t support opening concurrent requests.');
diff --git a/src/Symfony/Contracts/HttpClient/Test/HttpClientTestCase.php b/src/Symfony/Contracts/HttpClient/Test/HttpClientTestCase.php
@@ -810,6 +810,39 @@ public function testTimeoutWithActiveConcurrentStream()
         }
     }
 
+    public function testTimeoutOnInitialize()
+    {
+        $p1 = TestHttpServer::start(8067);
+        $p2 = TestHttpServer::start(8077);
+
+        $client = $this->getHttpClient(__FUNCTION__);
+        $start = microtime(true);
+        $responses = [];
+
+        $responses[] = $client->request('GET', 'http://localhost:8067/timeout-header', ['timeout' => 0.25]);
+        $responses[] = $client->request('GET', 'http://localhost:8077/timeout-header', ['timeout' => 0.25]);
+        $responses[] = $client->request('GET', 'http://localhost:8067/timeout-header', ['timeout' => 0.25]);
+        $responses[] = $client->request('GET', 'http://localhost:8077/timeout-header', ['timeout' => 0.25]);
+
+        try {
+            foreach ($responses as $response) {
+                try {
+                    $response->getContent();
+                    $this->fail(TransportExceptionInterface::class.' expected');
+                } catch (TransportExceptionInterface $e) {
+                }
+            }
+            $responses = [];
+
+            $duration = microtime(true) - $start;
+
+            $this->assertLessThan(1.0, $duration);
+        } finally {
+            $p1->stop();
+            $p2->stop();
+        }
+    }
+
     public function testTimeoutOnDestruct()
     {
         $p1 = TestHttpServer::start(8067);

Original file line number	Diff line number	Diff line change
`@@ -183,6 +183,7 @@ private function open(): void`
`183`	`183`	`return;`
`184`	`184`	`}`
`185`	`185`
	`186`	`+ $this->multi->lastTimeout = null;`
`186`	`187`	`$this->multi->openHandles[$this->id] = [$h, $this->buffer, $this->onProgress, &$this->remaining, &$this->info];`
`187`	`188`	`}`
`188`	`189`
Original file line number	Diff line number	Diff line change
`@@ -233,15 +233,15 @@ abstract protected static function perform(ClientState $multi, array &$responses`
`233`	`233`	`*/`
`234`	`234`	`abstract protected static function select(ClientState $multi, float $timeout): int;`
`235`	`235`
`236`		`- private static function initialize(self $response, float $timeout = null): void`
	`236`	`+ private static function initialize(self $response): void`
`237`	`237`	`{`
`238`	`238`	`if (null !== $response->info['error']) {`
`239`	`239`	`throw new TransportException($response->info['error']);`
`240`	`240`	`}`
`241`	`241`
`242`	`242`	`try {`
`243`		`- if (($response->initializer)($response, $timeout)) {`
`244`		`- foreach (self::stream([$response], $timeout) as $chunk) {`
	`243`	`+ if (($response->initializer)($response, -0.0)) {`
	`244`	`+ foreach (self::stream([$response], -0.0) as $chunk) {`
`245`	`245`	`if ($chunk->isFirst()) {`
`246`	`246`	`break;`
`247`	`247`	`}`
`@@ -304,7 +304,7 @@ private function doDestruct()`
`304`	`304`	`$this->shouldBuffer = true;`
`305`	`305`
`306`	`306`	`if ($this->initializer && null === $this->info['error']) {`
`307`		`- self::initialize($this, -0.0);`
	`307`	`+ self::initialize($this);`
`308`	`308`	`$this->checkStatusCode();`
`309`	`309`	`}`
`310`	`310`	`}`
Original file line number	Diff line number	Diff line change
`@@ -26,6 +26,11 @@ public function testInformationalResponseStream()`
`26`	`26`	`$this->markTestSkipped('NativeHttpClient doesn\'t support informational status codes.');`
`27`	`27`	`}`
`28`	`28`
	`29`	`+ public function testTimeoutOnInitialize()`
	`30`	`+ {`
	`31`	`+ $this->markTestSkipped('NativeHttpClient doesn\'t support opening concurrent requests.');`
	`32`	`+ }`
	`33`	`+`
`29`	`34`	`public function testTimeoutOnDestruct()`
`30`	`35`	`{`
`31`	`36`	`$this->markTestSkipped('NativeHttpClient doesn\'t support opening concurrent requests.');`