Skip to content
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

Corrected docs and default connect timeout value to 300 seconds #3075

Merged
merged 3 commits into from May 14, 2023

Conversation

aivus
Copy link
Contributor

@aivus aivus commented Sep 26, 2022

As per cURL documentation if CURLOPT_CONNECTTIMEOUT set to 0 it means 300 seconds timeout:

Set to zero to switch to the default built-in connection timeout - 300 seconds.

Additionally, curl provides 2 options to set connect timeout:
CURLOPT_CONNECTTIMEOUT
CURLOPT_CONNECTTIMEOUT_MS

As mentioned, If both CURLOPT_CONNECTTIMEOUT and CURLOPT_CONNECTTIMEOUT_MS are set, the value set last will be used.

Guzzle code sets CURLOPT_CONNECTTIMEOUT to default 150, but then uses CURLOPT_CONNECTTIMEOUT_MS to overwrite this value.

This PR replaces CURLOPT_CONNECTTIMEOUT on CURLOPT_CONNECTTIMEOUT_MS and changes the default value for connect timeout from 150 to 300 to get rid of the wrong behaviour.

@GrahamCampbell
Copy link
Member

Hmm. Do you know when this changed. I don't think this was always the case?

@aivus
Copy link
Contributor Author

aivus commented Oct 27, 2022

@GrahamCampbell unfortunately I don't know if it has ever been changed.

@gmponos
Copy link
Member

gmponos commented Oct 27, 2022

#2559

@GrahamCampbell
Copy link
Member

Hahahaha. I'd totally forgotten I PRed something similar. 😆

@stale
Copy link

stale bot commented Mar 11, 2023

This issue has been automatically marked as stale because it has not had recent activity. It will be closed after 2 weeks if no further activity occurs. Thank you for your contributions.

@stale stale bot added the lifecycle/stale No activity for a long time label Mar 11, 2023
@stale stale bot closed this Apr 2, 2023
@GrahamCampbell GrahamCampbell reopened this Apr 2, 2023
@stale stale bot removed the lifecycle/stale No activity for a long time label Apr 2, 2023
@GrahamCampbell GrahamCampbell changed the title Sync connect_timeout values around the code and docs Corrected docs and default connect timeout value to 300 seconds May 14, 2023
@GrahamCampbell GrahamCampbell merged commit 39ad11f into guzzle:7.5 May 14, 2023
23 checks passed
@aivus aivus deleted the connect-timeout branch May 14, 2023 09:54
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.

None yet

3 participants