From e325f51fe2b36829776a4be025f42df4c202b30c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=A9r=C3=A9my=20Deruss=C3=A9?= Date: Mon, 19 Oct 2020 18:27:34 +0200 Subject: [PATCH] [HttpClient] Fix decorating progress info in AsyncResponse --- .../FrameworkExtension.php | 2 +- .../DataCollector/HttpClientDataCollector.php | 6 ++++++ .../HttpClient/Response/AsyncContext.php | 6 ++++++ .../HttpClient/Response/AsyncResponse.php | 7 +++++++ .../HttpClient/RetryableHttpClient.php | 4 +--- .../Tests/AsyncDecoratorTraitTest.php | 19 +++++++++++++++++++ 6 files changed, 40 insertions(+), 4 deletions(-) diff --git a/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/FrameworkExtension.php b/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/FrameworkExtension.php index 24cce5f9c3..e99bef78c5 100644 --- a/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/FrameworkExtension.php +++ b/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/FrameworkExtension.php @@ -2097,7 +2097,7 @@ class FrameworkExtension extends Extension $container ->register($name.'.retryable', RetryableHttpClient::class) - ->setDecoratedService($name, null, -10) // lower priority than TraceableHttpClient + ->setDecoratedService($name, null, 10) // higher priority than TraceableHttpClient ->setArguments([new Reference($name.'.retryable.inner'), $retryStrategy, $options['max_retries'], new Reference('logger')]) ->addTag('monolog.logger', ['channel' => 'http_client']); } diff --git a/src/Symfony/Component/HttpClient/DataCollector/HttpClientDataCollector.php b/src/Symfony/Component/HttpClient/DataCollector/HttpClientDataCollector.php index 7a60bdd435..db8bbbdd69 100644 --- a/src/Symfony/Component/HttpClient/DataCollector/HttpClientDataCollector.php +++ b/src/Symfony/Component/HttpClient/DataCollector/HttpClientDataCollector.php @@ -98,6 +98,7 @@ final class HttpClientDataCollector extends DataCollector implements LateDataCol $errorCount = 0; $baseInfo = [ 'response_headers' => 1, + 'retry_count' => 1, 'redirect_count' => 1, 'redirect_url' => 1, 'user_data' => 1, @@ -152,6 +153,11 @@ final class HttpClientDataCollector extends DataCollector implements LateDataCol $content = []; } + if (isset($info['retry_count'])) { + $content['retries'] = $info['previous_info']; + unset($info['previous_info']); + } + $debugInfo = array_diff_key($info, $baseInfo); $info = ['info' => $debugInfo] + array_diff_key($info, $debugInfo) + $content; unset($traces[$i]['info']); // break PHP reference used by TraceableHttpClient diff --git a/src/Symfony/Component/HttpClient/Response/AsyncContext.php b/src/Symfony/Component/HttpClient/Response/AsyncContext.php index b0138968ff..ebadd1911a 100644 --- a/src/Symfony/Component/HttpClient/Response/AsyncContext.php +++ b/src/Symfony/Component/HttpClient/Response/AsyncContext.php @@ -151,6 +151,12 @@ final class AsyncContext public function replaceRequest(string $method, string $url, array $options = []): ResponseInterface { $this->info['previous_info'][] = $this->response->getInfo(); + if (null !== $onProgress = $options['on_progress'] ?? null) { + $thisInfo = &$this->info; + $options['on_progress'] = static function (int $dlNow, int $dlSize, array $info) use (&$thisInfo, $onProgress) { + $onProgress($dlNow, $dlSize, $thisInfo + $info); + }; + } return $this->response = $this->client->request($method, $url, ['buffer' => false] + $options); } diff --git a/src/Symfony/Component/HttpClient/Response/AsyncResponse.php b/src/Symfony/Component/HttpClient/Response/AsyncResponse.php index ab9dd6495f..0eb1355efd 100644 --- a/src/Symfony/Component/HttpClient/Response/AsyncResponse.php +++ b/src/Symfony/Component/HttpClient/Response/AsyncResponse.php @@ -43,6 +43,13 @@ final class AsyncResponse implements ResponseInterface, StreamableInterface { $this->client = $client; $this->shouldBuffer = $options['buffer'] ?? true; + + if (null !== $onProgress = $options['on_progress'] ?? null) { + $thisInfo = &$this->info; + $options['on_progress'] = static function (int $dlNow, int $dlSize, array $info) use (&$thisInfo, $onProgress) { + $onProgress($dlNow, $dlSize, $thisInfo + $info); + }; + } $this->response = $client->request($method, $url, ['buffer' => false] + $options); $this->passthru = $passthru; $this->initializer = static function (self $response) { diff --git a/src/Symfony/Component/HttpClient/RetryableHttpClient.php b/src/Symfony/Component/HttpClient/RetryableHttpClient.php index 6bca91e4f1..387a33fc9b 100644 --- a/src/Symfony/Component/HttpClient/RetryableHttpClient.php +++ b/src/Symfony/Component/HttpClient/RetryableHttpClient.php @@ -66,7 +66,6 @@ class RetryableHttpClient implements HttpClientInterface } } catch (TransportExceptionInterface $exception) { // catch TransportExceptionInterface to send it to the strategy - $context->setInfo('retry_count', $retryCount); } if (null !== $exception) { // always retry request that fail to resolve DNS @@ -91,8 +90,6 @@ class RetryableHttpClient implements HttpClientInterface } } } elseif ($chunk->isFirst()) { - $context->setInfo('retry_count', $retryCount); - if (false === $shouldRetry = $this->strategy->shouldRetry($context, null, null)) { $context->passthru(); yield $chunk; @@ -138,6 +135,7 @@ class RetryableHttpClient implements HttpClientInterface 'delay' => $delay, ]); + $context->setInfo('retry_count', $retryCount); $context->replaceRequest($method, $url, $options); $context->pause($delay / 1000); diff --git a/src/Symfony/Component/HttpClient/Tests/AsyncDecoratorTraitTest.php b/src/Symfony/Component/HttpClient/Tests/AsyncDecoratorTraitTest.php index 5bbe244600..90872ece75 100644 --- a/src/Symfony/Component/HttpClient/Tests/AsyncDecoratorTraitTest.php +++ b/src/Symfony/Component/HttpClient/Tests/AsyncDecoratorTraitTest.php @@ -263,4 +263,23 @@ class AsyncDecoratorTraitTest extends NativeHttpClientTest $this->assertSame('{"documents":[{"id":"\/json\/1"},{"id":"\/json\/2"},{"id":"\/json\/3"}]}', $content); } + + public function testInfoPassToDecorator() + { + $lastInfo = null; + $options = ['on_progress' => function (int $dlNow, int $dlSize, array $info) use (&$lastInfo) { + $lastInfo = $info; + }]; + $client = $this->getHttpClient(__FUNCTION__, function (ChunkInterface $chunk, AsyncContext $context) use ($options) { + $context->setInfo('foo', 'test'); + $context->getResponse()->cancel(); + $context->replaceRequest('GET', 'http://localhost:8057/', $options); + $context->passthru(); + }); + + $client->request('GET', 'http://localhost:8057')->getContent(); + $this->assertArrayHasKey('foo', $lastInfo); + $this->assertSame('test', $lastInfo['foo']); + $this->assertArrayHasKey('previous_info', $lastInfo); + } }