From da322643d6b107157b5cc2faf67c118016814871 Mon Sep 17 00:00:00 2001 From: Jordi Boggiano Date: Fri, 29 Apr 2022 14:50:58 +0200 Subject: [PATCH] Fix retry to add a small pause between retries after the second one, refs #10716 --- phpstan/baseline-8.1.neon | 2 +- phpstan/baseline.neon | 27 +---------------- src/Composer/Util/AuthHelper.php | 10 +++---- src/Composer/Util/Http/CurlDownloader.php | 36 ++++++++++++++++++----- 4 files changed, 35 insertions(+), 40 deletions(-) diff --git a/phpstan/baseline-8.1.neon b/phpstan/baseline-8.1.neon index bd3c458f2..72a234af4 100644 --- a/phpstan/baseline-8.1.neon +++ b/phpstan/baseline-8.1.neon @@ -256,7 +256,7 @@ parameters: path: ../src/Composer/Util/Http/CurlDownloader.php - - message: "#^Property Composer\\\\Util\\\\Http\\\\CurlDownloader\\:\\:\\$jobs \\(array\\\\) does not accept non\\-empty\\-array\\\\.$#" + message: "#^Property Composer\\\\Util\\\\Http\\\\CurlDownloader\\:\\:\\$jobs \\(array\\, retries\\: int\\<0, max\\>, storeAuth\\: 'prompt'\\|bool\\}, options\\: array, progress\\: array, curlHandle\\: resource, filename\\: string\\|null, headerHandle\\: resource, \\.\\.\\.\\}\\>\\) does not accept non\\-empty\\-array\\, retries\\: int\\<0, max\\>, storeAuth\\: 'prompt'\\|bool\\}, options\\: array, progress\\: array, curlHandle\\: CurlHandle\\|resource, filename\\: string\\|null, headerHandle\\: resource, \\.\\.\\.\\}\\>\\.$#" count: 1 path: ../src/Composer/Util/Http/CurlDownloader.php diff --git a/phpstan/baseline.neon b/phpstan/baseline.neon index 94faaa523..9fd8b8af7 100644 --- a/phpstan/baseline.neon +++ b/phpstan/baseline.neon @@ -4813,16 +4813,6 @@ parameters: count: 3 path: ../src/Composer/Util/Http/CurlDownloader.php - - - message: "#^Method Composer\\\\Util\\\\Http\\\\CurlDownloader\\:\\:isAuthenticatedRetryNeeded\\(\\) should return array\\{retry\\: bool, storeAuth\\: bool\\|string\\} but returns array\\{retry\\: bool, storeAuth\\: bool\\|string\\}\\|null\\.$#" - count: 2 - path: ../src/Composer/Util/Http/CurlDownloader.php - - - - message: "#^Offset 'retry' does not exist on array\\{retry\\: bool, storeAuth\\: bool\\|string\\}\\|null\\.$#" - count: 2 - path: ../src/Composer/Util/Http/CurlDownloader.php - - message: "#^Only booleans are allowed in &&, int given on the right side\\.$#" count: 2 @@ -4938,11 +4928,6 @@ parameters: count: 1 path: ../src/Composer/Util/Http/CurlDownloader.php - - - message: "#^Parameter \\#3 \\$attributes of method Composer\\\\Util\\\\Http\\\\CurlDownloader\\:\\:restartJob\\(\\) expects array\\{retryAuthFailure\\?\\: bool, redirects\\?\\: int, storeAuth\\?\\: bool\\}, array\\{storeAuth\\: bool\\|string\\} given\\.$#" - count: 1 - path: ../src/Composer/Util/Http/CurlDownloader.php - - message: "#^Parameter \\#3 \\$errorMessage of method Composer\\\\Util\\\\Http\\\\CurlDownloader\\:\\:failResponse\\(\\) expects string, string\\|null given\\.$#" count: 1 @@ -4954,7 +4939,7 @@ parameters: path: ../src/Composer/Util/Http/CurlDownloader.php - - message: "#^Property Composer\\\\Util\\\\Http\\\\CurlDownloader\\:\\:\\$jobs \\(array\\\\) does not accept non\\-empty\\-array\\\\.$#" + message: "#^Property Composer\\\\Util\\\\Http\\\\CurlDownloader\\:\\:\\$jobs \\(array\\, retries\\: int\\<0, max\\>, storeAuth\\: 'prompt'\\|bool\\}, options\\: array, progress\\: array, curlHandle\\: resource, filename\\: string\\|null, headerHandle\\: resource, \\.\\.\\.\\}\\>\\) does not accept non\\-empty\\-array\\, retries\\: int\\<0, max\\>, storeAuth\\: 'prompt'\\|bool\\}, options\\: array, progress\\: array, curlHandle\\: resource, filename\\: string\\|null, headerHandle\\: resource, \\.\\.\\.\\}\\>\\.$#" count: 1 path: ../src/Composer/Util/Http/CurlDownloader.php @@ -5363,16 +5348,6 @@ parameters: count: 1 path: ../src/Composer/Util/RemoteFilesystem.php - - - message: "#^Offset 'retry' does not exist on array\\{retry\\: bool, storeAuth\\: bool\\|string\\}\\|null\\.$#" - count: 1 - path: ../src/Composer/Util/RemoteFilesystem.php - - - - message: "#^Offset 'storeAuth' does not exist on array\\{retry\\: bool, storeAuth\\: bool\\|string\\}\\|null\\.$#" - count: 1 - path: ../src/Composer/Util/RemoteFilesystem.php - - message: "#^Only booleans are allowed in &&, bool\\|string given on the left side\\.$#" count: 1 diff --git a/src/Composer/Util/AuthHelper.php b/src/Composer/Util/AuthHelper.php index 1787e1362..148122fd1 100644 --- a/src/Composer/Util/AuthHelper.php +++ b/src/Composer/Util/AuthHelper.php @@ -37,7 +37,7 @@ class AuthHelper /** * @param string $origin - * @param string|bool $storeAuth + * @param 'prompt'|bool $storeAuth * * @return void */ @@ -79,11 +79,11 @@ class AuthHelper * @param int $statusCode HTTP status code that triggered this call * @param string|null $reason a message/description explaining why this was called * @param string[] $headers - * @return array|null containing retry (bool) and storeAuth (string|bool) keys, if retry is true the request should be + * @return array containing retry (bool) and storeAuth (string|bool) keys, if retry is true the request should be * retried, if storeAuth is true then on a successful retry the authentication should be persisted to auth.json - * @phpstan-return ?array{retry: bool, storeAuth: string|bool} + * @phpstan-return array{retry: bool, storeAuth: 'prompt'|bool} */ - public function promptAuthIfNeeded(string $url, string $origin, int $statusCode, ?string $reason = null, array $headers = array()): ?array + public function promptAuthIfNeeded(string $url, string $origin, int $statusCode, ?string $reason = null, array $headers = array()): array { $storeAuth = false; @@ -185,7 +185,7 @@ class AuthHelper } else { // 404s are only handled for github if ($statusCode === 404) { - return null; + return ['retry' => false, 'storeAuth' => false]; } // fail if the console is not interactive diff --git a/src/Composer/Util/Http/CurlDownloader.php b/src/Composer/Util/Http/CurlDownloader.php index 54628204e..93b76f305 100644 --- a/src/Composer/Util/Http/CurlDownloader.php +++ b/src/Composer/Util/Http/CurlDownloader.php @@ -27,7 +27,7 @@ use React\Promise\Promise; * @internal * @author Jordi Boggiano * @author Nicolas Grekas - * @phpstan-type Attributes array{retryAuthFailure: bool, redirects: int, retries: int, storeAuth: bool} + * @phpstan-type Attributes array{retryAuthFailure: bool, redirects: int<0, max>, retries: int<0, max>, storeAuth: 'prompt'|bool} * @phpstan-type Job array{url: string, origin: string, attributes: Attributes, options: mixed[], progress: mixed[], curlHandle: resource, filename: string|null, headerHandle: resource, bodyHandle: resource, resolve: callable, reject: callable} */ class CurlDownloader @@ -152,7 +152,7 @@ class CurlDownloader * @param mixed[] $options * @param null|string $copyTo * - * @param array{retryAuthFailure?: bool, redirects?: int, retries?: int, storeAuth?: bool} $attributes + * @param array{retryAuthFailure?: bool, redirects?: int<0, max>, retries?: int<0, max>, storeAuth?: 'prompt'|bool} $attributes * * @return int internal job id */ @@ -177,6 +177,7 @@ class CurlDownloader throw new \RuntimeException('Failed to open a temp stream to store curl headers'); } + if ($copyTo) { $errorMessage = ''; // @phpstan-ignore-next-line @@ -363,7 +364,7 @@ class CurlDownloader ) && $job['attributes']['retries'] < $this->maxRetries ) { $this->io->writeError('Retrying ('.($job['attributes']['retries'] + 1).') ' . Url::sanitize($job['url']) . ' due to curl error '. $errno, true, IOInterface::DEBUG); - $this->restartJob($job, $job['url'], array('retries' => $job['attributes']['retries'] + 1)); + $this->restartJobWithDelay($job, $job['url'], array('retries' => $job['attributes']['retries'] + 1)); continue; } @@ -427,14 +428,14 @@ class CurlDownloader && $job['attributes']['retries'] < $this->maxRetries ) { $this->io->writeError('Retrying ('.($job['attributes']['retries'] + 1).') ' . Url::sanitize($job['url']) . ' due to status code '. $statusCode, true, IOInterface::DEBUG); - $this->restartJob($job, $job['url'], array('retries' => $job['attributes']['retries'] + 1)); + $this->restartJobWithDelay($job, $job['url'], array('retries' => $job['attributes']['retries'] + 1)); continue; } throw $this->failResponse($job, $response, $response->getStatusMessage()); } - if ($job['attributes']['storeAuth']) { + if ($job['attributes']['storeAuth'] !== false) { $this->authHelper->storeAuth($job['origin'], $job['attributes']['storeAuth']); } @@ -524,8 +525,8 @@ class CurlDownloader } /** - * @param Job $job - * @return array{retry: bool, storeAuth: string|bool} + * @param Job $job + * @return array{retry: bool, storeAuth: 'prompt'|bool} */ private function isAuthenticatedRetryNeeded(array $job, Response $response): array { @@ -578,7 +579,7 @@ class CurlDownloader * @param Job $job * @param string $url * - * @param array{retryAuthFailure?: bool, redirects?: int, storeAuth?: bool} $attributes + * @param array{retryAuthFailure?: bool, redirects?: int<0, max>, storeAuth?: 'prompt'|bool, retries?: int<1, max>} $attributes * * @return void */ @@ -594,6 +595,25 @@ class CurlDownloader $this->initDownload($job['resolve'], $job['reject'], $origin, $url, $job['options'], $job['filename'], $attributes); } + /** + * @param Job $job + * @param string $url + * + * @param array{retryAuthFailure?: bool, redirects?: int<0, max>, storeAuth?: 'prompt'|bool, retries: int<1, max>} $attributes + * + * @return void + */ + private function restartJobWithDelay(array $job, string $url, array $attributes): void + { + if ($attributes['retries'] >= 3) { + usleep(500000); // half a second delay for 3rd retry and beyond + } elseif ($attributes['retries'] >= 2) { + usleep(100000); // 100ms delay for 2nd retry + } // no sleep for the first retry + + $this->restartJob($job, $url, $attributes); + } + /** * @param Job $job * @param string $errorMessage