From 36bb67d1cde1a3659b17545554db0f21e8d654f7 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 30 Oct 2025 16:43:16 +0000 Subject: [PATCH 1/6] Initial plan From ae273bc972b464e0ae0612c6b1d26348635b496e Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 30 Oct 2025 17:00:20 +0000 Subject: [PATCH 2/6] Add automatic retry mechanism for HTTP requests with configurable threshold Co-authored-by: swissspidy <841956+swissspidy@users.noreply.github.com> --- php/config-spec.php | 7 ++ php/utils.php | 152 +++++++++++++++++++++++++++++++------------- tests/UtilsTest.php | 61 ++++++++++++++++++ 3 files changed, 176 insertions(+), 44 deletions(-) diff --git a/php/config-spec.php b/php/config-spec.php index 04026d5f33..3552cec329 100644 --- a/php/config-spec.php +++ b/php/config-spec.php @@ -121,6 +121,13 @@ 'default' => [], ], + 'http_request_retries' => [ + 'file' => '', + 'runtime' => '=', + 'default' => 3, + 'desc' => 'Number of times to retry HTTP requests on transient failures (timeouts, connection issues).', + ], + # --allow-root => (NOT RECOMMENDED) Allow wp-cli to run as root. This poses # a security risk, so you probably do not want to do this. 'allow-root' => [ diff --git a/php/utils.php b/php/utils.php index c145acc9c4..c606603020 100644 --- a/php/utils.php +++ b/php/utils.php @@ -810,10 +810,45 @@ static function ( $matches ) use ( $file, $dir ) { ); } +/** + * Check if an HTTP exception is a transient error that should be retried. + * + * @param \Requests_Exception|\WpOrg\Requests\Exception $exception The exception to check. + * @return bool True if the error is transient and should be retried. + */ +function is_transient_http_error( $exception ) { + // Check if it's a curl error. + if ( 'curlerror' !== $exception->getType() ) { + return false; + } + + $curl_handle = $exception->getData(); + if ( ! is_resource( $curl_handle ) && ! $curl_handle instanceof \CurlHandle ) { + return false; + } + + $curl_errno = curl_errno( $curl_handle ); + + // List of curl error codes that are considered transient. + // These are typically network-related errors that may succeed on retry. + $transient_curl_errors = [ + CURLE_OPERATION_TIMEDOUT, // 28 - Operation timeout. + CURLE_COULDNT_RESOLVE_HOST, // 6 - Couldn't resolve host. + CURLE_COULDNT_CONNECT, // 7 - Failed to connect to host. + CURLE_PARTIAL_FILE, // 18 - Transferred a partial file. + CURLE_GOT_NOTHING, // 52 - Server returned nothing. + CURLE_SEND_ERROR, // 55 - Failed sending network data. + CURLE_RECV_ERROR, // 56 - Failure in receiving network data. + ]; + + return in_array( $curl_errno, $transient_curl_errors, true ); +} + /** * Make a HTTP request to a remote URL. * * Wraps the Requests HTTP library to ensure every request includes a cert. + * Automatically retries on transient failures (timeouts, connection issues). * * ``` * # `wp core download` verifies the hash for a downloaded WordPress archive @@ -839,17 +874,19 @@ static function ( $matches ) use ( $file, $dir ) { * or string absolute path to CA cert to use. * Defaults to detected CA cert bundled with the Requests library. * @type bool $insecure Whether to retry automatically without certificate validation. + * @type int $retries Number of times to retry on transient failures. Overrides http_request_retries config. * } * @return \Requests_Response|Response * @throws RuntimeException If the request failed. * @throws ExitException If the request failed and $halt_on_error is true. * - * @phpstan-param array{halt_on_error?: bool, verify?: bool|string, insecure?: bool} $options + * @phpstan-param array{halt_on_error?: bool, verify?: bool|string, insecure?: bool, retries?: int} $options */ function http_request( $method, $url, $data = null, $headers = [], $options = [] ) { $insecure = isset( $options['insecure'] ) && (bool) $options['insecure']; $halt_on_error = ! isset( $options['halt_on_error'] ) || (bool) $options['halt_on_error']; - unset( $options['halt_on_error'] ); + $max_retries = isset( $options['retries'] ) ? (int) $options['retries'] : (int) WP_CLI::get_config( 'http_request_retries' ); + unset( $options['halt_on_error'], $options['retries'] ); if ( ! isset( $options['verify'] ) ) { // 'curl.cainfo' enforces the CA file to use, otherwise fallback to system-wide defaults then use the embedded CA file. @@ -868,65 +905,92 @@ function http_request( $method, $url, $data = null, $headers = [], $options = [] */ $request_method = [ RequestsLibrary::get_class_name(), 'request' ]; - try { + $attempt = 0; + $last_exception = null; + $retry_after_delay = 1; // Start with 1 second delay. + + while ( $attempt <= $max_retries ) { + ++$attempt; + try { - return $request_method( $url, $headers, $data, $method, $options ); + try { + return $request_method( $url, $headers, $data, $method, $options ); + } catch ( \Requests_Exception | \WpOrg\Requests\Exception $exception ) { + /** + * @var \CurlHandle $curl_handle + */ + $curl_handle = $exception->getData(); + if ( + true !== $options['verify'] + || 'curlerror' !== $exception->getType() + || curl_errno( $curl_handle ) !== CURLE_SSL_CACERT + ) { + throw $exception; + } + + $options['verify'] = get_default_cacert( $halt_on_error ); + + return $request_method( $url, $headers, $data, $method, $options ); + } } catch ( \Requests_Exception | \WpOrg\Requests\Exception $exception ) { /** * @var \CurlHandle $curl_handle */ $curl_handle = $exception->getData(); if ( - true !== $options['verify'] - || 'curlerror' !== $exception->getType() - || curl_errno( $curl_handle ) !== CURLE_SSL_CACERT + ! $insecure + || + 'curlerror' !== $exception->getType() + || + ! in_array( curl_errno( $curl_handle ), [ CURLE_SSL_CONNECT_ERROR, CURLE_SSL_CERTPROBLEM, CURLE_SSL_CACERT_BADFILE ], true ) ) { - throw $exception; - } + // Check if this is a transient error that should be retried. + $is_retryable = is_transient_http_error( $exception ); - $options['verify'] = get_default_cacert( $halt_on_error ); + if ( ! $is_retryable || $attempt > $max_retries ) { + $error_msg = sprintf( "Failed to get url '%s': %s.", $url, $exception->getMessage() ); + if ( $halt_on_error ) { + WP_CLI::error( $error_msg ); + } + throw new RuntimeException( $error_msg, 0, $exception ); + } - return $request_method( $url, $headers, $data, $method, $options ); - } - } catch ( \Requests_Exception | \WpOrg\Requests\Exception $exception ) { - /** - * @var \CurlHandle $curl_handle - */ - $curl_handle = $exception->getData(); - if ( - ! $insecure - || - 'curlerror' !== $exception->getType() - || - ! in_array( curl_errno( $curl_handle ), [ CURLE_SSL_CONNECT_ERROR, CURLE_SSL_CERTPROBLEM, CURLE_SSL_CACERT_BADFILE ], true ) - ) { - $error_msg = sprintf( "Failed to get url '%s': %s.", $url, $exception->getMessage() ); - if ( $halt_on_error ) { - WP_CLI::error( $error_msg ); + // Store exception and retry. + $last_exception = $exception; + WP_CLI::debug( sprintf( 'Retrying HTTP request to %s (attempt %d/%d) after transient error: %s', $url, $attempt, $max_retries + 1, $exception->getMessage() ), 'http' ); + sleep( $retry_after_delay ); + $retry_after_delay = min( $retry_after_delay * 2, 10 ); // Exponential backoff, max 10 seconds. + continue; } - throw new RuntimeException( $error_msg, 0, $exception ); - } - $warning = sprintf( - "Re-trying without verify after failing to get verified url '%s' %s.", - $url, - $exception->getMessage() - ); - WP_CLI::warning( $warning ); + $warning = sprintf( + "Re-trying without verify after failing to get verified url '%s' %s.", + $url, + $exception->getMessage() + ); + WP_CLI::warning( $warning ); - // Disable certificate validation for the next try. - $options['verify'] = false; + // Disable certificate validation for the next try. + $options['verify'] = false; - try { - return $request_method( $url, $headers, $data, $method, $options ); - } catch ( \Requests_Exception | \WpOrg\Requests\Exception $exception ) { - $error_msg = sprintf( "Failed to get non-verified url '%s' %s.", $url, $exception->getMessage() ); - if ( $halt_on_error ) { - WP_CLI::error( $error_msg ); + try { + return $request_method( $url, $headers, $data, $method, $options ); + } catch ( \Requests_Exception | \WpOrg\Requests\Exception $exception ) { + $error_msg = sprintf( "Failed to get non-verified url '%s' %s.", $url, $exception->getMessage() ); + if ( $halt_on_error ) { + WP_CLI::error( $error_msg ); + } + throw new RuntimeException( $error_msg, 0, $exception ); } - throw new RuntimeException( $error_msg, 0, $exception ); } } + + // Should never reach here, but just in case. + $error_msg = sprintf( "Failed to get url '%s' after %d attempts.", $url, $max_retries + 1 ); + if ( $halt_on_error ) { + WP_CLI::error( $error_msg ); + } + throw new RuntimeException( $error_msg, 0, $last_exception ); } /** diff --git a/tests/UtilsTest.php b/tests/UtilsTest.php index 5faa465e4f..0484274fe6 100644 --- a/tests/UtilsTest.php +++ b/tests/UtilsTest.php @@ -1113,4 +1113,65 @@ public static function dataValidClassAndMethodPair(): array { [ [ 'Exception', 'getMessage' ], true ], ]; } + + public function testIsTransientHttpError(): void { + if ( ! extension_loaded( 'curl' ) ) { + $this->markTestSkipped( 'curl not available' ); + } + + // Test timeout error (CURLE_OPERATION_TIMEDOUT = 28). + $curl_handle = curl_init(); + curl_setopt( $curl_handle, CURLOPT_URL, 'https://example.com' ); + curl_setopt( $curl_handle, CURLOPT_TIMEOUT_MS, 1 ); // Very short timeout to force timeout. + curl_setopt( $curl_handle, CURLOPT_RETURNTRANSFER, true ); + curl_exec( $curl_handle ); + $curl_errno = curl_errno( $curl_handle ); + + // Create a mock exception with the curl handle. + $exception = $this->createMockRequestsException( 'curlerror', $curl_handle ); + + // Only test if we actually got a timeout error. + if ( CURLE_OPERATION_TIMEDOUT === $curl_errno ) { + $this->assertTrue( Utils\is_transient_http_error( $exception ) ); + } + + curl_close( $curl_handle ); + } + + public function testHttpRequestRetriesConfig(): void { + // Test that the default config value is used. + $prev_config = WP_CLI::get_config( 'http_request_retries' ); + WP_CLI::set_config_value( 'http_request_retries', 2 ); + + $this->assertEquals( 2, WP_CLI::get_config( 'http_request_retries' ) ); + + // Restore previous config. + if ( null !== $prev_config ) { + WP_CLI::set_config_value( 'http_request_retries', $prev_config ); + } + } + + /** + * Create a mock Requests exception. + * + * @param string $type Exception type. + * @param \CurlHandle $curl_handle Curl handle. + * @return \WpOrg\Requests\Exception Mock exception. + */ + private function createMockRequestsException( $type, $curl_handle ) { + $exception = $this->getMockBuilder( '\WpOrg\Requests\Exception' ) + ->disableOriginalConstructor() + ->getMock(); + + $exception->method( 'getType' ) + ->willReturn( $type ); + + $exception->method( 'getData' ) + ->willReturn( $curl_handle ); + + $exception->method( 'getMessage' ) + ->willReturn( 'Mock error message' ); + + return $exception; + } } From 8ec0df22e50582dff61c1791b6a13193a9d6e447 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 30 Oct 2025 17:06:24 +0000 Subject: [PATCH 3/6] Fix fallback to default value when config is not initialized Co-authored-by: swissspidy <841956+swissspidy@users.noreply.github.com> --- php/utils.php | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/php/utils.php b/php/utils.php index c606603020..5838ae074b 100644 --- a/php/utils.php +++ b/php/utils.php @@ -885,7 +885,8 @@ function is_transient_http_error( $exception ) { function http_request( $method, $url, $data = null, $headers = [], $options = [] ) { $insecure = isset( $options['insecure'] ) && (bool) $options['insecure']; $halt_on_error = ! isset( $options['halt_on_error'] ) || (bool) $options['halt_on_error']; - $max_retries = isset( $options['retries'] ) ? (int) $options['retries'] : (int) WP_CLI::get_config( 'http_request_retries' ); + $config_retries = WP_CLI::get_config( 'http_request_retries' ); + $max_retries = isset( $options['retries'] ) ? (int) $options['retries'] : ( null !== $config_retries ? (int) $config_retries : 3 ); unset( $options['halt_on_error'], $options['retries'] ); if ( ! isset( $options['verify'] ) ) { From 187f95daaff1e1a4a143ef621e20682e911fa36a Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 30 Oct 2025 21:23:45 +0000 Subject: [PATCH 4/6] Remove config option and hardcode retry count to 3, fix code formatting Co-authored-by: swissspidy <841956+swissspidy@users.noreply.github.com> --- php/config-spec.php | 7 ------- php/utils.php | 22 ++++++++++------------ tests/UtilsTest.php | 13 ------------- 3 files changed, 10 insertions(+), 32 deletions(-) diff --git a/php/config-spec.php b/php/config-spec.php index 3552cec329..04026d5f33 100644 --- a/php/config-spec.php +++ b/php/config-spec.php @@ -121,13 +121,6 @@ 'default' => [], ], - 'http_request_retries' => [ - 'file' => '', - 'runtime' => '=', - 'default' => 3, - 'desc' => 'Number of times to retry HTTP requests on transient failures (timeouts, connection issues).', - ], - # --allow-root => (NOT RECOMMENDED) Allow wp-cli to run as root. This poses # a security risk, so you probably do not want to do this. 'allow-root' => [ diff --git a/php/utils.php b/php/utils.php index 5838ae074b..e5fd229034 100644 --- a/php/utils.php +++ b/php/utils.php @@ -832,13 +832,13 @@ function is_transient_http_error( $exception ) { // List of curl error codes that are considered transient. // These are typically network-related errors that may succeed on retry. $transient_curl_errors = [ - CURLE_OPERATION_TIMEDOUT, // 28 - Operation timeout. - CURLE_COULDNT_RESOLVE_HOST, // 6 - Couldn't resolve host. - CURLE_COULDNT_CONNECT, // 7 - Failed to connect to host. - CURLE_PARTIAL_FILE, // 18 - Transferred a partial file. - CURLE_GOT_NOTHING, // 52 - Server returned nothing. - CURLE_SEND_ERROR, // 55 - Failed sending network data. - CURLE_RECV_ERROR, // 56 - Failure in receiving network data. + CURLE_OPERATION_TIMEDOUT, // 28 - Operation timeout. + CURLE_COULDNT_RESOLVE_HOST, // 6 - Couldn't resolve host. + CURLE_COULDNT_CONNECT, // 7 - Failed to connect to host. + CURLE_PARTIAL_FILE, // 18 - Transferred a partial file. + CURLE_GOT_NOTHING, // 52 - Server returned nothing. + CURLE_SEND_ERROR, // 55 - Failed sending network data. + CURLE_RECV_ERROR, // 56 - Failure in receiving network data. ]; return in_array( $curl_errno, $transient_curl_errors, true ); @@ -874,20 +874,18 @@ function is_transient_http_error( $exception ) { * or string absolute path to CA cert to use. * Defaults to detected CA cert bundled with the Requests library. * @type bool $insecure Whether to retry automatically without certificate validation. - * @type int $retries Number of times to retry on transient failures. Overrides http_request_retries config. * } * @return \Requests_Response|Response * @throws RuntimeException If the request failed. * @throws ExitException If the request failed and $halt_on_error is true. * - * @phpstan-param array{halt_on_error?: bool, verify?: bool|string, insecure?: bool, retries?: int} $options + * @phpstan-param array{halt_on_error?: bool, verify?: bool|string, insecure?: bool} $options */ function http_request( $method, $url, $data = null, $headers = [], $options = [] ) { $insecure = isset( $options['insecure'] ) && (bool) $options['insecure']; $halt_on_error = ! isset( $options['halt_on_error'] ) || (bool) $options['halt_on_error']; - $config_retries = WP_CLI::get_config( 'http_request_retries' ); - $max_retries = isset( $options['retries'] ) ? (int) $options['retries'] : ( null !== $config_retries ? (int) $config_retries : 3 ); - unset( $options['halt_on_error'], $options['retries'] ); + $max_retries = 3; + unset( $options['halt_on_error'] ); if ( ! isset( $options['verify'] ) ) { // 'curl.cainfo' enforces the CA file to use, otherwise fallback to system-wide defaults then use the embedded CA file. diff --git a/tests/UtilsTest.php b/tests/UtilsTest.php index 0484274fe6..b54f3cd10d 100644 --- a/tests/UtilsTest.php +++ b/tests/UtilsTest.php @@ -1138,19 +1138,6 @@ public function testIsTransientHttpError(): void { curl_close( $curl_handle ); } - public function testHttpRequestRetriesConfig(): void { - // Test that the default config value is used. - $prev_config = WP_CLI::get_config( 'http_request_retries' ); - WP_CLI::set_config_value( 'http_request_retries', 2 ); - - $this->assertEquals( 2, WP_CLI::get_config( 'http_request_retries' ) ); - - // Restore previous config. - if ( null !== $prev_config ) { - WP_CLI::set_config_value( 'http_request_retries', $prev_config ); - } - } - /** * Create a mock Requests exception. * From a89a6ab32aba6864a7796e1ba4cce26212fb5b56 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 31 Oct 2025 00:45:34 +0000 Subject: [PATCH 5/6] Fix curl_close deprecation, remove getMessage mock, add PHPStan type annotation Co-authored-by: swissspidy <841956+swissspidy@users.noreply.github.com> --- php/utils.php | 3 +++ tests/UtilsTest.php | 8 ++++---- 2 files changed, 7 insertions(+), 4 deletions(-) diff --git a/php/utils.php b/php/utils.php index e5fd229034..b6e53fcb7f 100644 --- a/php/utils.php +++ b/php/utils.php @@ -827,6 +827,9 @@ function is_transient_http_error( $exception ) { return false; } + /** + * @var \CurlHandle $curl_handle + */ $curl_errno = curl_errno( $curl_handle ); // List of curl error codes that are considered transient. diff --git a/tests/UtilsTest.php b/tests/UtilsTest.php index b54f3cd10d..eaefc7ea9d 100644 --- a/tests/UtilsTest.php +++ b/tests/UtilsTest.php @@ -1135,7 +1135,10 @@ public function testIsTransientHttpError(): void { $this->assertTrue( Utils\is_transient_http_error( $exception ) ); } - curl_close( $curl_handle ); + // Don't call curl_close if PHP 8.5+ as it's deprecated. + if ( PHP_VERSION_ID < 80500 ) { + curl_close( $curl_handle ); + } } /** @@ -1156,9 +1159,6 @@ private function createMockRequestsException( $type, $curl_handle ) { $exception->method( 'getData' ) ->willReturn( $curl_handle ); - $exception->method( 'getMessage' ) - ->willReturn( 'Mock error message' ); - return $exception; } } From 8d50182abeac4a0f4a3f8826976b009bea2396f2 Mon Sep 17 00:00:00 2001 From: Pascal Birchler Date: Sun, 2 Nov 2025 10:54:38 +0100 Subject: [PATCH 6/6] Lint fix --- tests/UtilsTest.php | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/UtilsTest.php b/tests/UtilsTest.php index eaefc7ea9d..d62d41c8dd 100644 --- a/tests/UtilsTest.php +++ b/tests/UtilsTest.php @@ -1137,6 +1137,7 @@ public function testIsTransientHttpError(): void { // Don't call curl_close if PHP 8.5+ as it's deprecated. if ( PHP_VERSION_ID < 80500 ) { + // phpcs:ignore PHPCompatibility.FunctionUse.RemovedFunctions.curl_closeDeprecated curl_close( $curl_handle ); } }