8000 [HttpClient] Idle timeout reached with negative default_socket_timeout · Issue #44477 · symfony/symfony · GitHub
[go: up one dir, main page]
Skip to content

[HttpClient] Idle timeout reached with negative default_socket_timeout #44477

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
caugner opened this issue Dec 6, 2021 · 5 comments
Closed

[HttpClient] Idle timeout reached with negative default_socket_timeout #44477

caugner opened this issue Dec 6, 2021 · 5 comments
Labels
Bug HttpClient Status: Needs Review

Comments

@caugner
Copy link
Contributor
caugner commented Dec 6, 2021 •
edited
Loading

Symfony version(s) affected

5.4.0

Description

We have set PHP's configuration directive default_socket_timeout to -1 (meaning infinite timeout), and noticed that all requests created with symfony/http-client failed with a "Idle timeout reached" exception.

This looked like #44198, but the issue persisted beyond upgrading from 5.3.7 to 5.4.0.

How to reproduce

ini_set('default_socket_timeout', '-1');
$response = \Symfony\Component\HttpClient\HttpClient::create()->request('GET', 'https://symfony.com');
dd($response->getStatusCode());
// Symfony\Component\HttpClient\Exception\TimeoutException with message 'Idle timeout reached for "https://symfony.com/".'

Possible Solution

The HttpClientTrait falls back to the default_socket_timeout value from PHP:

symfony/src/Symfony/Component/HttpClient/HttpClientTrait.php

Line 162 in d2600e8

$options['timeout'] = (float) ($options['timeout'] ?? ini_get('default_socket_timeout'));

However, the TransportResponseTrait does not seem to handle this well:

symfony/src/Symfony/Component/HttpClient/Response/TransportResponseTrait.php

Line 184 in d2600e8

$timeoutMin = min($timeoutMin, $response->timeout, 1);

Consequently (I think), the following usleep() call is skipped:

symfony/src/Symfony/Component/HttpClient/Response/TransportResponseTrait.php

Lines 312 to 314 in d2600e8

if (-1 === self::select($multi, min($timeoutMin, $timeoutMax - $elapsedTimeout))) {
usleep(min(500, 1E6 * $timeoutMin));
}

So curl may not have a chance to actually proceed with the request, and since no time is elapsed, $elapsedTimeout and $timeoutMax are still both 0, causing the following ErrorChunk:

symfony/src/Symfony/Component/HttpClient/Response/TransportResponseTrait.php

Lines 197 to 199 in d2600e8

} elseif ($elapsedTimeout >= $timeoutMax) {
$multi->handlesActivity[$j] = [new ErrorChunk($response->offset, sprintf('Idle timeout reached for "%s".', $response->getInfo('url')))];
$multi->lastTimeout ?? $multi->lastTimeout = $lastActivity;

Additional Context

Originally posted here: meilisearch/meilisearch-php#262

 Symfony\Component\HttpClient\Exception\TimeoutException: Idle timeout reached for "http://meilisearch:7700/indexes/projects/documents?primaryKey=id".
 
 /var/www/html/vendor/symfony/http-client/Chunk/ErrorChunk.php:62
 /var/www/html/vendor/symfony/http-client/Response/CommonResponseTrait.php:150
 /var/www/html/vendor/symfony/http-client/Response/TransportResponseTrait.php:55
 /var/www/html/vendor/symfony/http-client/Psr18Client.php:100
 (...)
@caugner caugner added the Bug label Dec 6, 2021
@caugner
Copy link
Contributor Author
caugner commented Dec 6, 2021

Probably, the easiest solution would be to either update HttpClientTrait to set $options['timeout'] to INF (or null?) if ini_get('default_socket_timeout')) is negative, or to update TransportResponseTrait to ignore negative timeout values.

@caugner caugner changed the title [HttpClient] Idle timeout reached when default_socket_timeout is negative [HttpClient] Idle timeout reached with negative default_socket_timeout Dec 6, 2021
@caugner caugner mentioned this issue Dec 6, 2021
Closed
@nicolas-grekas
Copy link
Member

I never considered the timeout could be set to a negative value (I'm not sure it makes sense either to use infinite timeouts...)
I'd be fine turning them into something long enough. A (few) day(s) is already long enough IMHO, no need to use INF IMHO.
PR welcome.

@fancyweb fancyweb mentioned this issue Dec 31, 2021
Merged
@fabpot fabpot closed this as completed Jan 1, 2022
fabpot added a commit that referenced this issue Jan 1, 2022
@fabpot
8000
bug #44878 [HttpClient] Turn negative timeout to a very long timeout …
c886540 
…(fancyweb)

This PR was merged into the 4.4 branch.

Discussion
----------

[HttpClient] Turn negative timeout to a very long timeout

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

0 keeps on throwing.

Commits
-------

6360c31 [HttpClient] Turn negative timeout to a very long timeout
@vyshkant
Copy link
Contributor
vyshkant commented Jan 6, 2022 •
edited
Loading

I'm having a similar issue with this code:

$response = $httpClient->request('GET', 'https://example.org');
$stream = $response->toStream();

the call chain is next:

where at the last point $timeoutMax equals 0.

So in fact even though -0.0 was passed to the last method, we still have 0 set to $timeoutMax at that point.

@fabpot is this an expected behavior? If no, should I create a separate issue for this?

@nicolas-grekas
Copy link
Member

Please open another issue yes, with a detailed description of what happens for you and a reproducer if possible

@vyshkant vyshkant mentioned this issue Jan 6, 2022
Closed
@stof
Copy link
Member
stof commented Jan 6, 2022

to me, having -0 being treated the same than 0 is OK.

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
6 participants
@fabpot @nicolas-grekas @stof @caugner @vyshkant @carsonbot
0