[HttpClient] Asynchronous request processing breaks when some response have 403 code

[a-menshchikov]

Symfony version(s) affected: 4.4.2

Description
If I try to process a few requests asynchronously and some of them respond with 403 http code, foreach loop over stream freezes until timeout or max_duration expires.

How to reproduce
https://gist.github.com/a-menshchikov/49b04cbaf85750e2e3980e787fad882c

Additional context
php 7.4.1
curl 7.64.0-4
debian 10.1

[nicolas-grekas]

You never deal with the status code, that's why. You get caught by the police :)
You must deal with the status code when $chunk->isFirst().
Please consider sending a doc PR to explain this better.

[a-menshchikov]

@nicolas-grekas I updated gist. I'm making SplObjectStorage for collect invalid responses and don't try to process them chunks if it status code >= 400. But I have to unset response objects manually, because if I don’t do it I'm catching TransportException when execute method ends (and $ stream and $ response objects are destructed).
Tell me if I'm doing something wrong. Or it seems like a bug in destruction logic.

[nicolas-grekas]

It looks what you want is calling $response->cancel() when the status code is >=400
no need for the object storage

[a-menshchikov]

@nicolas-grekas thank you, $response->cancel() solve a half the problem.
What's about the necessary of manually destructing of response object to be able to catch Exception?

[nicolas-grekas]

On closer look, the reason is that the check for the timeout is wrong. By being after isFirst/isLast, you never reach it. A timeout is not an error, so that it doesn't interupt the response stream. If you move the isTimeout check before isFirst and "continue", the reponse will continue to proceed, until its destructor is called, which will make it wait another timeout until it gives up.
This destruction happens inside/outside the try/catch depending on the unset().

So, if you want to properly deal with that, you must first check for timeouts, then for status code, then you'll have made all required checks. Otherwise, you're caught 🚓

[a-menshchikov]

@nicolas-grekas thanks a lot!
I will make a doc PR ASAP.

[a-menshchikov]

@nicolas-grekas what branch I should use as a base for this PR?

[nicolas-grekas]

4.3 please for the doc