Skip to content

[8.x] Add a default timeout in the HTTP client #40180

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
wants to merge 2 commits into from

Conversation

themsaid
Copy link
Member

@themsaid themsaid commented Dec 27, 2021

Update

Moved to the 9.x release: #40187

@@ -162,6 +162,7 @@ public function __construct(Factory $factory = null)

$this->options = [
'http_errors' => false,
'timeout' => 10,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

10 seconds is probably too low for a lot of people. I'd recommend going with:

'connect_timeout' => 10,
'timeout' => 15,

like in https://github.com/GrahamCampbell/Guzzle-Factory/blob/5.0/src/GuzzleFactory.php.

@mabdullahsari
Copy link
Contributor

Please target 9.x.

This change will break an app I work on because it has API calls that last 3 mins (I did not design it 🙃).

@SjorsO
Copy link
Contributor

SjorsO commented Dec 27, 2021

A default timeout is a great idea. I've had queue workers get stuck because HTTP calls timed-out in a weird way (even though supervisor reported they were working fine).

@leonardovee
Copy link

A default timeout is sure a good idea, but shouldn't this be treat as a breaking change?

@taylorotwell taylorotwell changed the base branch from 8.x to master December 27, 2021 21:06
@taylorotwell taylorotwell reopened this Dec 27, 2021
@taylorotwell taylorotwell changed the base branch from master to 8.x December 27, 2021 21:06
@taylorotwell
Copy link
Member

@themsaid I agree we should probably send this to 9.x / master.

@themsaid themsaid closed this Dec 28, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants