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

Respect http_timeout core setting for Guzzle HTTP requests #21096

Merged
merged 1 commit into from
Aug 12, 2021

Conversation

mattwire
Copy link
Contributor

Overview

CiviCRM core has a http_timeout setting but USPS, Google geocode and recaptcha is not respecting that timeout and using guzzle default which is 0 (unlimited wait).

Before

Guzzle HTTP requests not respecting core http_timeout setting.

After

Guzzle HTTP requests respecting core http_timeout setting (default 5).

Technical Details

Comments

@civibot
Copy link

civibot bot commented Aug 11, 2021

(Standard links)

@civibot civibot bot added the master label Aug 11, 2021
@demeritcowboy
Copy link
Contributor

  • General standards
    • [r-explain] PASS
    • [r-user] PASS
    • [r-doc] PASS
    • [r-run] PASS
      • I couldn't find what screen the setting was on but I could set it with cv. It also has a default of 5.
      • I only tested the google one. I tested it by hacking the google url to be 10.0.0.3.
        • Before: (took about 30 seconds) GuzzleHttp\Exception\ConnectException: "cURL error 28: Failed to connect to 10.0.0.3 port 80: Timed out (see https://curl.haxx.se/libcurl/c/libcurl-errors.html)"
        • After: GuzzleHttp\Exception\ConnectException: "cURL error 28: Connection timed out after 5013 milliseconds (see https://curl.haxx.se/libcurl/c/libcurl-errors.html)"
      • Out of scope but it would be nice to display a nicer error message than the fatal error screen. But it shouldn't come up too often.
  • Developer standards
    • [r-tech] PASS
    • [r-code] PASS
    • [r-maint] ?
    • [r-test] PASS

@demeritcowboy demeritcowboy merged commit d32b0c3 into civicrm:master Aug 12, 2021
@mattwire
Copy link
Contributor Author

Thanks @demeritcowboy

@mattwire mattwire deleted the guzzletimeout branch August 12, 2021 16:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants