bug #41656 [HttpClient] throw exception when AsyncDecoratorTrait gets an already consumed response (nicolas-grekas)
This PR was merged into the 5.2 branch.
Discussion
----------
[HttpClient] throw exception when AsyncDecoratorTrait gets an already consumed response
| Q | A
| ------------- | ---
| Branch? | 5.2
| Bug fix? | yes
| New feature? | no
| Deprecations? | no
| Tickets | Fix #40639
| License | MIT
| Doc PR | -
Commits
-------
38dae138fd
[HttpClient] throw exception when AsyncDecoratorTrait gets an already consumed response
This commit is contained in:
commit
5b21ce2642
@ -31,12 +31,15 @@ final class AsyncResponse implements ResponseInterface, StreamableInterface
|
|||||||
{
|
{
|
||||||
use CommonResponseTrait;
|
use CommonResponseTrait;
|
||||||
|
|
||||||
|
private const FIRST_CHUNK_YIELDED = 1;
|
||||||
|
private const LAST_CHUNK_YIELDED = 2;
|
||||||
|
|
||||||
private $client;
|
private $client;
|
||||||
private $response;
|
private $response;
|
||||||
private $info = ['canceled' => false];
|
private $info = ['canceled' => false];
|
||||||
private $passthru;
|
private $passthru;
|
||||||
private $stream;
|
private $stream;
|
||||||
private $lastYielded = false;
|
private $yieldedState;
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* @param ?callable(ChunkInterface, AsyncContext): ?\Iterator $passthru
|
* @param ?callable(ChunkInterface, AsyncContext): ?\Iterator $passthru
|
||||||
@ -272,6 +275,14 @@ final class AsyncResponse implements ResponseInterface, StreamableInterface
|
|||||||
continue;
|
continue;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
if (null !== $chunk->getError()) {
|
||||||
|
// no-op
|
||||||
|
} elseif ($chunk->isFirst()) {
|
||||||
|
$r->yieldedState = self::FIRST_CHUNK_YIELDED;
|
||||||
|
} elseif (self::FIRST_CHUNK_YIELDED !== $r->yieldedState && null === $chunk->getInformationalStatus()) {
|
||||||
|
throw new \LogicException(sprintf('Instance of "%s" is already consumed and cannot be managed by "%s". A decorated client should not call any of the response\'s methods in its "request()" method.', get_debug_type($response), $class ?? static::class));
|
||||||
|
}
|
||||||
|
|
||||||
foreach (self::passthru($r->client, $r, $chunk, $asyncMap) as $chunk) {
|
foreach (self::passthru($r->client, $r, $chunk, $asyncMap) as $chunk) {
|
||||||
yield $r => $chunk;
|
yield $r => $chunk;
|
||||||
}
|
}
|
||||||
@ -282,9 +293,9 @@ final class AsyncResponse implements ResponseInterface, StreamableInterface
|
|||||||
}
|
}
|
||||||
|
|
||||||
if (null === $chunk->getError() && $chunk->isLast()) {
|
if (null === $chunk->getError() && $chunk->isLast()) {
|
||||||
$r->lastYielded = true;
|
$r->yieldedState = self::LAST_CHUNK_YIELDED;
|
||||||
}
|
}
|
||||||
if (null === $chunk->getError() && !$r->lastYielded && $r->response === $response && null !== $r->client) {
|
if (null === $chunk->getError() && self::LAST_CHUNK_YIELDED !== $r->yieldedState && $r->response === $response && null !== $r->client) {
|
||||||
throw new \LogicException('A chunk passthru must yield an "isLast()" chunk before ending a stream.');
|
throw new \LogicException('A chunk passthru must yield an "isLast()" chunk before ending a stream.');
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@ -19,10 +19,11 @@ use Symfony\Contracts\HttpClient\Exception\ClientExceptionInterface;
|
|||||||
use Symfony\Contracts\HttpClient\Exception\TransportExceptionInterface;
|
use Symfony\Contracts\HttpClient\Exception\TransportExceptionInterface;
|
||||||
use Symfony\Contracts\HttpClient\HttpClientInterface;
|
use Symfony\Contracts\HttpClient\HttpClientInterface;
|
||||||
use Symfony\Contracts\HttpClient\ResponseInterface;
|
use Symfony\Contracts\HttpClient\ResponseInterface;
|
||||||
|
use Symfony\Contracts\HttpClient\ResponseStreamInterface;
|
||||||
|
|
||||||
class AsyncDecoratorTraitTest extends NativeHttpClientTest
|
class AsyncDecoratorTraitTest extends NativeHttpClientTest
|
||||||
{
|
{
|
||||||
protected function getHttpClient(string $testCase, \Closure $chunkFilter = null): HttpClientInterface
|
protected function getHttpClient(string $testCase, \Closure $chunkFilter = null, HttpClientInterface $decoratedClient = null): HttpClientInterface
|
||||||
{
|
{
|
||||||
if ('testHandleIsRemovedOnException' === $testCase) {
|
if ('testHandleIsRemovedOnException' === $testCase) {
|
||||||
$this->markTestSkipped("AsyncDecoratorTrait doesn't cache handles");
|
$this->markTestSkipped("AsyncDecoratorTrait doesn't cache handles");
|
||||||
@ -30,7 +31,7 @@ class AsyncDecoratorTraitTest extends NativeHttpClientTest
|
|||||||
|
|
||||||
$chunkFilter = $chunkFilter ?? static function (ChunkInterface $chunk, AsyncContext $context) { yield $chunk; };
|
$chunkFilter = $chunkFilter ?? static function (ChunkInterface $chunk, AsyncContext $context) { yield $chunk; };
|
||||||
|
|
||||||
return new class(parent::getHttpClient($testCase), $chunkFilter) implements HttpClientInterface {
|
return new class($decoratedClient ?? parent::getHttpClient($testCase), $chunkFilter) implements HttpClientInterface {
|
||||||
use AsyncDecoratorTrait;
|
use AsyncDecoratorTrait;
|
||||||
|
|
||||||
private $chunkFilter;
|
private $chunkFilter;
|
||||||
@ -303,4 +304,30 @@ class AsyncDecoratorTraitTest extends NativeHttpClientTest
|
|||||||
$this->assertSame(404, $response->getStatusCode());
|
$this->assertSame(404, $response->getStatusCode());
|
||||||
$this->assertStringContainsString('injectedFoo', $response->getContent(false));
|
$this->assertStringContainsString('injectedFoo', $response->getContent(false));
|
||||||
}
|
}
|
||||||
|
|
||||||
|
public function testConsumingDecoratedClient()
|
||||||
|
{
|
||||||
|
$client = $this->getHttpClient(__FUNCTION__, null, new class(parent::getHttpClient(__FUNCTION__)) implements HttpClientInterface {
|
||||||
|
use AsyncDecoratorTrait;
|
||||||
|
|
||||||
|
public function request(string $method, string $url, array $options = []): ResponseInterface
|
||||||
|
{
|
||||||
|
$response = $this->client->request($method, $url, $options);
|
||||||
|
$response->getStatusCode(); // should be avoided and breaks compatibility with AsyncDecoratorTrait
|
||||||
|
|
||||||
|
return $response;
|
||||||
|
}
|
||||||
|
|
||||||
|
public function stream($responses, float $timeout = null): ResponseStreamInterface
|
||||||
|
{
|
||||||
|
return $this->client->stream($responses, $timeout);
|
||||||
|
}
|
||||||
|
});
|
||||||
|
|
||||||
|
$response = $client->request('GET', 'http://localhost:8057/');
|
||||||
|
|
||||||
|
$this->expectException(\LogicException::class);
|
||||||
|
$this->expectExceptionMessage('Instance of "Symfony\Component\HttpClient\Response\NativeResponse" is already consumed and cannot be managed by "Symfony\Component\HttpClient\Response\AsyncResponse". A decorated client should not call any of the response\'s methods in its "request()" method.');
|
||||||
|
$response->getStatusCode();
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
Reference in New Issue
Block a user