Skip to content

[HttpClient] Calling $httpClient->request(...) can result in unexpected exceptions #35458

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Closed
mleczakm opened this issue Jan 24, 2020 · 3 comments
Closed

[HttpClient] Calling $httpClient->request(...) can result in unexpected exceptions #35458

mleczakm opened this issue Jan 24, 2020 · 3 comments
Labels
Bug HttpClient Status: Needs Review

Comments

@mleczakm
Copy link
Contributor

mleczakm commented Jan 24, 2020 •
edited
Loading

Symfony version(s) affected: Probably 4.4 / 5.0

Description
Calling $httpClient->request(...) (using Curl client) results in immediately destroyed CurlResponse object, which calls during destroying checkStatusCode method from ResponseTrait if no $this->info['error'] is set. Problem is that when request timed out over max_duration, $this->info['error'] can be empty (error message is shown in debug IIRC) and checking for status code throws exception.

How to reproduce

// create Symfony/HttpClient with max_duration, expect unexpected exceptions thrown from `doDestruct` method of `ResponseTrait`
$httpClient->request('HEAD', 'some-timing-out-url');

Possible Solution
Do not check status code in doDestruct method, fix empty $info['error'] when request timed out over max_duration.
@nicolas-grekas
Copy link
Member

when request timed out over max_duration, $this->info['error'] can be empty

Can you provide a reproducer that would lead to an empty message?
The rest of your description is by design: the destructor throws when the status code is unchecked, check https://symfony.com/doc/current/components/http_client.html

@mleczakm
Copy link
Contributor Author

So I would consider changing this design, because there are situations when some action need to be ended and i shouldn't need to care if there are somewhere unacked responses. I will try to reproduce this missing error message ASAP, because it breaks also our custom progress logger - after changing to max_duration timeout errors are no longer logged.

@mleczakm
Copy link
Contributor Author

Unfortunately, cancelling request in TraceableHttpClient results in another bug - request.CRITICAL: Uncaught PHP Exception ErrorException: "Notice: Undefined index: http_method" at /var/www/html/vendor/symfony/http-client/DataCollector/HttpClientDataCollector.php line 119.

@nicolas-grekas nicolas-grekas mentioned this issue Feb 2, 2020
Merged
nicolas-grekas added a commit that referenced this issue Feb 3, 2020
@nicolas-grekas
bug #35562 [HttpClient] fix HttpClientDataCollector when handling can…
a2d6d11 
…celed responses (nicolas-grekas)

This PR was merged into the 4.4 branch.

Discussion
----------

[HttpClient] fix HttpClientDataCollector when handling canceled responses

| Q             | A
| ------------- | ---
| Branch?       | 4.4
| Bug fix?      | yes
| New feature?  | no
| Deprecations? | no
| Tickets       | Fix #35458
| License       | MIT
| Doc PR        | -

Commits
-------

303f9e5 [HttpClient] fix HttpClientDataCollector when handling canceled responses
@stevro stevro mentioned this issue Feb 18, 2020
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Bug HttpClient Status: Needs Review
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
4 participants
@nicolas-grekas @xabbuh @mleczakm @carsonbot