Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
bug #33643 [HttpClient] fix throwing HTTP exceptions when the 1st chu…
…nk is emitted (nicolas-grekas)

This PR was merged into the 4.3 branch.

Discussion
----------

[HttpClient] fix throwing HTTP exceptions when the 1st chunk is emitted

| Q             | A
| ------------- | ---
| Branch?       | 4.3
| Bug fix?      | yes
| New feature?  | no
| Deprecations? | no
| Tickets       | -
| License       | MIT
| Doc PR        | -

Spotted while discussing the client with @Seldaek
The current behavior is transient: depending on the speed of the network/server, the exception can be thrown, or not.

This forces one do deal with 3/4/5xx when the first chunk is yielded.

Commits
-------

3c93764 [HttpClient] fix throwing HTTP exceptions when the 1st chunk is emitted
  • Loading branch information
nicolas-grekas committed Sep 20, 2019
2 parents f550ca9 + 3c93764 commit 1575e27
Show file tree
Hide file tree
Showing 2 changed files with 25 additions and 7 deletions.
22 changes: 15 additions & 7 deletions src/Symfony/Component/HttpClient/Response/ResponseTrait.php
Expand Up @@ -317,20 +317,28 @@ public static function stream(iterable $responses, float $timeout = null): \Gene
} elseif ($chunk instanceof ErrorChunk) {
unset($responses[$j]);
$isTimeout = true;
} elseif ($chunk instanceof FirstChunk && $response->logger) {
$info = $response->getInfo();
$response->logger->info(sprintf('Response: "%s %s"', $info['http_code'], $info['url']));
} elseif ($chunk instanceof FirstChunk) {
if ($response->logger) {
$info = $response->getInfo();
$response->logger->info(sprintf('Response: "%s %s"', $info['http_code'], $info['url']));
}

yield $response => $chunk;

if ($response->initializer && null === $response->info['error']) {
// Ensure the HTTP status code is always checked
$response->getHeaders(true);
}

continue;
}

yield $response => $chunk;
}

unset($multi->handlesActivity[$j]);

if ($chunk instanceof FirstChunk && null === $response->initializer && null === $response->info['error']) {
// Ensure the HTTP status code is always checked
$response->getHeaders(true);
} elseif ($chunk instanceof ErrorChunk && !$chunk->didThrow()) {
if ($chunk instanceof ErrorChunk && !$chunk->didThrow()) {
// Ensure transport exceptions are always thrown
$chunk->getContent();
}
Expand Down
10 changes: 10 additions & 0 deletions src/Symfony/Contracts/HttpClient/Test/HttpClientTestCase.php
Expand Up @@ -152,6 +152,16 @@ public function testClientError()
$this->assertSame(404, $response->getStatusCode());
$this->assertSame(['application/json'], $response->getHeaders(false)['content-type']);
$this->assertNotEmpty($response->getContent(false));

$response = $client->request('GET', 'http://localhost:8057/404');

try {
foreach ($client->stream($response) as $chunk) {
$this->assertTrue($chunk->isFirst());
}
$this->fail(ClientExceptionInterface::class.' expected');
} catch (ClientExceptionInterface $e) {
}
}

public function testIgnoreErrors()
Expand Down

0 comments on commit 1575e27

Please sign in to comment.