bug #36766 [HttpClient] add TimeoutExceptionInterface (nicolas-grekas)

This PR was merged into the 5.1-dev branch.

Discussion
----------

[HttpClient] add TimeoutExceptionInterface

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

This is fixing a design issue: unlike any other `TransportExceptionInterface`, timeouts are not fatal transport errors. For this reason, we must allow one to identify them.

~This PR also marks exception classes as `@internal`, to encourage ppl to catch them using their interface and not their class name.~ this would revert #30549

Commits
-------

ab8eca0ef6 [HttpClient] add TimeoutExceptionInterface
This commit is contained in:
Fabien Potencier 2020-05-10 08:03:39 +02:00
commit 3acc28f7e3
8 changed files with 79 additions and 36 deletions

View File

@ -11,6 +11,7 @@
namespace Symfony\Component\HttpClient\Chunk;
use Symfony\Component\HttpClient\Exception\TimeoutException;
use Symfony\Component\HttpClient\Exception\TransportException;
use Symfony\Contracts\HttpClient\ChunkInterface;
@ -61,7 +62,7 @@ class ErrorChunk implements ChunkInterface
public function isFirst(): bool
{
$this->didThrow = true;
throw new TransportException($this->errorMessage, 0, $this->error);
throw null !== $this->error ? new TransportException($this->errorMessage, 0, $this->error) : new TimeoutException($this->errorMessage);
}
/**
@ -70,7 +71,7 @@ class ErrorChunk implements ChunkInterface
public function isLast(): bool
{
$this->didThrow = true;
throw new TransportException($this->errorMessage, 0, $this->error);
throw null !== $this->error ? new TransportException($this->errorMessage, 0, $this->error) : new TimeoutException($this->errorMessage);
}
/**
@ -79,7 +80,7 @@ class ErrorChunk implements ChunkInterface
public function getInformationalStatus(): ?array
{
$this->didThrow = true;
throw new TransportException($this->errorMessage, 0, $this->error);
throw null !== $this->error ? new TransportException($this->errorMessage, 0, $this->error) : new TimeoutException($this->errorMessage);
}
/**
@ -88,7 +89,7 @@ class ErrorChunk implements ChunkInterface
public function getContent(): string
{
$this->didThrow = true;
throw new TransportException($this->errorMessage, 0, $this->error);
throw null !== $this->error ? new TransportException($this->errorMessage, 0, $this->error) : new TimeoutException($this->errorMessage);
}
/**
@ -119,7 +120,7 @@ class ErrorChunk implements ChunkInterface
{
if (!$this->didThrow) {
$this->didThrow = true;
throw new TransportException($this->errorMessage, 0, $this->error);
throw null !== $this->error ? new TransportException($this->errorMessage, 0, $this->error) : new TimeoutException($this->errorMessage);
}
}
}

View File

@ -0,0 +1,21 @@
<?php
/*
* This file is part of the Symfony package.
*
* (c) Fabien Potencier <fabien@symfony.com>
*
* For the full copyright and license information, please view the LICENSE
* file that was distributed with this source code.
*/
namespace Symfony\Component\HttpClient\Exception;
use Symfony\Contracts\HttpClient\Exception\TimeoutExceptionInterface;
/**
* @author Nicolas Grekas <p@tchwork.com>
*/
final class TimeoutException extends TransportException implements TimeoutExceptionInterface
{
}

View File

@ -16,6 +16,6 @@ use Symfony\Contracts\HttpClient\Exception\TransportExceptionInterface;
/**
* @author Nicolas Grekas <p@tchwork.com>
*/
final class TransportException extends \RuntimeException implements TransportExceptionInterface
class TransportException extends \RuntimeException implements TransportExceptionInterface
{
}

View File

@ -12,7 +12,6 @@
namespace Symfony\Component\HttpClient\Tests;
use Symfony\Component\HttpClient\Exception\ClientException;
use Symfony\Component\HttpClient\Exception\TransportException;
use Symfony\Component\HttpClient\Response\StreamWrapper;
use Symfony\Component\Process\Exception\ProcessFailedException;
use Symfony\Component\Process\Process;
@ -105,32 +104,6 @@ abstract class HttpClientTestCase extends BaseHttpClientTestCase
$this->assertTrue(feof($stream));
}
public function testTimeoutIsNotAFatalError()
{
$client = $this->getHttpClient(__FUNCTION__);
$response = $client->request('GET', 'http://localhost:8057/timeout-body', [
'timeout' => 0.25,
]);
try {
$response->getContent();
$this->fail(TransportException::class.' expected');
} catch (TransportException $e) {
}
for ($i = 0; $i < 10; ++$i) {
try {
$this->assertSame('<1><2>', $response->getContent());
break;
} catch (TransportException $e) {
}
}
if (10 === $i) {
throw $e;
}
}
public function testResponseStreamRewind()
{
$client = $this->getHttpClient(__FUNCTION__);

View File

@ -23,7 +23,7 @@
"require": {
"php": "^7.2.5",
"psr/log": "^1.0",
"symfony/http-client-contracts": "^1.1.8|^2",
"symfony/http-client-contracts": "^2.1.1",
"symfony/polyfill-php73": "^1.11",
"symfony/polyfill-php80": "^1.15",
"symfony/service-contracts": "^1.0|^2"

View File

@ -0,0 +1,21 @@
<?php
/*
* This file is part of the Symfony package.
*
* (c) Fabien Potencier <fabien@symfony.com>
*
* For the full copyright and license information, please view the LICENSE
* file that was distributed with this source code.
*/
namespace Symfony\Contracts\HttpClient\Exception;
/**
* When an idle timeout occurs.
*
* @author Nicolas Grekas <p@tchwork.com>
*/
interface TimeoutExceptionInterface extends TransportExceptionInterface
{
}

View File

@ -116,7 +116,7 @@ switch ($vars['REQUEST_URI']) {
echo '<1>';
@ob_flush();
flush();
usleep(500000);
usleep(600000);
echo '<2>';
exit;

View File

@ -14,6 +14,7 @@ namespace Symfony\Contracts\HttpClient\Test;
use PHPUnit\Framework\TestCase;
use Symfony\Contracts\HttpClient\Exception\ClientExceptionInterface;
use Symfony\Contracts\HttpClient\Exception\RedirectionExceptionInterface;
use Symfony\Contracts\HttpClient\Exception\TimeoutExceptionInterface;
use Symfony\Contracts\HttpClient\Exception\TransportExceptionInterface;
use Symfony\Contracts\HttpClient\HttpClientInterface;
@ -739,9 +740,35 @@ abstract class HttpClientTestCase extends TestCase
$response->getHeaders();
}
public function testTimeoutOnStream()
public function testTimeoutIsNotAFatalError()
{
usleep(300000); // wait for the previous test to release the server
$client = $this->getHttpClient(__FUNCTION__);
$response = $client->request('GET', 'http://localhost:8057/timeout-body', [
'timeout' => 0.3,
]);
try {
$response->getContent();
$this->fail(TimeoutExceptionInterface::class.' expected');
} catch (TimeoutExceptionInterface $e) {
}
for ($i = 0; $i < 10; ++$i) {
try {
$this->assertSame('<1><2>', $response->getContent());
break;
} catch (TimeoutExceptionInterface $e) {
}
}
if (10 === $i) {
throw $e;
}
}
public function testTimeoutOnStream()
{
$client = $this->getHttpClient(__FUNCTION__);
$response = $client->request('GET', 'http://localhost:8057/timeout-body');