[HttpClient] Fix that staged exceptions were not thrown when calling MockResponse::getStatusCode()

[mpdude]

Q	A
Branch?	6.4
Bug fix?	yes
New feature?	no
Deprecations?	no
Issues
License	MIT

Disclaimer: I don't fully grasp all the internal workings and the async tricks the HTTP Client does, so take this with a grain of salt.

I have application code like the following:

$response = $httpClient->request('GET', $url);

if (200 !== $response->getStatusCode()) {
  // ... do something about it
}

To my understanding, conditions like e. g. a DNS resolution error will lead to a TransportException being thrown, and this may happen from getStatusCode(), since this is the first method that really has to wait for results to arrive over the network.

I'd like to write a test to make sure my code deals with the exception in an appropriate way. I have been following the examples given at https://symfony.com/doc/current/http_client.html#testing-network-transport-exceptions, and also at the definition of ExternalArticleService::createArticle() as given in https://symfony.com/doc/current/http_client.html#full-example. Note that createArticle() also accesses getStatusCode(), like my code does.

However, I could not get this test to work. Defining $httpClient either as

$httpClient = new MockHttpClient([
    new MockResponse([new \RuntimeException('Error at transport level')]),
]);

or

$httpClient = new MockHttpClient((static function (): \Generator {
    yield new TransportException('Error at transport level');
})());

did not lead to the exception being thrown when accessing $response->getStatusCode().

Digging into \Symfony\Component\HttpClient\Response\MockResponse::readResponse I noticed that the canned exception was thrown only to be caught again a few lines down, seemingly using it as a plain body part. I do not fully understand what the code is trying to do there. Maybe the code is the result of #44438 and #44568 that were merged in close timely relationship, but against different target branches (4.4 and 6.1).

I have added new tests to cover exception handling also when accessing the getStatusCode() method, with exceptions being yielded from a generator or being part of an array.

Also, I split up some test cases where – to my understanding – two different things were tested at the same time.

[mpdude]

Inviting @fancyweb since you authored the last relevant changes in that area of code and you seem to have a good understanding of what's going on.

[nicolas-grekas]

Did you try setting the "error" info when constructing the MockResponse instead?

[mpdude]

No, I was not aware of that.

I can confirm the following code throws:

        $httpClient = new MockHttpClient([
            new MockResponse(info: ['error' => 'network error']),
        ]);
        $response = $httpClient->request('GET', '/test');
        $code = $response->getStatusCode(); // throws a TransportException

So, the documentation at https://symfony.com/doc/current/http_client.html#testing-network-transport-exceptions is wrong? It suggests to do the following:

        $httpClient = new MockHttpClient([
            // You can create the exception directly in the body...
            new MockResponse([new \RuntimeException('Error at transport level')]),

            // ... or you can yield the exception from a callback
            new MockResponse((static function (): \Generator {
                yield new TransportException('Error at transport level');
            })()),
        ]);

[nicolas-grekas]

Those are mocking two different moments where exceptions can be thrown: before headers vs after them;

[nicolas-grekas]

Doc PR welcome!

[mpdude]

Docs PR at symfony/symfony-docs#19997. Please check if what I wrote is correct.