8000 [HttpClient] Fix handling timeouts when responses are destructed · symfony/symfony@8d2c718 · GitHub
[go: up one dir, main page]
Skip to content

Commit 8d2c718

Browse files
nicolas-grekasnicolas-grekas
committed
[HttpClient] Fix handling timeouts when responses are destructed
1 parent 2ed7672 commit 8d2c718

File tree

8 files changed

+69
-7
lines changed

8 files changed

+69
-7
lines changed

‎src/Symfony/Component/HttpClient/Internal/ClientState.php

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,4 +22,5 @@ class ClientState
2222
{
2323
public $handlesActivity = [];
2424
public $openHandles = [];
25+
public $lastTimeout;
2526
}

‎src/Symfony/Component/HttpClient/Response/CurlResponse.php

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,6 @@ final class CurlResponse implements ResponseInterface
3232
}
3333

3434
private static $performing = false;
35-
private $multi;
3635
private $debugBuffer;
3736

3837
/**

‎src/Symfony/Component/HttpClient/Response/NativeResponse.php

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,6 @@ final class NativeResponse implements ResponseInterface
3434
private $onProgress;
3535
private $remaining;
3636
private $buffer;
37-
private $multi;
3837
private $debugBuffer;
3938
private $shouldBuffer;
4039

‎src/Symfony/Component/HttpClient/Response/ResponseTrait.php

Lines changed: 18 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -61,6 +61,8 @@ trait ResponseTrait
6161
private $finalInfo;
6262
private $offset = 0;
6363
private $jsonData;
64+
/** @var ClientState|null */
65+
private $multi = null;
6466

6567
/**
6668
* {@inheritdoc}
@@ -233,15 +235,15 @@ abstract protected static function perform(ClientState $multi, array &$responses
233235
*/
234236
abstract protected static function select(ClientState $multi, float $timeout): int;
235237

236-
private static function initialize(self $response): void
238+
private static function initialize(self $response, float $timeout = null): void
237239
{
238240
if (null !== $response->info['error']) {
239241
throw new TransportException($response->info['error']);
240242
}
241243

242244
try {
243-
if (($response->initializer)($response)) {
244-
foreach (self::stream([$response]) as $chunk) {
245+
if (($response->initializer)($response, $timeout)) {
246+
foreach (self::stream([$response], $timeout) as $chunk) {
245247
if ($chunk->isFirst()) {
246248
break;
247249
}
@@ -304,7 +306,7 @@ private function doDestruct()
304306
$this->shouldBuffer = true;
305307

306308
if ($this->initializer && null === $this->info['error']) {
307-
self::initialize($this);
309+
self::initialize($this, -0.0);
308310
$this->checkStatusCode();
309311
}
310312
}
@@ -325,6 +327,12 @@ public static function stream(iterable $responses, float $timeout = null): \Gene
325327
$lastActivity = microtime(true);
326328
$elapsedTimeout = 0;
327329

330+
if ($fromLastTimeout = 0.0 === $timeout && '-0' === (string) $timeout) {
331+
$timeout = null;
332+
} elseif ($fromLastTimeout = 0 > $timeout) {
333+
$timeout = -$timeout;
334+
}
335+
328336
while (true) {
329337
$hasActivity = false;
330338
$timeoutMax = 0;
@@ -340,13 +348,18 @@ public static function stream(iterable $responses, float $timeout = null): \Gene
340348
$timeoutMin = min($timeoutMin, $response->timeout, 1);
341349
$chunk = false;
342350

351+
if ($fromLastTimeout && null !== $multi->lastTimeout) {
352+
$elapsedTimeout = microtime(true) - $multi->lastTimeout;
353+
}
354+
343355
if (isset($multi->handlesActivity[$j])) {
344-
// no-op
356+
$multi->lastTimeout = null;
345357
} elseif (!isset($multi->openHandles[$j])) {
346358
unset($responses[$j]);
347359
continue;
348360
} elseif ($elapsedTimeout >= $timeoutMax) {
349361
$multi->handlesActivity[$j] = [new ErrorChunk($response->offset, sprintf('Idle timeout reached for "%s".', $response->getInfo('url')))];
362+
$multi->lastTimeout ?? $multi->lastTimeout = F438 $lastActivity;
350363
} else {
351364
continue;
352365
}

‎src/Symfony/Component/HttpClient/Tests/HttpClientTestCase.php

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,15 @@
1919

2020
abstract class HttpClientTestCase extends BaseHttpClientTestCase
2121
{
22+
public function testTimeoutOnDestruct()
23+
{
24+
if (!method_exists(parent::class, 'testTimeoutOnDestruct')) {
25+
$this->markTestSkipped('BaseHttpClientTestCase doesn\'t have testTimeoutOnDestruct().');
26+
}
27+
28+
parent::testTimeoutOnDestruct();
29+
}
30+
2231
public function testAcceptHeader()
2332
{
2433
$client = $this->getHttpClient(__FUNCTION__);

‎src/Symfony/Component/HttpClient/Tests/MockHttpClientTest.php

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -115,6 +115,10 @@ protected function getHttpClient(string $testCase): HttpClientInterface
115115
$this->markTestSkipped('Real transport required');
116116
break;
117117

118+
case 'testTimeoutOnDestruct':
119+
$this->markTestSkipped('Real transport required');
120+
break;
121+
118122
c 10000 ase 'testDestruct':
119123
$this->markTestSkipped("MockHttpClient doesn't timeout on destruct");
120124
break;

‎src/Symfony/Component/HttpClient/Tests/NativeHttpClientTest.php

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,4 +25,9 @@ public function testInformationalResponseStream()
2525
{
2626
$this->markTestSkipped('NativeHttpClient doesn\'t support informational status codes.');
2727
}
28+
29+
public function testTimeoutOnDestruct()
30+
{
31+
$this->markTestSkipped('NativeHttpClient doesn\'t support opening concurrent requests.');
32+
}
2833
}

‎src/Symfony/Contracts/HttpClient/Test/HttpClientTestCase.php

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -810,6 +810,38 @@ public function testTimeoutWithActiveConcurrentStream()
810810
}
811811
}
812812

813+
public function testTimeoutOnDestruct()
814+
{
815+
$p1 = TestHttpServer::start(8067);
816+
$p2 = TestHttpServer::start(8077);
817+
818+
$client = $this->getHttpClient(__FUNCTION__);
819+
$start = microtime(true);
820+
$responses = [];
821+
822+
$responses[] = $client->request('GET', 'http://localhost:8067/timeout-header', ['timeout' => 0.25]);
823+
$responses[] = $client->request('GET', 'http://localhost:8077/timeout-header', ['timeout' => 0.25]);
824+
$responses[] = $client->request('GET', 'http://localhost:8067/timeout-header', ['timeout' => 0.25]);
825+
$responses[] = $client->request('GET', 'http://localhost:8077/timeout-header', ['timeout' => 0.25]);
826+
827+
try {
828+
while ($response = array_shift($responses)) {
829+
try {
830+
unset($response);
831+
$this->fail(TransportExceptionInterface::class.' expected');
832+
} catch (TransportExceptionInterface $e) {
833+
}
834+
}
835+
836+
$duration = microtime(true) - $start;
837+
838+
$this->assertLessThan(0.75, $duration);
839+
} finally {
840+
$p1->stop();
841+
$p2->stop();
842+
}
843+
}
844+
813845
public function testDestruct()
814846
{
815847
$client = $this->getHttpClient(__FUNCTION__);

0 commit comments

Comments
 (0)
0