8000 [HttpClient] Configure total timeout of request · Issue #32765 · symfony/symfony · GitHub
[go: up one dir, main page]

Skip to content

[HttpClient] Configure total timeout of request #32765

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.

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

Closed
pyrech opened this issue Jul 26, 2019 · 4 comments
Closed

[HttpClient] Configure total timeout of request #32765

pyrech opened this issue Jul 26, 2019 · 4 comments

Comments

@pyrech
Copy link
Contributor
pyrech commented Jul 26, 2019

Symfony version(s) affected: 4.3.x

Description
Symfony's HTTP CURL Client supports a timeout option that will set the option CURLOPT_CONNECTTIMEOUT_MS of curl. This parameter only configures the timeout for the connect phase.

But in some use cases (like a crawler f.e) we need to limit the maximum time a request is allowed to take, not only the connect duration. This is what the curl options CURLOPT_TIMEOUT and CURLOPT_TIMEOUT_MS do.

Unless I'm wrong, there is no way to configure the curl client with some custom config. Is that really intended? If yes, how can we change this timeout behavior?

How to reproduce

$httpClient = new CurlHttpClient();
$httpClient->request('GET', 'http://example.com/a-page-that-takes-a-long-time-to-render',  [
    'timeout' => 1,
]);

Possible Solution
Either add a way to override curl option or change the timeout behavior of the curl client

@nicolas-grekas
Copy link
Member
nicolas-grekas commented Jul 26, 2019

Hi, that's intended yes: timeout is an idle timeout. Having a transfer timeout could be possible.

It's also possible to implement it in userland using the stream function (checking the duration in the foreach loop)

Allowing to pass the curl option as-is is no-go on my side: I'd really like the component to not leak the underlying implem. Said another way: we might add a new transfer_timeout option, but then it should be implemented by both curl and native clients.

Up for a PR?

@pyrech
Copy link
Contributor Author
pyrech commented Jul 26, 2019

Ok, I understand, a transfer_timeout option would be great 👍

Sorry I do not have the time to give it a try at the moment.

@fancyweb
Copy link
Contributor

I'll have a look at it.

@fancyweb
Copy link
Contributor

@nicolas-grekas curl CURLOPT_TIMEOUT_MS is actually a "max execution time" timeout. It includes the connection time. Shouldn't we use something like total_timeout for the new option name (since it's not only transfer time).

symfony-splitter pushed a commit to symfony/http-client that referenced this issue Aug 6, 2019
This PR was merged into the 4.4 branch.

Discussion
----------

[HttpClient] add "max_duration" option

| Q             | A
| ------------- | ---
| Branch?       | 4.4
| Bug fix?      | no
| New feature?  | yes
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | symfony/symfony#32765
| License       | MIT
| Doc PR        | symfony/symfony-docs#12073

Commits
-------

a4178f1369 [HttpClient] add "max_duration" option
nicolas-grekas added a commit that referenced this issue Aug 6, 2019
This PR was merged into the 4.4 branch.

Discussion
----------

[HttpClient] add "max_duration" option

| Q             | A
| ------------- | ---
| Branch?       | 4.4
| Bug fix?      | no
| New feature?  | yes
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | #32765
| License       | MIT
| Doc PR        | symfony/symfony-docs#12073

Commits
-------

a4178f1 [HttpClient] add "max_duration" option
symfony-splitter pushed a commit to symfony/framework-bundle that referenced this issue Aug 6, 2019
This PR was merged into the 4.4 branch.

Discussion
----------

[HttpClient] add "max_duration" option

| Q             | A
| ------------- | ---
| Branch?       | 4.4
| Bug fix?      | no
| New feature?  | yes
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | symfony/symfony#32765
| License       | MIT
| Doc PR        | symfony/symfony-docs#12073

Commits
-------

a4178f1369 [HttpClient] add "max_duration" option
symfony-splitter pushed a commit to symfony/contracts that referenced this issue Aug 6, 2019
This PR was merged into the 4.4 branch.

Discussion
----------

[HttpClient] add "max_duration" option

| Q             | A
| ------------- | ---
| Branch?       | 4.4
| Bug fix?      | no
| New feature?  | yes
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | symfony/symfony#32765
| License       | MIT
| Doc PR        | symfony/symfony-docs#12073

Commits
-------

a4178f1369 [HttpClient] add "max_duration" option
symfony-splitter pushed a commit to symfony/http-client-contracts that referenced this issue Aug 6, 2019
This PR was merged into the 4.4 branch.

Discussion
----------

[HttpClient] add "max_duration" option

| Q             | A
| ------------- | ---
| Branch?       | 4.4
| Bug fix?      | no
| New feature?  | yes
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | symfony/symfony#32765
| License       | MIT
| Doc PR        | symfony/symfony-docs#12073

Commits
-------

a4178f1369 [HttpClient] add "max_duration" option
6886
sadafrangian3 pushed a commit to sadafrangian3/Dependency-Injection-http-client that referenced this issue Nov 2, 2022
This PR was merged into the 4.4 branch.

Discussion
----------

[HttpClient] add "max_duration" option

| Q             | A
| ------------- | ---
| Branch?       | 4.4
| Bug fix?      | no
| New feature?  | yes
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | symfony/symfony#32765
| License       | MIT
| Doc PR        | symfony/symfony-docs#12073

Commits
-------

a4178f1369 [HttpClient] add "max_duration" option
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants
0