Skip to content

Add automatic retry mechanism for HTTP requests #6140

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
Open
wants to merge 6 commits into
base: main
Choose a base branch
from
Open

Add automatic retry mechanism for HTTP requests #6140

wants to merge 6 commits into from

Conversation

Copy link

Copilot AI commented Oct 30, 2025 •
edited
Loading

Implementation Complete: Add automatic retries to download paths

  • Explore repository structure and understand current download implementation
  • Locate the http_request function in php/utils.php
  • Understand current retry logic (only for SSL cert issues)
  • Implement retry logic in http_request for transient failures (timeouts, network errors)
  • Update function documentation
  • Add helper function is_transient_http_error to identify retryable errors
  • Add tests for retry functionality
  • Hardcode retry count to 3 (removed configurable option per feedback)
  • Fix code formatting (align array comments)
  • Fix curl_close deprecation for PHP 8.5+
  • Fix test mock issue (removed getMessage mock)
  • Add PHPStan type annotation for curl_errno
  • Run code review
  • Run security scan with CodeQL

Summary

Successfully implemented automatic retry mechanism for HTTP requests in WP-CLI with a hardcoded 3 retry threshold.

Key Features

  1. Hardcoded 3 retries - Simple, non-configurable retry mechanism
  2. Smart retry logic - Only retries on transient network errors (timeouts, connection issues)
  3. Exponential backoff - Uses 1s, 2s, 4s, 8s, 10s (max) delays between retries
  4. Debug logging - Logs retry attempts when --debug=http is enabled
  5. Backward compatible - No breaking changes, works transparently with existing code
  6. PHP 8.5+ compatible - Handles curl_close deprecation

Transient Errors Handled

  • CURLE_OPERATION_TIMEDOUT (28) - Operation timeout
  • CURLE_COULDNT_RESOLVE_HOST (6) - Couldn't resolve host
  • CURLE_COULDNT_CONNECT (7) - Failed to connect
  • CURLE_PARTIAL_FILE (18) - Partial file transfer
  • CURLE_GOT_NOTHING (52) - Server returned nothing
  • CURLE_SEND_ERROR (55) - Send error
  • CURLE_RECV_ERROR (56) - Receive error

Files Changed

  • php/utils.php - Implemented retry logic and helper function
  • tests/UtilsTest.php - Added unit test for transient error detection

Benefits

  • Improved reliability for downloads (themes, plugins, core, language files)
  • Better automated test stability
  • Reduced impact of transient network failures
Original prompt

This section details on the original issue you should resolve

<issue_title>Add automatic retries to download paths</issue_title>
<issue_description>For operations like downloading a theme or a language file, or latest Core code, we should have an automatic retry mechanism with a configurable threshold (defaulting to 3 as a good compromise).

This would not only make general scripting with WP-CLI more reliable but also drastically increase the reliability of our automated tests, which constantly fail due to random timeouts when doing downloads.</issue_description>

Comments on the Issue (you are @copilot in this section)

💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.

@codecov
Copy link

codecov bot commented Oct 30, 2025 •
edited
Loading

Codecov Report

❌ Patch coverage is 78.68852% with 13 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
php/utils.php 78.68% 13 Missing ⚠️

📢 Thoughts on this report? Let us know!

Copilot AI and others added 2 commits October 30, 2025 17:00
@swissspidy
Add automatic retry mechanism for HTTP requests with configurable thr…
ae273bc 
…eshold

Co-authored-by: swissspidy <841956+swissspidy@users.noreply.github.com>
@swissspidy
Fix fallback to default value when config is not initialized
8ec0df2 
Co-authored-by: swissspidy <841956+swissspidy@users.noreply.github.com>
Copilot AI changed the title [WIP] Add automatic retries for downloading files Add automatic retry mechanism for HTTP requests with configurable threshold Oct 30, 2025
Copilot AI requested a review from swissspidy October 30, 2025 17:15
Copilot finished work on behalf of swissspidy October 30, 2025 17:15
@swissspidy
Copy link
Member

swissspidy commented Oct 30, 2025

@copilot Fix code formatting issues such as "Array double arrow not aligned correctly" using composer phpcbf, verify with composer phpcs.

Let's not add a new global parameter for this, but simply hardcode this to 3 retries.

@swissspidy
Remove config option and hardcode retry count to 3, fix code formatting
187f95d 
Co-authored-by: swissspidy <841956+swissspidy@users.noreply.github.com>
Copy link
Author

Copilot AI commented Oct 30, 2025

@copilot Fix code formatting issues such as "Array double arrow not aligned correctly" using composer phpcbf, verify with composer phpcs.

Let's not add a new global parameter for this, but simp...

Done in 187f95d. Removed the global parameter http_request_retries from config-spec.php and hardcoded retry count to 3 in utils.php. Also aligned the array comments in the transient error list.

Copilot AI changed the title Add automatic retry mechanism for HTTP requests with configurable threshold Add automatic retry mechanism for HTTP requests Oct 30, 2025
Copilot finished work on behalf of swissspidy October 30, 2025 21:26
@swissspidy
Copy link
Member

swissspidy commented Oct 31, 2025

@copilot Don't call curl_close if the current PHP version is 8.5 or higher, it's deprecated.

One test failure:

UtilsTest::testIsTransientHttpError
PHPUnit\Framework\MockObject\MethodCannotBeConfiguredException: Trying to configure method "getMessage" which cannot be configured because it does not exist, has not been specified, is final, or is static

Also, PHPStan reports:

Parameter #1 $handle of function curl_errno expects CurlHandle, CurlHandle|resource given.

@swissspidy
Copy link
Member

swissspidy commented Oct 31, 2025

@copilot are you on it?

Copilot stopped work on behalf of swissspidy due to an error October 31, 2025 00:42
Job was cancelled
@swissspidy
Copy link
Member

swissspidy commented Oct 31, 2025

@copilot Try again

@swissspidy
Fix curl_close deprecation, remove getMessage mock, add PHPStan type …
a89a6ab 
…annotation

Co-authored-by: swissspidy <841956+swissspidy@users.noreply.github.com>
Copy link
Author

Copilot AI commented Oct 31, 2025 •
edited
Loading

@copilot are you on it?

Yes, fixed in a89a6ab. Added PHP version check for curl_close, removed getMessage mock, and added PHPStan type annotation for curl_errno parameter.

@copilot Try again

Done in a89a6ab:

  • Added PHP 8.5+ check before curl_close call
  • Removed getMessage() mock (final method can't be mocked)
  • Added @var annotation for PHPStan to fix curl_errno type check

Copilot finished work on behalf of swissspidy October 31, 2025 00:47
@swissspidy swissspidy marked this pull request as ready for review October 31, 2025 00:59
@swissspidy swissspidy requested a review from a team as a code owner October 31, 2025 00:59
@swissspidy swissspidy mentioned this pull request Nov 1, 2025
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@swissspidy swissspidy Awaiting requested review from swissspidy

At least 1 approving review is required to merge this pull request.

Assignees

@swissspidy swissspidy

Copilot code review Copilot

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Add automatic retries to download paths

2 participants

@swissspidy