Skip to content

Commit

Permalink
Fix retry to add a small pause between retries after the second one, …
Browse files Browse the repository at this point in the history
…refs #10716
  • Loading branch information
Seldaek committed Apr 29, 2022
1 parent 1f75af6 commit da32264
Show file tree
Hide file tree
Showing 4 changed files with 35 additions and 40 deletions.
2 changes: 1 addition & 1 deletion phpstan/baseline-8.1.neon
Expand Up @@ -256,7 +256,7 @@ parameters:
path: ../src/Composer/Util/Http/CurlDownloader.php

-
message: "#^Property Composer\\\\Util\\\\Http\\\\CurlDownloader\\:\\:\\$jobs \\(array\\<array\\{url\\: string, origin\\: string, attributes\\: array\\{retryAuthFailure\\: bool, redirects\\: int, retries\\: int, storeAuth\\: bool\\}, options\\: array, progress\\: array, curlHandle\\: resource, filename\\: string\\|null, headerHandle\\: resource, \\.\\.\\.\\}\\>\\) does not accept non\\-empty\\-array\\<array\\{url\\: string, origin\\: string, attributes\\: array\\{retryAuthFailure\\: bool, redirects\\: int, retries\\: int, storeAuth\\: bool\\}, options\\: array, progress\\: array, curlHandle\\: CurlHandle\\|resource, filename\\: string\\|null, headerHandle\\: resource, \\.\\.\\.\\}\\>\\.$#"
message: "#^Property Composer\\\\Util\\\\Http\\\\CurlDownloader\\:\\:\\$jobs \\(array\\<array\\{url\\: string, origin\\: string, attributes\\: array\\{retryAuthFailure\\: bool, redirects\\: int\\<0, max\\>, retries\\: int\\<0, max\\>, storeAuth\\: 'prompt'\\|bool\\}, options\\: array, progress\\: array, curlHandle\\: resource, filename\\: string\\|null, headerHandle\\: resource, \\.\\.\\.\\}\\>\\) does not accept non\\-empty\\-array\\<array\\{url\\: string, origin\\: string, attributes\\: array\\{retryAuthFailure\\: bool, redirects\\: int\\<0, max\\>, 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

Expand Down
27 changes: 1 addition & 26 deletions phpstan/baseline.neon
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -4954,7 +4939,7 @@ parameters:
path: ../src/Composer/Util/Http/CurlDownloader.php

-
message: "#^Property Composer\\\\Util\\\\Http\\\\CurlDownloader\\:\\:\\$jobs \\(array\\<array\\{url\\: string, origin\\: string, attributes\\: array\\{retryAuthFailure\\: bool, redirects\\: int, retries\\: int, storeAuth\\: bool\\}, options\\: array, progress\\: array, curlHandle\\: resource, filename\\: string\\|null, headerHandle\\: resource, \\.\\.\\.\\}\\>\\) does not accept non\\-empty\\-array\\<array\\{url\\: string, origin\\: string, attributes\\: array\\{retryAuthFailure\\: bool, redirects\\: int, retries\\: int, storeAuth\\: bool\\}, options\\: array, progress\\: array, curlHandle\\: resource, filename\\: string\\|null, headerHandle\\: resource, \\.\\.\\.\\}\\>\\.$#"
message: "#^Property Composer\\\\Util\\\\Http\\\\CurlDownloader\\:\\:\\$jobs \\(array\\<array\\{url\\: string, origin\\: string, attributes\\: array\\{retryAuthFailure\\: bool, redirects\\: int\\<0, max\\>, retries\\: int\\<0, max\\>, storeAuth\\: 'prompt'\\|bool\\}, options\\: array, progress\\: array, curlHandle\\: resource, filename\\: string\\|null, headerHandle\\: resource, \\.\\.\\.\\}\\>\\) does not accept non\\-empty\\-array\\<array\\{url\\: string, origin\\: string, attributes\\: array\\{retryAuthFailure\\: bool, redirects\\: int\\<0, max\\>, 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

Expand Down Expand Up @@ -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
Expand Down
10 changes: 5 additions & 5 deletions src/Composer/Util/AuthHelper.php
Expand Up @@ -37,7 +37,7 @@ public function __construct(IOInterface $io, Config $config)

/**
* @param string $origin
* @param string|bool $storeAuth
* @param 'prompt'|bool $storeAuth
*
* @return void
*/
Expand Down Expand Up @@ -79,11 +79,11 @@ function ($value): string {
* @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;

Expand Down Expand Up @@ -185,7 +185,7 @@ public function promptAuthIfNeeded(string $url, string $origin, int $statusCode,
} else {
// 404s are only handled for github
if ($statusCode === 404) {
return null;
return ['retry' => false, 'storeAuth' => false];
}

// fail if the console is not interactive
Expand Down
36 changes: 28 additions & 8 deletions src/Composer/Util/Http/CurlDownloader.php
Expand Up @@ -27,7 +27,7 @@
* @internal
* @author Jordi Boggiano <j.boggiano@seld.be>
* @author Nicolas Grekas <p@tchwork.com>
* @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
Expand Down Expand Up @@ -152,7 +152,7 @@ public function download(callable $resolve, callable $reject, string $origin, st
* @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
*/
Expand All @@ -177,6 +177,7 @@ private function initDownload(callable $resolve, callable $reject, string $origi
throw new \RuntimeException('Failed to open a temp stream to store curl headers');
}


if ($copyTo) {
$errorMessage = '';
// @phpstan-ignore-next-line
Expand Down Expand Up @@ -363,7 +364,7 @@ public function tick(): void
) && $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;
}

Expand Down Expand Up @@ -427,14 +428,14 @@ public function tick(): void
&& $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']);
}

Expand Down Expand Up @@ -524,8 +525,8 @@ private function handleRedirect(array $job, Response $response): string
}

/**
* @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
{
Expand Down Expand Up @@ -578,7 +579,7 @@ private function isAuthenticatedRetryNeeded(array $job, Response $response): arr
* @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
*/
Expand All @@ -594,6 +595,25 @@ private function restartJob(array $job, string $url, array $attributes = array()
$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
Expand Down

0 comments on commit da32264

Please sign in to comment.