diff --git a/README.md b/README.md index 987e1cd..6abf007 100644 --- a/README.md +++ b/README.md @@ -68,6 +68,7 @@ With DLoad, you can: - [PHAR Tools Management](#phar-tools-management) - [Frontend Asset Distribution](#frontend-asset-distribution) - [API Rate Limits](#api-rate-limits) +- [Failure Reporting](#failure-reporting) - [Gitlab CI configuration](#gitlab-ci-configuration) - [Contributing](#contributing) @@ -574,6 +575,31 @@ GITLAB_TOKEN=your_token_here ./vendor/bin/dload get Add to CI/CD environment variables for automated downloads. +> [!NOTE] +> In GitHub Actions, `secrets.GITHUB_TOKEN` is scoped to the current repository and shares a limit of +> 1,000 requests per hour across all jobs of the repository. With a large job matrix the limit may run out, +> and downloads from other repositories may be rejected. Use a personal access token if that happens. + +## Failure Reporting + +`dload get` exits with a non-zero code when at least one requested package was not installed, and prints +the reason for every failed download: the API error (invalid token, exhausted rate limit, missing repository), +the number of matched releases, the assets each checked release contains, and the filters that rejected them. + +``` + Failed to download `rr` +Requested: version `any`, OS `linux`, architecture `amd64`, minimum stability `stable`, asset type `any`. +Tried 1 repository(ies): + 1) github `roadrunner-server/roadrunner` + GitHub API rejected the credentials (HTTP 401: Bad credentials). + The API token from the GITHUB_TOKEN environment variable is invalid, expired or revoked. Provide a valid + token or unset the variable to use anonymous access. + +1 of 1 download(s) failed. +``` + +Run with `-vvv` to also get stack traces and the full request log. + ## Gitlab CI configuration When you make a release in Gitlab, make sure to upload your assets to the release page via diff --git a/psalm-baseline.xml b/psalm-baseline.xml index 0f1aded..ca11921 100644 --- a/psalm-baseline.xml +++ b/psalm-baseline.xml @@ -198,6 +198,8 @@ + release->getAssets()]]> + @@ -220,7 +222,9 @@ + + diff --git a/src/Command/Get.php b/src/Command/Get.php index a165b35..7653508 100644 --- a/src/Command/Get.php +++ b/src/Command/Get.php @@ -10,10 +10,12 @@ use Internal\DLoad\Module\Common\Stability; use Internal\DLoad\Module\Config\Schema\Action\Download as DownloadConfig; use Internal\DLoad\Module\Config\Schema\Actions; +use Internal\DLoad\Module\Downloader\Exception\DownloadFailed; use Internal\DLoad\Service\Container; use Symfony\Component\Console\Attribute\AsCommand; use Symfony\Component\Console\Command\Command; use Symfony\Component\Console\Exception\InvalidArgumentException; +use Symfony\Component\Console\Formatter\OutputFormatter; use Symfony\Component\Console\Input\InputArgument; use Symfony\Component\Console\Input\InputInterface; use Symfony\Component\Console\Input\InputOption; @@ -81,7 +83,8 @@ public function configure(): void * @param InputInterface $input Command input * @param OutputInterface $output Command output * - * @return int Command result code + * @return int `Command::SUCCESS` when every requested package is in place, + * `Command::FAILURE` when at least one download failed * * @throws \RuntimeException When no software is specified to download */ @@ -104,14 +107,43 @@ protected function execute(InputInterface $input, OutputInterface $output): int /** @var DLoad $dload */ $dload = $container->get(DLoad::class); - $forceDownload = $input->getOption('force'); + $forceDownload = (bool) $input->getOption('force'); + /** @var list $failures */ + $failures = []; foreach ($actions as $action) { - $dload->addTask($action, $forceDownload); + try { + $dload->addTask($action, $forceDownload)->then( + null, + static function (\Throwable $e) use (&$failures, $action): void { + $failures[] = [$action->software, $e]; + }, + ); + } catch (\Throwable $e) { + // A task may fail before it is even scheduled, e.g. when the software is unknown + $failures[] = [$action->software, $e]; + } } $dload->run(); - return Command::SUCCESS; + return $failures === [] + ? Command::SUCCESS + : $this->reportFailures($output, $failures, \count($actions)); + } + + /** + * Builds a user-facing explanation of a failure. + */ + private static function describeFailure(\Throwable $error): string + { + $message = $error->getMessage(); + + return match (true) { + // The report already describes the whole context of the failure + $error instanceof DownloadFailed => $error->report, + $message === '' => \sprintf('Unexpected %s without a message.', $error::class), + default => $message, + }; } /** @@ -201,4 +233,28 @@ private static function applyFlags(InputInterface $input, Container $container): "Unknown architecture: {$arch}.", )); } + + /** + * Prints the reason of every failed download and returns a failure exit code. + * + * @param list $failures Software identifier with its failure + * @param int<1, max> $total Total number of requested downloads + */ + private function reportFailures(OutputInterface $output, array $failures, int $total): int + { + foreach ($failures as [$software, $error]) { + $output->writeln(''); + $output->writeln(\sprintf(' Failed to download `%s` ', OutputFormatter::escape($software))); + $output->writeln(OutputFormatter::escape(self::describeFailure($error))); + + $output->isVeryVerbose() and $this->logger->exception($error, important: true); + } + + $output->writeln(''); + $output->writeln( + \sprintf('%d of %d download(s) failed.', \count($failures), $total), + ); + + return Command::FAILURE; + } } diff --git a/src/DLoad.php b/src/DLoad.php index 5bb9e55..96218c8 100644 --- a/src/DLoad.php +++ b/src/DLoad.php @@ -16,6 +16,7 @@ use Internal\DLoad\Module\Config\Schema\Embed\File; use Internal\DLoad\Module\Config\Schema\Embed\Software; use Internal\DLoad\Module\Downloader\Downloader; +use Internal\DLoad\Module\Downloader\Exception\NothingExtracted; use Internal\DLoad\Module\Downloader\SoftwareCollection; use Internal\DLoad\Module\Downloader\Task\DownloadResult; use Internal\DLoad\Module\Downloader\Task\DownloadTask; @@ -234,11 +235,14 @@ private function prepareExtractTask( $extractor = $archive->extract(); $this->logger->info('Extracting %s', $fileInfo->getFilename()); $binaryPattern = $this->generateBinaryExtractionConfig($software->binary); + $extractionRules = $this->describeExtractionRules($software, $binaryPattern); + $archiveFiles = []; while ($extractor->valid()) { $to = $rule = null; $file = $extractor->current(); \assert($file instanceof \SplFileInfo); + $archiveFiles[] = $file->getFilename(); # Check if it's binary and should be extracted $isBinary = false; @@ -283,6 +287,13 @@ private function prepareExtractTask( } } + # A downloaded asset without a single matching file means nothing was installed + $resultFiles === [] and throw new NothingExtracted( + assetName: $fileInfo->getFilename(), + rules: $extractionRules, + files: $archiveFiles, + ); + return new DloadResult($resultFiles, $resultBinary); } finally { // Cleanup: Delete the temporary downloaded file @@ -336,6 +347,24 @@ private function getDestinationPath(DownloadConfig $action): Path return Path::create($this->configDestination->path ?? $action->extractPath ?? (string) \getcwd()); } + /** + * Lists the patterns applied to archive entries, to explain why nothing was extracted. + * + * @param File|null $binaryPattern Generated binary extraction rule + * @return list + */ + private function describeExtractionRules(Software $software, ?File $binaryPattern): array + { + $rules = []; + $binaryPattern === null or $rules[] = \sprintf('binary `%s`', $binaryPattern->pattern); + + foreach ($software->files as $file) { + $rules[] = \sprintf('file `%s`', $file->pattern); + } + + return $rules; + } + /** * Generates a binary extraction configuration based on the provided binary configuration. * diff --git a/src/Module/Downloader/Downloader.php b/src/Module/Downloader/Downloader.php index 68f828b..2b88e49 100644 --- a/src/Module/Downloader/Downloader.php +++ b/src/Module/Downloader/Downloader.php @@ -10,17 +10,21 @@ use Internal\DLoad\Module\Common\FileSystem\FS; use Internal\DLoad\Module\Common\OperatingSystem; use Internal\DLoad\Module\Common\Stability; -use Internal\DLoad\Module\Config\Schema\Action\Download; use Internal\DLoad\Module\Config\Schema\Action\Download as DownloadConfig; use Internal\DLoad\Module\Config\Schema\Action\Type; use Internal\DLoad\Module\Config\Schema\Downloader as DownloaderConfig; use Internal\DLoad\Module\Config\Schema\Embed\Software; +use Internal\DLoad\Module\Downloader\Exception\DownloadFailed; use Internal\DLoad\Module\Downloader\Exception\NotFound; +use Internal\DLoad\Module\Downloader\Internal\Diagnostics\DownloadDiagnostics; use Internal\DLoad\Module\Downloader\Internal\DownloadContext; use Internal\DLoad\Module\Downloader\Task\DownloadResult; use Internal\DLoad\Module\Downloader\Task\DownloadTask; use Internal\DLoad\Module\Repository\AssetInterface; use Internal\DLoad\Module\Repository\Collection\AssetsCollection; +use Internal\DLoad\Module\Repository\Collection\ReleasesCollection; +use Internal\DLoad\Module\Repository\Exception\RateLimitException; +use Internal\DLoad\Module\Repository\Exception\RepositoryException; use Internal\DLoad\Module\Repository\ReleaseInterface; use Internal\DLoad\Module\Repository\Repository; use Internal\DLoad\Module\Repository\RepositoryProvider; @@ -51,6 +55,9 @@ */ final class Downloader { + /** Number of release names collected for the failure report. */ + private const FETCHED_RELEASES_LIMIT = 10; + public function __construct( private readonly DownloaderConfig $config, private readonly Logger $logger, @@ -83,6 +90,13 @@ public function download( onProgress: $onProgress, actionConfig: $actionConfig, tempDir: $this->getTempDirectory(), + diagnostics: new DownloadDiagnostics( + software: $software, + actionConfig: $actionConfig, + operatingSystem: $this->operatingSystem, + architecture: $this->architecture, + stability: $this->stability, + ), ); $repositories = $software->repositories; @@ -90,9 +104,17 @@ public function download( return coroutine(function () use ($repositories, $context) { // Try every repo to load software. start: - $repositories === [] and throw new NotFound('No relevant repository found.'); + $repositories === [] and throw new DownloadFailed( + software: $context->software->getId(), + report: $context->diagnostics->render(), + ); $context->repoConfig = \array_shift($repositories); $repository = $this->repositoryProvider->getByConfig($context->repoConfig); + $context->repositoryAttempt = $context->diagnostics->addRepository( + type: $context->repoConfig->type, + name: $repository->getName(), + assetPattern: $context->repoConfig->assetPattern, + ); $this->logger->debug('Trying to load from repo `%s`', $repository->getName()); @@ -104,11 +126,18 @@ public function download( version: $context->release->getVersion(), ); } catch (NotFound $e) { + // Nothing suitable in this repository: the reason is already in the diagnostics + $this->logger->debug($e->getMessage()); + goto start; + } catch (RepositoryException $e) { + // The repository is unusable (API error, invalid token, rate limit, etc.): + // remember the reason and fall back to the next repository. + $context->repositoryAttempt->error = $e; $this->logger->debug($e->getMessage()); + $this->logger->exception($e, important: false); goto start; } catch (\Throwable $e) { - $this->logger->error($e->getMessage()); - $this->logger->exception($e); + $this->logger->exception($e, important: false); throw $e; } finally { $repository instanceof Destroyable and $repository->destroy(); @@ -141,14 +170,15 @@ private function processRepository(Repository $repository, DownloadContext $cont $repository->getName(), ); + $allReleases = $repository->getReleases(); if ($context->actionConfig->version !== null) { $constraint = Constraint::fromConstraintString($context->actionConfig->version); // Filter by version if specified - $releasesCollection = $repository->getReleases() + $releasesCollection = $allReleases ->minimumStability($constraint->minimumStability) ->satisfies($constraint); } else { - $releasesCollection = $repository->getReleases() + $releasesCollection = $allReleases ->minimumStability($this->stability); } @@ -160,18 +190,29 @@ private function processRepository(Repository $repository, DownloadContext $cont // Try without limit $releases === [] and $releases = $releasesCollection->limit(0)->toArray(); + $context->repositoryAttempt->matchedReleases = \count($releases); + + if ($releases === []) { + // Show what the repository actually offers: it explains version and stability mismatches + $context->repositoryAttempt->registerFetchedReleases($this->fetchReleaseNames($allReleases)); + + throw new NotFound('No relevant release found.'); + } + process_release: $releases === [] and throw new NotFound('No relevant release found.'); $context->release = \array_shift($releases); + $context->releaseAttempt = $context->repositoryAttempt->addRelease($context->release->getName()); - $this->logger->info('Loading release `%s`', $context->release->getName()); + $this->logger->debug('Loading release `%s`', $context->release->getName()); try { await(coroutine($this->processRelease($context))); return $context->release; } catch (NotFound $e) { + $context->releaseAttempt->reason ??= $e->getMessage(); $this->logger->debug($e->getMessage()); - $this->logger->exception($e); + $this->logger->exception($e, important: false); goto process_release; } }; @@ -188,13 +229,23 @@ private function processRepository(Repository $repository, DownloadContext $cont */ private function processRelease(DownloadContext $context): \Closure { - return fn(): AssetInterface => match (true) { - // Phar assets usually don't depend on OS or architecture, so we can use gradual filtering - $context->actionConfig->type === Type::Phar => $this->findAssetWithGradualFiltering($context), - // Use strict filtering when binary configuration exists - $context->software->binary !== null => $this->findAssetWithStrictFiltering($context), - // Use gradual filtering when no binary configuration exists - default => $this->findAssetWithGradualFiltering($context), + return function () use ($context): AssetInterface { + // Remember all the release assets: it makes a "nothing matched" report meaningful + $names = []; + foreach ($context->release->getAssets() as $asset) { + $names[] = $asset->getName(); + } + + $context->releaseAttempt->registerAssets($names); + + return match (true) { + // Phar assets usually don't depend on OS or architecture, so we can use gradual filtering + $context->actionConfig->type === Type::Phar => $this->findAssetWithGradualFiltering($context), + // Use strict filtering when binary configuration exists + $context->software->binary !== null => $this->findAssetWithStrictFiltering($context), + // Use gradual filtering when no binary configuration exists + default => $this->findAssetWithGradualFiltering($context), + }; }; } @@ -217,7 +268,15 @@ private function findAssetWithStrictFiltering(DownloadContext $context): AssetIn $allAssets = $this->addFormatFilter($assetsCollection, $context->actionConfig)->toArray(); $this->logger->debug('%d matching assets found.', \count($allAssets)); - $allAssets === [] and throw new NotFound('No relevant assets found.'); + $allAssets === [] and throw new NotFound( + \sprintf( + 'no asset matches OS `%s`, architecture `%s`, name pattern `%s`%s', + $this->operatingSystem->value, + $this->architecture->value, + $context->repoConfig->assetPattern, + $this->describeFormatFilter($context->actionConfig), + ), + ); // Sort assets by priority and try to process them $sortedAssets = $this->sortAssetsByPriority($allAssets, $this->archiveService->getSupportedExtensions()); @@ -241,7 +300,13 @@ private function findAssetWithGradualFiltering(DownloadContext $context): AssetI $supportedExtensions = $this->archiveService->getSupportedExtensions(); // If we got here, no assets were found with any filter combination - \count($assetsCollection) === 0 and throw new NotFound('No relevant assets found.'); + \count($assetsCollection) === 0 and throw new NotFound( + \sprintf( + 'no asset matches name pattern `%s`%s', + $context->repoConfig->assetPattern, + $this->describeFormatFilter($context->actionConfig), + ), + ); // Try #1: Filter by both OS and architecture (most specific) $filteredAssets = $assetsCollection @@ -279,7 +344,7 @@ private function findAssetWithGradualFiltering(DownloadContext $context): AssetI $sortedAssets = $this->sortAssetsByPriority($filteredAssets, $supportedExtensions); try { return $this->tryProcessAssets($sortedAssets, $context); - } catch (\RuntimeException $e) { + } catch (NotFound $e) { $this->logger->debug('Failed to process assets with OS-only filtering: %s', $e->getMessage()); // Continue to next filter strategy } @@ -299,7 +364,7 @@ private function findAssetWithGradualFiltering(DownloadContext $context): AssetI $sortedAssets = $this->sortAssetsByPriority($filteredAssets, $supportedExtensions); try { return $this->tryProcessAssets($sortedAssets, $context); - } catch (\RuntimeException $e) { + } catch (NotFound $e) { $this->logger->debug('Failed to process assets with architecture-only filtering: %s', $e->getMessage()); // Continue to next filter strategy } @@ -327,18 +392,56 @@ private function findAssetWithGradualFiltering(DownloadContext $context): AssetI private function tryProcessAssets(array $assets, DownloadContext $context): AssetInterface { process_asset: - $assets === [] and throw new NotFound('No relevant asset found.'); + $assets === [] and throw new NotFound('none of the matching assets could be downloaded'); $context->asset = \array_shift($assets); $this->logger->debug('Trying to load asset `%s`', $context->asset->getName()); try { await(coroutine($this->processAsset($context))); return $context->asset; + } catch (RateLimitException $e) { + // Retrying other assets makes the situation worse: report the limit immediately + throw $e; } catch (\Throwable $e) { - $this->logger->exception($e); + $context->releaseAttempt->addFailure($context->asset->getName(), $e); + $this->logger->exception($e, important: false); goto process_asset; } } + /** + * Collects names of the first releases available in the repository for a failure report. + * + * @return list + */ + private function fetchReleaseNames(ReleasesCollection $releases): array + { + $names = []; + foreach ($releases as $release) { + $names[] = $release->getName(); + + if (\count($names) >= self::FETCHED_RELEASES_LIMIT) { + break; + } + } + + return $names; + } + + /** + * Describes the asset format restriction for failure reports. + */ + private function describeFormatFilter(DownloadConfig $actionOptions): string + { + return match ($actionOptions->type) { + Type::Phar => ' and the `phar` extension', + Type::Archive => \sprintf( + ' and one of the archive extensions: %s', + \implode(', ', $this->archiveService->getSupportedExtensions()), + ), + default => '', + }; + } + /** * Sorts assets by priority with supported archives first, then other files. * @@ -434,10 +537,10 @@ private function getTempDirectory(): Path * Adds format filter to the assets collection if specified in action options. * * @param AssetsCollection $collection Collection of assets to filter - * @param Download $actionOptions Download action options + * @param DownloadConfig $actionOptions Download action options * @return AssetsCollection Filtered collection */ - private function addFormatFilter(AssetsCollection $collection, Download $actionOptions): AssetsCollection + private function addFormatFilter(AssetsCollection $collection, DownloadConfig $actionOptions): AssetsCollection { return match ($actionOptions->type) { Type::Phar => $collection->whereFileExtensions(['phar']), diff --git a/src/Module/Downloader/Exception/DownloadFailed.php b/src/Module/Downloader/Exception/DownloadFailed.php new file mode 100644 index 0000000..2d53d21 --- /dev/null +++ b/src/Module/Downloader/Exception/DownloadFailed.php @@ -0,0 +1,31 @@ + $rules Patterns that were applied to the archive entries + * @param list $files Names of the files found in the archive + */ + public function __construct( + string $assetName, + array $rules, + array $files, + ) { + $listed = \array_slice($files, 0, self::FILES_LIMIT); + $hidden = \count($files) - \count($listed); + + parent::__construct( + \sprintf( + "Nothing was extracted from `%s`: none of the %d file(s) inside matches the extraction rules.\n" + . "Extraction rules: %s\n" + . 'Files in the asset: %s%s', + $assetName, + \count($files), + $rules === [] ? 'none' : \implode(', ', $rules), + $listed === [] ? 'none' : \implode(', ', $listed), + $hidden > 0 ? \sprintf(' and %d more', $hidden) : '', + ), + ); + } +} diff --git a/src/Module/Downloader/Internal/Diagnostics/DownloadDiagnostics.php b/src/Module/Downloader/Internal/Diagnostics/DownloadDiagnostics.php new file mode 100644 index 0000000..7161e0f --- /dev/null +++ b/src/Module/Downloader/Internal/Diagnostics/DownloadDiagnostics.php @@ -0,0 +1,97 @@ +addRepository('github', 'owner/repo', '/^.*$/'); + * $repository->matchedReleases = 0; + * throw new DownloadFailed($diagnostics->render()); + * ``` + * + * @internal + */ +final class DownloadDiagnostics +{ + /** @var list */ + private array $repositories = []; + + public function __construct( + private readonly Software $software, + private readonly DownloadConfig $actionConfig, + private readonly OperatingSystem $operatingSystem, + private readonly Architecture $architecture, + private readonly Stability $stability, + ) {} + + /** + * @param non-empty-string $type Repository type, e.g. `github` + * @param non-empty-string $name Repository name, e.g. `owner/repo` + * @param non-empty-string $assetPattern Asset name pattern from the configuration + */ + public function addRepository(string $type, string $name, string $assetPattern): RepositoryAttempt + { + return $this->repositories[] = new RepositoryAttempt($type, $name, $assetPattern); + } + + /** + * Builds the human-readable report about all the attempts. + * + * @return non-empty-string + */ + public function render(): string + { + $lines = [$this->renderRequest()]; + + if ($this->repositories === []) { + $lines[] = \sprintf( + 'No repositories are configured for `%s`. Add a `repository` entry to the software definition.', + $this->software->getId(), + ); + + return \implode("\n", $lines); + } + + $lines[] = \sprintf('Tried %d repository(ies):', \count($this->repositories)); + + foreach ($this->repositories as $index => $repository) { + foreach ($repository->describe() as $lineIndex => $line) { + $lines[] = $lineIndex === 0 + ? \sprintf(' %d) %s', $index + 1, $line) + : ' ' . $line; + } + } + + return \implode("\n", $lines); + } + + /** + * @return non-empty-string + */ + private function renderRequest(): string + { + return \sprintf( + 'Requested: version `%s`, OS `%s`, architecture `%s`, minimum stability `%s`, asset type `%s`.', + $this->actionConfig->version ?? 'any', + $this->operatingSystem->value, + $this->architecture->value, + $this->stability->value, + $this->actionConfig->type?->value ?? 'any', + ); + } +} diff --git a/src/Module/Downloader/Internal/Diagnostics/ReleaseAttempt.php b/src/Module/Downloader/Internal/Diagnostics/ReleaseAttempt.php new file mode 100644 index 0000000..ec0c57e --- /dev/null +++ b/src/Module/Downloader/Internal/Diagnostics/ReleaseAttempt.php @@ -0,0 +1,88 @@ +|null Total number of assets in the release, or null when the list was not fetched */ + public ?int $assetsTotal = null; + + /** @var string|null Why the release was rejected */ + public ?string $reason = null; + + /** @var list Names of all assets in the release */ + private array $assetNames = []; + + /** @var list Errors occurred while downloading matched assets */ + private array $failures = []; + + /** + * @param non-empty-string $name Release name or tag + */ + public function __construct( + public readonly string $name, + ) {} + + /** + * @param list $names Names of all assets available in the release + */ + public function registerAssets(array $names): void + { + $this->assetsTotal = \count($names); + $this->assetNames = $names; + } + + /** + * Registers a failure of a matched asset, e.g. a broken download. + * + * @param non-empty-string $assetName + */ + public function addFailure(string $assetName, \Throwable $error): void + { + $this->failures[] = \sprintf('`%s`: %s', $assetName, $error->getMessage()); + } + + /** + * Renders release details as report lines. + * + * @return list + */ + public function describe(): array + { + $lines = [ + \sprintf( + '%s: %s%s', + $this->name, + // A missing asset list (an error before it was fetched) is not the same as an empty one + $this->assetsTotal === null ? 'asset list not loaded' : \sprintf('%d asset(s)', $this->assetsTotal), + $this->reason === null ? '' : ', ' . $this->reason, + ), + ]; + + if ($this->assetNames !== []) { + $listed = \array_slice($this->assetNames, 0, self::ASSETS_LIMIT); + $hidden = \count($this->assetNames) - \count($listed); + $lines[] = \sprintf( + ' Assets: %s%s', + \implode(', ', $listed), + $hidden > 0 ? \sprintf(' and %d more', $hidden) : '', + ); + } + + foreach ($this->failures as $failure) { + $lines[] = ' Failed asset ' . $failure; + } + + return $lines; + } +} diff --git a/src/Module/Downloader/Internal/Diagnostics/RepositoryAttempt.php b/src/Module/Downloader/Internal/Diagnostics/RepositoryAttempt.php new file mode 100644 index 0000000..0d7a135 --- /dev/null +++ b/src/Module/Downloader/Internal/Diagnostics/RepositoryAttempt.php @@ -0,0 +1,108 @@ +|null Number of releases that match the version constraint and stability */ + public ?int $matchedReleases = null; + + /** @var list Release names fetched from the repository */ + private array $fetchedReleases = []; + + /** @var list */ + private array $releases = []; + + /** @var int<0, max> Number of releases processed but not included into the report */ + private int $hiddenReleases = 0; + + /** + * @param non-empty-string $type Repository type, e.g. `github` + * @param non-empty-string $name Repository name, e.g. `owner/repo` + * @param non-empty-string $assetPattern Asset name pattern from the configuration + */ + public function __construct( + public readonly string $type, + public readonly string $name, + public readonly string $assetPattern, + ) {} + + /** + * @param list $names Release names available in the repository + */ + public function registerFetchedReleases(array $names): void + { + $this->fetchedReleases = $names; + } + + /** + * @param non-empty-string $name Release name or tag + */ + public function addRelease(string $name): ReleaseAttempt + { + $attempt = new ReleaseAttempt($name); + + if (\count($this->releases) < self::RELEASES_LIMIT) { + $this->releases[] = $attempt; + } else { + ++$this->hiddenReleases; + } + + return $attempt; + } + + /** + * Renders repository details as report lines. + * + * @return list + */ + public function describe(): array + { + $lines = [\sprintf('%s `%s`', $this->type, $this->name)]; + + if ($this->error !== null) { + foreach (\explode("\n", $this->error->getMessage()) as $line) { + $lines[] = ' ' . $line; + } + } + + $this->matchedReleases === null or $lines[] = \sprintf( + ' %d release(s) match the requested version and stability.', + $this->matchedReleases, + ); + + if ($this->matchedReleases === 0 && $this->fetchedReleases !== []) { + $listed = \array_slice($this->fetchedReleases, 0, self::FETCHED_RELEASES_LIMIT); + $lines[] = \sprintf(' Releases available in the repository: %s', \implode(', ', $listed)); + } + + if ($this->releases !== []) { + $lines[] = ' Checked releases:'; + foreach ($this->releases as $release) { + foreach ($release->describe() as $index => $line) { + $lines[] = $index === 0 ? ' - ' . $line : ' ' . $line; + } + } + + $this->hiddenReleases === 0 or $lines[] = \sprintf(' and %d more release(s)', $this->hiddenReleases); + } + + return $lines; + } +} diff --git a/src/Module/Downloader/Internal/DownloadContext.php b/src/Module/Downloader/Internal/DownloadContext.php index cedaf14..9db0fc3 100644 --- a/src/Module/Downloader/Internal/DownloadContext.php +++ b/src/Module/Downloader/Internal/DownloadContext.php @@ -7,6 +7,9 @@ use Internal\DLoad\Module\Config\Schema\Action\Download as DownloadConfig; use Internal\DLoad\Module\Config\Schema\Embed\Repository; use Internal\DLoad\Module\Config\Schema\Embed\Software; +use Internal\DLoad\Module\Downloader\Internal\Diagnostics\DownloadDiagnostics; +use Internal\DLoad\Module\Downloader\Internal\Diagnostics\ReleaseAttempt; +use Internal\DLoad\Module\Downloader\Internal\Diagnostics\RepositoryAttempt; use Internal\DLoad\Module\Repository\AssetInterface; use Internal\DLoad\Module\Repository\ReleaseInterface; use Internal\DLoad\Module\Task\Progress; @@ -34,6 +37,12 @@ final class DownloadContext /** @var ReleaseInterface Current release being processed */ public ReleaseInterface $release; + /** @var RepositoryAttempt Diagnostics of the repository being processed */ + public RepositoryAttempt $repositoryAttempt; + + /** @var ReleaseAttempt Diagnostics of the release being processed */ + public ReleaseAttempt $releaseAttempt; + /** * Creates a new download context. * @@ -42,11 +51,13 @@ final class DownloadContext * Exception thrown in this callback will stop and revert the task. * @param DownloadConfig $actionConfig Download action configuration * @param Path $tempDir Temporary directory for downloads + * @param DownloadDiagnostics $diagnostics Collector of failure reasons for the final report */ public function __construct( public readonly Software $software, public readonly \Closure $onProgress, public readonly DownloadConfig $actionConfig, public readonly Path $tempDir, + public readonly DownloadDiagnostics $diagnostics, ) {} } diff --git a/src/Module/Repository/Exception/AccessDeniedException.php b/src/Module/Repository/Exception/AccessDeniedException.php new file mode 100644 index 0000000..d322206 --- /dev/null +++ b/src/Module/Repository/Exception/AccessDeniedException.php @@ -0,0 +1,12 @@ + 'application/vnd.github.v3+json', ]; + private readonly ResponseValidator $validator; + public function __construct( private readonly HttpFactory $httpFactory, private readonly ClientInterface $client, @@ -39,13 +41,14 @@ public function __construct( ) { // Add authorization header if token is available $this->gitHubConfig->token !== null and $this->defaultHeaders['authorization'] = 'Bearer ' . $this->gitHubConfig->token; + + $this->validator = new ResponseValidator(authenticated: $this->gitHubConfig->token !== null); } /** * @param Method|non-empty-string $method * @param array $headers - * @throws GitHubRateLimitException - * @throws ClientExceptionInterface + * @throws RepositoryException */ public function request(Method|string $method, string|UriInterface $uri, array $headers = []): ResponseInterface { @@ -55,45 +58,18 @@ public function request(Method|string $method, string|UriInterface $uri, array $ } /** - * @throws GitHubRateLimitException - * @throws ClientExceptionInterface + * @throws RepositoryException */ public function sendRequest(RequestInterface $request): ResponseInterface { - $response = $this->client->sendRequest($request); - - $this->checkForRateLimit($response); - - return $response; - } - - /** - * @throws GitHubRateLimitException - */ - private function checkForRateLimit(ResponseInterface $response): void - { - // GitHub rate limit responses typically have 403 status - if ($response->getStatusCode() !== 403) { - return; + try { + $response = $this->client->sendRequest($request); + } catch (ClientExceptionInterface $e) { + throw $this->validator->transportFailure($request, $e); } - $body = $response->getBody()->__toString(); + $this->validator->validate($request, $response); - try { - /** @var mixed $decoded */ - $decoded = \json_decode($body, true, 512, JSON_THROW_ON_ERROR); - - // GitHub rate limit responses have format: ["API rate limit ...", "https://docs.github.com/..."] - if (\is_array($decoded) - && \count($decoded) === 2 - && \is_string(\reset($decoded)) - && \is_string(\next($decoded)) - && \str_contains(\reset($decoded), 'API rate limit') - ) { - throw GitHubRateLimitException::fromApiResponse($decoded); - } - } catch (\JsonException) { - // Not a JSON response, continue without rate limit check - } + return $response; } } diff --git a/src/Module/Repository/Internal/GitHub/Api/RepositoryApi.php b/src/Module/Repository/Internal/GitHub/Api/RepositoryApi.php index 943d5aa..c3ce8fb 100644 --- a/src/Module/Repository/Internal/GitHub/Api/RepositoryApi.php +++ b/src/Module/Repository/Internal/GitHub/Api/RepositoryApi.php @@ -6,12 +6,12 @@ use Internal\DLoad\Module\HttpClient\Factory as HttpFactory; use Internal\DLoad\Module\HttpClient\Method; +use Internal\DLoad\Module\Repository\Exception\ApiException; +use Internal\DLoad\Module\Repository\Exception\RepositoryException; use Internal\DLoad\Module\Repository\Internal\GitHub\Api\Response\ReleaseInfo; use Internal\DLoad\Module\Repository\Internal\GitHub\Api\Response\RepositoryInfo; -use Internal\DLoad\Module\Repository\Internal\GitHub\Exception\GitHubRateLimitException; use Internal\DLoad\Module\Repository\Internal\Paginator; use Internal\DLoad\Service\Logger; -use Psr\Http\Client\ClientExceptionInterface; use Psr\Http\Message\ResponseInterface; use Psr\Http\Message\UriInterface; @@ -50,8 +50,7 @@ public function __construct( /** * @param Method|non-empty-string $method * @param array $headers - * @throws GitHubRateLimitException - * @throws ClientExceptionInterface + * @throws RepositoryException */ public function request(Method|string $method, string|UriInterface $uri, array $headers = []): ResponseInterface { @@ -59,8 +58,7 @@ public function request(Method|string $method, string|UriInterface $uri, array $ } /** - * @throws GitHubRateLimitException - * @throws ClientExceptionInterface + * @throws RepositoryException */ public function getRepository(): RepositoryInfo { @@ -83,8 +81,7 @@ public function getRepository(): RepositoryInfo /** * @param int<1, max> $page * @return Paginator - * @throws GitHubRateLimitException - * @throws ClientExceptionInterface + * @throws RepositoryException */ public function getReleases(int $page = 1): Paginator { @@ -92,59 +89,110 @@ public function getReleases(int $page = 1): Paginator $currentPage = $page; do { - try { - $response = $this->releasesRequest($currentPage); - - /** @var array, - * prerelease: bool, - * draft: bool - * }> $data */ - $data = \json_decode($response->getBody()->__toString(), true, 512, JSON_THROW_ON_ERROR); - - // If empty response, no more pages - if ($data === []) { - return; - } + $response = $this->releasesRequest($currentPage); + + /** @var list, + * prerelease: bool, + * draft: bool + * }> $data */ + $data = $this->decodeReleasesResponse($response); + + // If empty response, no more pages + if ($data === []) { + return; + } - $releases = []; - foreach ($data as $releaseData) { - try { - $releases[] = ReleaseInfo::fromApiResponse($releaseData); - } catch (\Throwable $e) { - $this->logger->exception($e, important: false); - // Skip invalid releases - continue; - } + $releases = []; + $failure = null; + foreach ($data as $releaseData) { + try { + $releases[] = ReleaseInfo::fromApiResponse($releaseData); + } catch (\Throwable $e) { + $failure ??= $e; + $this->logger->exception($e, important: false); + // Skip invalid releases + continue; } + } - yield $releases; - - // Check if there are more pages - $hasMorePages = $this->hasNextPage($response); - $currentPage++; - } catch (ClientExceptionInterface $e) { - $this->logger->exception($e, important: false); - return; + // The whole page is unreadable: the response structure is not what we expect + if ($releases === [] && $failure !== null) { + throw new ApiException( + \sprintf( + 'GitHub API returned %d release(s) for repository `%s`, but none of them could be read: %s', + \count($data), + $this->repositoryPath, + $failure->getMessage(), + ), + $this->repositoryPath, + $failure, + ); } + + yield $releases; + + // Check if there are more pages + $hasMorePages = $this->hasNextPage($response); + $currentPage++; } while ($hasMorePages); }; return Paginator::createFromGenerator($pageLoader(), null); } + /** + * Decodes a releases list response and validates its shape. + * + * @return list> + * @throws ApiException When the response is not a list of releases. + */ + private function decodeReleasesResponse(ResponseInterface $response): array + { + $body = $response->getBody()->__toString(); + + try { + /** @var mixed $data */ + $data = \json_decode($body, true, 512, JSON_THROW_ON_ERROR); + } catch (\JsonException $e) { + throw new ApiException( + \sprintf( + 'GitHub API returned a malformed response for repository `%s`: %s', + $this->repositoryPath, + $e->getMessage(), + ), + $this->repositoryPath, + $e, + ); + } + + if (!\is_array($data) || !\array_is_list($data)) { + throw new ApiException( + \sprintf( + 'GitHub API returned an unexpected response for repository `%s`: ' + . 'a list of releases is expected, got %s.', + $this->repositoryPath, + \is_array($data) ? 'an object: ' . \substr($body, 0, 200) : \get_debug_type($data), + ), + $this->repositoryPath, + ); + } + + /** @var list> */ + return $data; + } + /** * @param positive-int $page - * @throws GitHubRateLimitException - * @throws ClientExceptionInterface + * @throws RepositoryException */ private function releasesRequest(int $page): ResponseInterface { diff --git a/src/Module/Repository/Internal/GitHub/Api/ResponseValidator.php b/src/Module/Repository/Internal/GitHub/Api/ResponseValidator.php new file mode 100644 index 0000000..01ca4fb --- /dev/null +++ b/src/Module/Repository/Internal/GitHub/Api/ResponseValidator.php @@ -0,0 +1,64 @@ + - * @throws ClientExceptionInterface + * @throws RepositoryException */ public function download(?\Closure $progress = null): \Generator { diff --git a/src/Module/Repository/Internal/GitHub/GitHubRepository.php b/src/Module/Repository/Internal/GitHub/GitHubRepository.php index ee0ecdd..2c4d9be 100644 --- a/src/Module/Repository/Internal/GitHub/GitHubRepository.php +++ b/src/Module/Repository/Internal/GitHub/GitHubRepository.php @@ -6,8 +6,8 @@ use Internal\Destroy\Destroyable; use Internal\DLoad\Module\Repository\Collection\ReleasesCollection; +use Internal\DLoad\Module\Repository\Exception\RateLimitException; use Internal\DLoad\Module\Repository\Internal\GitHub\Api\RepositoryApi; -use Internal\DLoad\Module\Repository\Internal\GitHub\Exception\GitHubRateLimitException; use Internal\DLoad\Module\Repository\Repository; use Internal\DLoad\Service\Logger; @@ -54,6 +54,7 @@ public function getReleases(): ReleasesCollection // Create a generator function for lazy loading release pages $pageLoader = function (): \Generator { $page = 0; + $anyPageLoaded = false; do { try { @@ -74,12 +75,22 @@ public function getReleases(): ReleasesCollection } } yield $toYield; + $anyPageLoaded = true; // Check if there are more pages by getting next page $hasMorePages = $paginator->getNextPage() !== null; - } catch (GitHubRateLimitException $e) { - throw $e; - } catch (\Throwable) { + } catch (\Throwable $e) { + # The first page is mandatory: when it fails, there is nothing to download and the reason + # (invalid token, rate limit, missing repository, etc.) must reach the user. + $anyPageLoaded or throw $e; + + # A rate limit leaves the release list incomplete: hiding it would produce a report + # that claims the repository has nothing more, so it must reach the user as well. + $e instanceof RateLimitException and throw $e; + + # Already loaded releases are enough to continue, so a failure of a subsequent page + # only stops the pagination. + $this->logger->exception($e, important: false); return; } } while ($hasMorePages); diff --git a/src/Module/Repository/Internal/GitLab/Api/Client.php b/src/Module/Repository/Internal/GitLab/Api/Client.php index 634408e..2306957 100644 --- a/src/Module/Repository/Internal/GitLab/Api/Client.php +++ b/src/Module/Repository/Internal/GitLab/Api/Client.php @@ -7,7 +7,7 @@ use Internal\DLoad\Module\Config\Schema\GitLab; use Internal\DLoad\Module\HttpClient\Factory as HttpFactory; use Internal\DLoad\Module\HttpClient\Method; -use Internal\DLoad\Module\Repository\Internal\GitLab\Exception\GitLabRateLimitException; +use Internal\DLoad\Module\Repository\Exception\RepositoryException; use Psr\Http\Client\ClientExceptionInterface; use Psr\Http\Client\ClientInterface; use Psr\Http\Message\RequestInterface; @@ -17,8 +17,8 @@ /** * HTTP client wrapper with GitLab-specific error handling and authentication. * - * Detects and handles GitLab Rate Limit responses automatically. - * Adds GitLab API token authentication when available. + * Converts unsuccessful responses (rate limits, invalid token, missing project, etc.) + * into exceptions with actionable messages. Adds GitLab API token authentication when available. * * @internal * @psalm-internal Internal\DLoad\Module\Repository\Internal\GitLab @@ -32,6 +32,8 @@ final class Client 'accept' => 'application/json', ]; + private readonly ResponseValidator $validator; + public function __construct( private readonly HttpFactory $httpFactory, private readonly ClientInterface $client, @@ -39,11 +41,12 @@ public function __construct( ) { // Add authorization header if token is available $this->gitLabConfig->token !== null and $this->defaultHeaders['authorization'] = 'Bearer ' . $this->gitLabConfig->token; + + $this->validator = new ResponseValidator(authenticated: $this->gitLabConfig->token !== null); } /** - * @throws GitLabRateLimitException - * @throws ClientExceptionInterface + * @throws RepositoryException */ public function downloadArtifact(string|UriInterface $uri): ResponseInterface { @@ -62,8 +65,7 @@ public function downloadArtifact(string|UriInterface $uri): ResponseInterface /** * @param Method|non-empty-string $method * @param array $headers - * @throws GitLabRateLimitException - * @throws ClientExceptionInterface + * @throws RepositoryException */ public function request(Method|string $method, string|UriInterface $uri, array $headers = []): ResponseInterface { @@ -73,17 +75,18 @@ public function request(Method|string $method, string|UriInterface $uri, array $ } /** - * @throws GitLabRateLimitException - * @throws ClientExceptionInterface + * @throws RepositoryException */ public function sendRequest(RequestInterface $request): ResponseInterface { - $response = $this->client->sendRequest($request); - - if ($response->getStatusCode() === 429) { - throw new GitLabRateLimitException(); + try { + $response = $this->client->sendRequest($request); + } catch (ClientExceptionInterface $e) { + throw $this->validator->transportFailure($request, $e); } + $this->validator->validate($request, $response); + return $response; } } diff --git a/src/Module/Repository/Internal/GitLab/Api/RepositoryApi.php b/src/Module/Repository/Internal/GitLab/Api/RepositoryApi.php index 020aeb1..fab47a3 100644 --- a/src/Module/Repository/Internal/GitLab/Api/RepositoryApi.php +++ b/src/Module/Repository/Internal/GitLab/Api/RepositoryApi.php @@ -6,11 +6,11 @@ use Internal\DLoad\Module\HttpClient\Factory as HttpFactory; use Internal\DLoad\Module\HttpClient\Method; +use Internal\DLoad\Module\Repository\Exception\ApiException; +use Internal\DLoad\Module\Repository\Exception\RepositoryException; use Internal\DLoad\Module\Repository\Internal\GitLab\Api\Response\ReleaseInfo; use Internal\DLoad\Module\Repository\Internal\GitLab\Api\Response\RepositoryInfo; -use Internal\DLoad\Module\Repository\Internal\GitLab\Exception\GitLabRateLimitException; use Internal\DLoad\Module\Repository\Internal\Paginator; -use Psr\Http\Client\ClientExceptionInterface; use Psr\Http\Message\ResponseInterface; use Psr\Http\Message\UriInterface; @@ -45,8 +45,7 @@ public function __construct( * @param non-empty-string $repositoryPath * @param non-empty-string $releaseName * @param non-empty-string $fileName - * @throws GitLabRateLimitException - * @throws ClientExceptionInterface + * @throws RepositoryException */ public function downloadArtifact(string $repositoryPath, string $releaseName, string $fileName): ResponseInterface { @@ -57,8 +56,7 @@ public function downloadArtifact(string $repositoryPath, string $releaseName, st /** * @param Method|non-empty-string $method * @param array $headers - * @throws GitLabRateLimitException - * @throws ClientExceptionInterface + * @throws RepositoryException */ public function request(Method|string $method, string|UriInterface $uri, array $headers = []): ResponseInterface { @@ -66,8 +64,7 @@ public function request(Method|string $method, string|UriInterface $uri, array $ } /** - * @throws GitLabRateLimitException - * @throws ClientExceptionInterface + * @throws RepositoryException */ public function getRepository(): RepositoryInfo { @@ -90,8 +87,7 @@ public function getRepository(): RepositoryInfo /** * @param int<1, max> $page * @return Paginator - * @throws GitLabRateLimitException - * @throws ClientExceptionInterface + * @throws RepositoryException */ public function getReleases(int $page = 1): Paginator { @@ -99,60 +95,112 @@ public function getReleases(int $page = 1): Paginator $currentPage = $page; do { - try { - $response = $this->releasesRequest($currentPage); - - /** @var array - * }, - * upcoming_release: bool - * }> $data */ - $data = \json_decode($response->getBody()->__toString(), true, 512, JSON_THROW_ON_ERROR); - - // If empty response, no more pages - if ($data === []) { - return; - } + $response = $this->releasesRequest($currentPage); + + /** @var list + * }, + * upcoming_release: bool + * }> $data */ + $data = $this->decodeReleasesResponse($response); + + // If empty response, no more pages + if ($data === []) { + return; + } - $releases = []; - foreach ($data as $releaseData) { - try { - $releases[] = ReleaseInfo::fromApiResponse($releaseData); - } catch (\Throwable) { - // Skip invalid releases - continue; - } + $releases = []; + $failure = null; + foreach ($data as $releaseData) { + try { + $releases[] = ReleaseInfo::fromApiResponse($releaseData); + } catch (\Throwable $e) { + $failure ??= $e; + // Skip invalid releases + continue; } + } - yield $releases; - - // Check if there are more pages - $hasMorePages = $this->hasNextPage($response); - $currentPage++; - } catch (ClientExceptionInterface) { - return; + // The whole page is unreadable: the response structure is not what we expect + if ($releases === [] && $failure !== null) { + throw new ApiException( + \sprintf( + 'GitLab API returned %d release(s) for project `%s`, but none of them could be read: %s', + \count($data), + $this->repositoryPath, + $failure->getMessage(), + ), + $this->repositoryPath, + $failure, + ); } + + yield $releases; + + // Check if there are more pages + $hasMorePages = $this->hasNextPage($response); + $currentPage++; } while ($hasMorePages); }; return Paginator::createFromGenerator($pageLoader(), null); } + /** + * Decodes a releases list response and validates its shape. + * + * @return list> + * @throws ApiException When the response is not a list of releases. + */ + private function decodeReleasesResponse(ResponseInterface $response): array + { + $body = $response->getBody()->__toString(); + + try { + /** @var mixed $data */ + $data = \json_decode($body, true, 512, JSON_THROW_ON_ERROR); + } catch (\JsonException $e) { + throw new ApiException( + \sprintf( + 'GitLab API returned a malformed response for project `%s`: %s', + $this->repositoryPath, + $e->getMessage(), + ), + $this->repositoryPath, + $e, + ); + } + + if (!\is_array($data) || !\array_is_list($data)) { + throw new ApiException( + \sprintf( + 'GitLab API returned an unexpected response for project `%s`: ' + . 'a list of releases is expected, got %s.', + $this->repositoryPath, + \is_array($data) ? 'an object: ' . \substr($body, 0, 200) : \get_debug_type($data), + ), + $this->repositoryPath, + ); + } + + /** @var list> */ + return $data; + } + /** * @param positive-int $page - * @throws GitLabRateLimitException - * @throws ClientExceptionInterface + * @throws RepositoryException */ private function releasesRequest(int $page): ResponseInterface { diff --git a/src/Module/Repository/Internal/GitLab/Api/ResponseValidator.php b/src/Module/Repository/Internal/GitLab/Api/ResponseValidator.php new file mode 100644 index 0000000..6e135ce --- /dev/null +++ b/src/Module/Repository/Internal/GitLab/Api/ResponseValidator.php @@ -0,0 +1,69 @@ +gitLabClient = new Client( $httpFactory, @@ -46,7 +48,7 @@ public function create(RepositoryConfig $config): GitLabRepository $uri = \parse_url($config->uri, PHP_URL_PATH) ?? $config->uri; $api = $this->createRepositoryApi($uri); - return new GitLabRepository($api, $uri); + return new GitLabRepository($api, $uri, $this->logger); } /** diff --git a/src/Module/Repository/Internal/GitLab/GitLabAsset.php b/src/Module/Repository/Internal/GitLab/GitLabAsset.php index b4c9383..9c6a97a 100644 --- a/src/Module/Repository/Internal/GitLab/GitLabAsset.php +++ b/src/Module/Repository/Internal/GitLab/GitLabAsset.php @@ -10,7 +10,7 @@ use Internal\DLoad\Module\Repository\Internal\Asset; use Internal\DLoad\Module\Repository\Internal\GitLab\Api\Response\AssetInfo; use Internal\DLoad\Module\Repository\Internal\GitLab\Api\RepositoryApi; -use Psr\Http\Client\ClientExceptionInterface; +use Internal\DLoad\Module\Repository\Exception\RepositoryException; /** * GitLab Asset class representing a downloadable asset from a GitLab release. @@ -54,7 +54,7 @@ public static function fromDTO( * it SHOULD be called on upload/download of data and at least 1/s * * @return \Generator - * @throws ClientExceptionInterface + * @throws RepositoryException */ public function download(?\Closure $progress = null): \Generator { diff --git a/src/Module/Repository/Internal/GitLab/GitLabRepository.php b/src/Module/Repository/Internal/GitLab/GitLabRepository.php index 8e1c9c4..94f3dff 100644 --- a/src/Module/Repository/Internal/GitLab/GitLabRepository.php +++ b/src/Module/Repository/Internal/GitLab/GitLabRepository.php @@ -6,9 +6,10 @@ use Internal\Destroy\Destroyable; use Internal\DLoad\Module\Repository\Collection\ReleasesCollection; +use Internal\DLoad\Module\Repository\Exception\RateLimitException; use Internal\DLoad\Module\Repository\Internal\GitLab\Api\RepositoryApi; -use Internal\DLoad\Module\Repository\Internal\GitLab\Exception\GitLabRateLimitException; use Internal\DLoad\Module\Repository\Repository; +use Internal\DLoad\Service\Logger; /** * GitLab Repository class representing a GitLab repository. @@ -33,6 +34,7 @@ final class GitLabRepository implements Repository, Destroyable public function __construct( private readonly RepositoryApi $api, string $projectPath, + private readonly Logger $logger, ) { $this->name = $projectPath; } @@ -50,6 +52,7 @@ public function getReleases(): ReleasesCollection // Create a generator function for lazy loading release pages $pageLoader = function (): \Generator { $page = 0; + $anyPageLoaded = false; do { try { @@ -69,12 +72,22 @@ public function getReleases(): ReleasesCollection } } yield $toYield; + $anyPageLoaded = true; // Check if there are more pages by getting next page $hasMorePages = $paginator->getNextPage() !== null; - } catch (GitLabRateLimitException $e) { - throw $e; - } catch (\Throwable) { + } catch (\Throwable $e) { + # The first page is mandatory: when it fails, there is nothing to download and the reason + # (invalid token, rate limit, missing project, etc.) must reach the user. + $anyPageLoaded or throw $e; + + # A rate limit leaves the release list incomplete: hiding it would produce a report + # that claims the project has nothing more, so it must reach the user as well. + $e instanceof RateLimitException and throw $e; + + # Already loaded releases are enough to continue, so a failure of a subsequent page + # only stops the pagination. + $this->logger->exception($e, important: false); return; } } while ($hasMorePages); diff --git a/src/Module/Repository/Internal/ResponseValidator.php b/src/Module/Repository/Internal/ResponseValidator.php new file mode 100644 index 0000000..608a089 --- /dev/null +++ b/src/Module/Repository/Internal/ResponseValidator.php @@ -0,0 +1,422 @@ +getStatusCode(); + if ($status < 400) { + return; + } + + $apiMessage = self::extractApiMessage($response); + $repository = $this->repositoryFromUri((string) $request->getUri()); + $endpoint = \sprintf('%s %s', $request->getMethod(), $request->getUri()); + + if ($this->isRateLimited($status, $response, $apiMessage)) { + throw $this->createRateLimitException($response, $apiMessage, $repository); + } + + throw match (true) { + $status === 401 => new AuthenticationException( + $this->authenticationMessage($apiMessage), + $repository, + ), + $status === 403 => new AccessDeniedException( + $this->accessDeniedMessage($apiMessage, $repository), + $repository, + ), + $status === 404 => new RepositoryNotFoundException( + $this->notFoundMessage($apiMessage, $repository, $endpoint), + $repository, + ), + $status >= 500 => new ApiException( + \sprintf( + '%s API is unavailable: HTTP %d %s (%s).%s Try again later.', + $this->providerName(), + $status, + $response->getReasonPhrase(), + $endpoint, + $apiMessage === null ? '' : ' ' . $apiMessage, + ), + $repository, + ), + default => new ApiException( + \sprintf( + '%s API request failed with HTTP %d %s (%s).%s', + $this->providerName(), + $status, + $response->getReasonPhrase(), + $endpoint, + $apiMessage === null ? '' : ' ' . $apiMessage, + ), + $repository, + ), + }; + } + + /** + * Wraps a transport-level failure (DNS, TLS, timeout, etc.) into a readable exception. + */ + public function transportFailure(RequestInterface $request, \Throwable $e): ApiException + { + return new ApiException( + \sprintf( + 'Failed to reach %s API (%s %s): %s', + $this->providerName(), + $request->getMethod(), + $request->getUri(), + $e->getMessage(), + ), + $this->repositoryFromUri((string) $request->getUri()), + $e, + ); + } + + /** + * @return non-empty-string Provider display name, e.g. `GitHub`. + */ + abstract protected function providerName(): string; + + /** + * @return non-empty-string Environment variable that holds the API token. + */ + abstract protected function tokenEnvVariable(): string; + + /** + * @return non-empty-string How the provider calls a repository, e.g. `repository` or `project`. + */ + protected function repositoryTerm(): string + { + return 'repository'; + } + + /** + * Extracts a repository identifier from an API URL for better error messages. + * + * @return non-empty-string|null + */ + abstract protected function repositoryFromUri(string $uri): ?string; + + /** + * @return positive-int|null Requests per hour allowed without a token. + */ + abstract protected function anonymousRateLimit(): ?int; + + /** + * @return positive-int|null Requests per hour allowed with a token. + */ + abstract protected function authenticatedRateLimit(): ?int; + + /** + * Creates a provider specific rate limit exception. + * + * @param non-empty-string $message + * @param non-empty-string|null $repository + */ + abstract protected function instantiateRateLimitException( + string $message, + ?string $repository, + ?\DateTimeImmutable $resetAt, + ): RateLimitException; + + /** + * Detects a rate limit response. + * + * Rate limiting is reported inconsistently: HTTP 429, or HTTP 403 with an exhausted + * `x-ratelimit-remaining` header, or HTTP 403 with a message about primary/secondary limits. + */ + protected function isRateLimited(int $status, ResponseInterface $response, ?string $apiMessage): bool + { + if ($status === 429) { + return true; + } + + if ($status !== 403) { + return false; + } + + return $response->getHeaderLine('x-ratelimit-remaining') === '0' + || $response->getHeaderLine('ratelimit-remaining') === '0' + || ($apiMessage !== null && \str_contains(\strtolower($apiMessage), 'rate limit')); + } + + /** + * @param non-empty-string|null $repository + */ + protected function createRateLimitException( + ResponseInterface $response, + ?string $apiMessage, + ?string $repository, + ): RateLimitException { + $resetAt = self::resetTime($response); + $isSecondary = $apiMessage !== null && \str_contains(\strtolower($apiMessage), 'secondary rate limit'); + $authenticatedLimit = $this->authenticatedRateLimit(); + $limit = $response->getHeaderLine('x-ratelimit-limit'); + $limit === '' and $limit = (string) ($this->authenticated + ? $authenticatedLimit + : $this->anonymousRateLimit()); + + $message = \sprintf( + '%s API %srate limit exceeded%s.', + $this->providerName(), + $isSecondary ? 'secondary ' : '', + $limit === '' ? '' : \sprintf(' (limit: %s requests per hour)', $limit), + ); + + $resetAt === null or $message .= \sprintf( + ' The limit resets at %s (in %s).', + $resetAt->format('Y-m-d H:i:s T'), + self::humanizeInterval($resetAt), + ); + + $apiMessage === null or $message .= \sprintf(' API message: %s', $apiMessage); + + $message .= "\n" . ($this->authenticated + ? \sprintf( + 'The API token from the %s environment variable has spent its quota: wait for the reset or use another token.', + $this->tokenEnvVariable(), + ) + : \sprintf( + 'No API token is configured. Set the %s environment variable to raise the limit%s.', + $this->tokenEnvVariable(), + $authenticatedLimit === null + ? '' + : \sprintf(' up to %d requests per hour', $authenticatedLimit), + )); + + return $this->instantiateRateLimitException($message, $repository, $resetAt); + } + + /** + * @return non-empty-string + */ + protected function authenticationMessage(?string $apiMessage): string + { + return \sprintf( + "%s API rejected the credentials (HTTP 401%s).\n%s", + $this->providerName(), + $apiMessage === null ? '' : ': ' . $apiMessage, + $this->authenticated + ? \sprintf( + 'The API token from the %s environment variable is invalid, expired or revoked. ' + . 'Provide a valid token or unset the variable to use anonymous access.', + $this->tokenEnvVariable(), + ) + : \sprintf( + 'No API token is configured, so the request was anonymous. ' + . 'Set the %s environment variable with a valid token.', + $this->tokenEnvVariable(), + ), + ); + } + + /** + * @param non-empty-string|null $repository + * @return non-empty-string + */ + protected function accessDeniedMessage(?string $apiMessage, ?string $repository): string + { + return \sprintf( + "%s API denied access%s (HTTP 403%s).\n%s", + $this->providerName(), + $repository === null ? '' : \sprintf(' to `%s`', $repository), + $apiMessage === null ? '' : ': ' . $apiMessage, + $this->authenticated + ? \sprintf( + 'The API token from the %s environment variable has no read access to this %s. ' + . 'Use a token with read permissions for it.', + $this->tokenEnvVariable(), + $this->repositoryTerm(), + ) + : \sprintf( + 'No API token is configured. Set the %s environment variable with a token ' + . 'that has read access to this %s.', + $this->tokenEnvVariable(), + $this->repositoryTerm(), + ), + ); + } + + /** + * @param non-empty-string|null $repository + * @return non-empty-string + */ + protected function notFoundMessage(?string $apiMessage, ?string $repository, string $endpoint): string + { + return \sprintf( + "%s API returned HTTP 404 for %s%s.\n%s", + $this->providerName(), + $repository === null ? $endpoint : \sprintf('%s `%s`', $this->repositoryTerm(), $repository), + $apiMessage === null ? '' : ': ' . $apiMessage, + $this->authenticated + ? \sprintf( + 'Check the %1$s address in the configuration. If the %1$s is private, ' + . 'make sure the token from the %2$s environment variable has read access to it ' + . '(a 404 is also returned instead of 403 when access is missing).', + $this->repositoryTerm(), + $this->tokenEnvVariable(), + ) + : \sprintf( + 'Check the %1$s address in the configuration. If the %1$s is private, ' + . 'set the %2$s environment variable with a token that has read access to it.', + $this->repositoryTerm(), + $this->tokenEnvVariable(), + ), + ); + } + + /** + * Reads a human-readable message from an API error response. + */ + private static function extractApiMessage(ResponseInterface $response): ?string + { + $body = \trim($response->getBody()->__toString()); + if ($body === '') { + return null; + } + + try { + /** @var mixed $decoded */ + $decoded = \json_decode($body, true, 512, JSON_THROW_ON_ERROR); + } catch (\JsonException) { + // Not a JSON response: quote the raw body + return self::truncate($body); + } + + $message = match (true) { + \is_string($decoded) => $decoded, + \is_array($decoded) => self::messageFromArray($decoded), + default => null, + }; + + return $message === null ? null : self::truncate($message); + } + + /** + * @param array $decoded + */ + private static function messageFromArray(array $decoded): ?string + { + // GitHub: {"message": "...", "documentation_url": "..."} + // GitLab: {"message": "404 Project Not Found"} or {"error": "..."} + foreach (['message', 'error', 'error_description'] as $key) { + /** @var mixed $value */ + $value = $decoded[$key] ?? null; + + if (\is_string($value) && $value !== '') { + return $value; + } + + if (!\is_array($value)) { + continue; + } + + // GitLab may report a list of messages + $parts = []; + /** @var mixed $item */ + foreach ($value as $item) { + \is_string($item) and $parts[] = $item; + } + + if ($parts !== []) { + return \implode(' ', $parts); + } + } + + // Older GitHub responses use a plain list: ["API rate limit exceeded...", "https://docs..."] + /** @var mixed $first */ + $first = $decoded[0] ?? null; + + return \is_string($first) && $first !== '' ? $first : null; + } + + /** + * Resolves the moment when a rate limit is reset from response headers. + */ + private static function resetTime(ResponseInterface $response): ?\DateTimeImmutable + { + $reset = $response->getHeaderLine('x-ratelimit-reset'); + if (\preg_match('/^\d+$/', $reset) === 1) { + return (new \DateTimeImmutable('@' . $reset))->setTimezone(new \DateTimeZone(\date_default_timezone_get())); + } + + $retryAfter = $response->getHeaderLine('retry-after'); + if (\preg_match('/^\d+$/', $retryAfter) === 1) { + return new \DateTimeImmutable(\sprintf('+%d seconds', (int) $retryAfter)); + } + + return null; + } + + /** + * @return non-empty-string Time left until the given moment, e.g. `42 min 5 sec`. + */ + private static function humanizeInterval(\DateTimeImmutable $until): string + { + $seconds = $until->getTimestamp() - \time(); + if ($seconds <= 0) { + return 'a moment'; + } + + $minutes = \intdiv($seconds, 60); + return $minutes === 0 + ? \sprintf('%d sec', $seconds) + : \sprintf('%d min %d sec', $minutes, $seconds % 60); + } + + private static function truncate(string $message): string + { + $message = \trim(\preg_replace('/\s+/', ' ', $message) ?? $message); + + if (\strlen($message) <= self::MESSAGE_MAX_LENGTH) { + return $message; + } + + $cut = \substr($message, 0, self::MESSAGE_MAX_LENGTH); + + // A byte-based cut may split a multibyte UTF-8 character: drop its leftover bytes + for ($i = 0; $i < 3 && $cut !== '' && \preg_match('//u', $cut) !== 1; ++$i) { + $cut = \substr($cut, 0, -1); + } + + return $cut . '…'; + } +} diff --git a/src/Module/Task/Manager.php b/src/Module/Task/Manager.php index 2be8c98..a73434b 100644 --- a/src/Module/Task/Manager.php +++ b/src/Module/Task/Manager.php @@ -93,8 +93,8 @@ public function getProcessor(): \Generator yield $task->resume(); } catch (\Throwable $e) { - $this->logger->error($e->getMessage()); - $this->logger->exception($e); + # The failure is delivered via the promise, so the caller decides how to report it + $this->logger->exception($e, important: false); unset($this->tasks[$key]); $deferred->reject($e); yield $e; diff --git a/tests/Unit/Module/Downloader/Exception/NothingExtractedTest.php b/tests/Unit/Module/Downloader/Exception/NothingExtractedTest.php new file mode 100644 index 0000000..448399c --- /dev/null +++ b/tests/Unit/Module/Downloader/Exception/NothingExtractedTest.php @@ -0,0 +1,50 @@ +getMessage(); + + // Assert + self::assertStringContainsString( + 'Nothing was extracted from `roadrunner-2025.1.15-windows-amd64.zip`: ' + . 'none of the 3 file(s) inside matches the extraction rules.', + $message, + ); + self::assertStringContainsString('Extraction rules: binary `/^roadrunner-.*/`', $message); + self::assertStringContainsString('Files in the asset: CHANGELOG.md, LICENSE, rr.exe', $message); + } + + public function testMessageTruncatesLongFileList(): void + { + // Arrange + $files = \array_map(static fn(int $i): string => "file-{$i}.txt", \range(1, 25)); + + // Act + $message = (new NothingExtracted('archive.tar.gz', [], $files))->getMessage(); + + // Assert + self::assertStringContainsString('Extraction rules: none', $message); + self::assertStringContainsString('file-20.txt', $message); + self::assertStringNotContainsString('file-21.txt', $message); + self::assertStringContainsString('and 5 more', $message); + } +} diff --git a/tests/Unit/Module/Downloader/Internal/Diagnostics/DownloadDiagnosticsTest.php b/tests/Unit/Module/Downloader/Internal/Diagnostics/DownloadDiagnosticsTest.php new file mode 100644 index 0000000..174229d --- /dev/null +++ b/tests/Unit/Module/Downloader/Internal/Diagnostics/DownloadDiagnosticsTest.php @@ -0,0 +1,179 @@ +render(); + + // Assert + self::assertStringContainsString( + 'Requested: version `^2.0`, OS `linux`, architecture `amd64`, ' + . 'minimum stability `stable`, asset type `binary`.', + $report, + ); + } + + public function testReportMentionsMissingRepositoryConfiguration(): void + { + // Arrange + $diagnostics = self::diagnostics(); + + // Act + $report = $diagnostics->render(); + + // Assert + self::assertStringContainsString('No repositories are configured for `app`', $report); + } + + public function testReportListsAvailableReleasesWhenNothingMatches(): void + { + // Arrange + $diagnostics = self::diagnostics(version: '^5.0'); + $repository = $diagnostics->addRepository('github', 'owner/repo', '/^app-.*/'); + $repository->matchedReleases = 0; + $repository->registerFetchedReleases(['v1.2.0', 'v1.1.0']); + + // Act + $report = $diagnostics->render(); + + // Assert + self::assertStringContainsString('Tried 1 repository(ies):', $report); + self::assertStringContainsString('1) github `owner/repo`', $report); + self::assertStringContainsString('0 release(s) match the requested version and stability.', $report); + self::assertStringContainsString('Releases available in the repository: v1.2.0, v1.1.0', $report); + } + + public function testReportListsCheckedReleasesWithTheirAssets(): void + { + // Arrange + $diagnostics = self::diagnostics(); + $repository = $diagnostics->addRepository('github', 'owner/repo', '/^app-.*/'); + $repository->matchedReleases = 2; + + $release = $repository->addRelease('v1.2.0'); + $release->registerAssets(['app-1.2.0-darwin-arm64.tar.gz', 'app-1.2.0-windows-amd64.zip']); + $release->reason = 'no asset matches OS `linux`, architecture `amd64`, name pattern `/^app-.*/`'; + + $failed = $repository->addRelease('v1.1.0'); + $failed->registerAssets(['app-1.1.0-linux-amd64.tar.gz']); + $failed->addFailure('app-1.1.0-linux-amd64.tar.gz', new \RuntimeException('Broken archive')); + + // Act + $report = $diagnostics->render(); + + // Assert + self::assertStringContainsString('2 release(s) match the requested version and stability.', $report); + self::assertStringContainsString('Checked releases:', $report); + self::assertStringContainsString('- v1.2.0: 2 asset(s), no asset matches OS `linux`', $report); + self::assertStringContainsString('Assets: app-1.2.0-darwin-arm64.tar.gz, app-1.2.0-windows-amd64.zip', $report); + self::assertStringContainsString( + 'Failed asset `app-1.1.0-linux-amd64.tar.gz`: Broken archive', + $report, + ); + } + + public function testReportContainsRepositoryLevelError(): void + { + // Arrange + $diagnostics = self::diagnostics(); + $repository = $diagnostics->addRepository('github', 'owner/repo', '/^app-.*/'); + $repository->error = new ApiException( + "GitHub API rate limit exceeded.\nSet the GITHUB_TOKEN environment variable.", + 'owner/repo', + ); + + $fallback = $diagnostics->addRepository('gitlab', 'group/app', '/^app-.*/'); + $fallback->matchedReleases = 0; + + // Act + $report = $diagnostics->render(); + + // Assert + self::assertStringContainsString('Tried 2 repository(ies):', $report); + self::assertStringContainsString('GitHub API rate limit exceeded.', $report); + self::assertStringContainsString('Set the GITHUB_TOKEN environment variable.', $report); + self::assertStringContainsString('2) gitlab `group/app`', $report); + } + + public function testReleaseWithoutFetchedAssetListIsNotReportedAsEmpty(): void + { + // Arrange + $diagnostics = self::diagnostics(); + $repository = $diagnostics->addRepository('github', 'owner/repo', '/^app-.*/'); + $repository->matchedReleases = 1; + + // An error interrupted the attempt before the asset list was fetched + $repository->addRelease('v1.2.0')->reason = 'GitHub API is unavailable: HTTP 502 Bad Gateway'; + + // Act + $report = $diagnostics->render(); + + // Assert + self::assertStringContainsString('- v1.2.0: asset list not loaded, GitHub API is unavailable', $report); + self::assertStringNotContainsString('0 asset(s)', $report); + } + + public function testReportLimitsTheNumberOfDescribedReleases(): void + { + // Arrange + $diagnostics = self::diagnostics(); + $repository = $diagnostics->addRepository('github', 'owner/repo', '/^app-.*/'); + $repository->matchedReleases = 8; + + for ($i = 8; $i > 0; --$i) { + $repository->addRelease("v1.0.{$i}")->registerAssets(["app-1.0.{$i}-linux-amd64.tar.gz"]); + } + + // Act + $report = $diagnostics->render(); + + // Assert + self::assertStringContainsString('- v1.0.8:', $report); + self::assertStringContainsString('- v1.0.6:', $report); + self::assertStringNotContainsString('- v1.0.5:', $report); + self::assertStringContainsString('and 5 more release(s)', $report); + } + + /** + * @param non-empty-string|null $version + */ + private static function diagnostics(?string $version = null, ?Type $type = null): DownloadDiagnostics + { + $software = Software::fromArray(['name' => 'App', 'alias' => 'app']); + + $config = DownloadConfig::fromSoftwareId('app'); + $config->version = $version; + $config->type = $type; + + return new DownloadDiagnostics( + software: $software, + actionConfig: $config, + operatingSystem: OperatingSystem::Linux, + architecture: Architecture::X86_64, + stability: Stability::Stable, + ); + } +} diff --git a/tests/Unit/Module/Repository/Internal/GitHub/Api/ClientTest.php b/tests/Unit/Module/Repository/Internal/GitHub/Api/ClientTest.php index aee24cc..5ff50d2 100644 --- a/tests/Unit/Module/Repository/Internal/GitHub/Api/ClientTest.php +++ b/tests/Unit/Module/Repository/Internal/GitHub/Api/ClientTest.php @@ -5,12 +5,16 @@ namespace Internal\DLoad\Tests\Unit\Module\Repository\Internal\GitHub\Api; use Internal\DLoad\Module\Config\Schema\GitHub; +use Internal\DLoad\Module\Repository\Exception\AccessDeniedException; +use Internal\DLoad\Module\Repository\Exception\ApiException; +use Internal\DLoad\Module\Repository\Exception\AuthenticationException; +use Internal\DLoad\Module\Repository\Exception\RateLimitException; +use Internal\DLoad\Module\Repository\Exception\RepositoryNotFoundException; use Internal\DLoad\Module\Repository\Internal\GitHub\Api\Client; -use Internal\DLoad\Module\Repository\Internal\GitHub\Exception\GitHubRateLimitException; use Internal\DLoad\Tests\Unit\Module\Repository\Internal\GitHub\Stub\ClientStub; use Internal\DLoad\Tests\Unit\Module\Repository\Internal\GitHub\Stub\GitHubConfigStub; use Internal\DLoad\Tests\Unit\Module\Repository\Internal\GitHub\Stub\HttpFactoryStub; -use Internal\DLoad\Tests\Unit\Module\Repository\Internal\GitHub\Stub\ResponseStub; +use Internal\DLoad\Tests\Unit\Module\Repository\Stub\ResponseStub; use PHPUnit\Framework\Attributes\CoversClass; use PHPUnit\Framework\Attributes\DataProvider; use PHPUnit\Framework\TestCase; @@ -33,51 +37,96 @@ public static function provideRequestHeaders(): \Generator yield 'override default headers' => [['accept' => 'application/json']]; } - public static function provideRateLimitScenarios(): \Generator + /** + * @return \Generator, class-string<\Throwable>|null}> + */ + public static function provideErrorScenarios(): \Generator { - yield 'valid rate limit response' => [ + yield 'legacy rate limit response' => [ 403, \json_encode([ - 'API rate limit exceeded for user ID 1234. Check the hourly limit for your plan at https://docs.github.com/rest/overview/resources-in-the-rest-api#rate-limiting', + 'API rate limit exceeded for user ID 1234.', 'https://docs.github.com/rest/overview/resources-in-the-rest-api#rate-limiting', ]), - true, + [], + RateLimitException::class, ]; - yield 'non-403 status code' => [ + yield 'rate limit reported with 429' => [ 429, - \json_encode(['API rate limit exceeded', 'https://docs.github.com']), - false, + \json_encode(['message' => 'API rate limit exceeded', 'documentation_url' => 'https://docs.github.com']), + [], + RateLimitException::class, ]; - yield '403 with invalid JSON' => [ + yield 'rate limit detected by header' => [ 403, - 'invalid json response', - false, + \json_encode(['message' => 'Request forbidden', 'documentation_url' => 'https://docs.github.com']), + ['x-ratelimit-remaining' => ['0']], + RateLimitException::class, ]; - yield '403 with wrong array structure' => [ + yield 'secondary rate limit' => [ 403, - \json_encode(['message' => 'Forbidden']), - false, + \json_encode(['message' => 'You have exceeded a secondary rate limit.']), + [], + RateLimitException::class, ]; - yield '403 with wrong array count' => [ - 403, - \json_encode(['API rate limit exceeded']), - false, + yield 'invalid token' => [ + 401, + \json_encode(['message' => 'Bad credentials']), + [], + AuthenticationException::class, ]; - yield '403 with non-string elements' => [ + yield 'forbidden without rate limit' => [ 403, - \json_encode([123, 456]), - false, + \json_encode(['message' => 'Resource not accessible by integration']), + [], + AccessDeniedException::class, ]; - yield '403 without rate limit text' => [ + yield 'non-JSON forbidden body' => [ 403, - \json_encode(['Something else', 'https://docs.github.com']), - false, + 'invalid json response', + [], + AccessDeniedException::class, + ]; + + yield 'missing repository' => [ + 404, + \json_encode(['message' => 'Not Found']), + [], + RepositoryNotFoundException::class, + ]; + + yield 'server error' => [ + 502, + 'Bad Gateway', + [], + ApiException::class, + ]; + + yield 'unprocessable entity' => [ + 422, + \json_encode(['message' => 'Validation Failed']), + [], + ApiException::class, + ]; + + yield 'successful response' => [ + 200, + '[]', + [], + null, + ]; + + yield 'redirect is not an error' => [ + 302, + '', + [], + null, ]; } @@ -151,68 +200,49 @@ public function testDetectsRateLimitResponseAndThrowsException(): void $this->client = new Client($this->httpFactory, $this->httpClient, $this->gitHubConfig); // Assert (before Act for exceptions) - $this->expectException(GitHubRateLimitException::class); - $this->expectExceptionMessage('API rate limit exceeded for user ID 1234'); + $this->expectException(RateLimitException::class); + $this->expectExceptionMessage('rate limit exceeded'); // Act $this->client->request($method, $uri); } - public function testDoesNotThrowExceptionForNon403Response(): void + public function testRateLimitMessageSuggestsTokenWhenThereIsNoToken(): void { // Arrange - $method = 'GET'; - $uri = $this->createMock(UriInterface::class); $request = $this->createMock(RequestInterface::class); - $response = ResponseStub::ok(); - - $this->httpFactory = $this->httpFactory->withRequest($method, $uri, $request); - $this->httpClient = $this->httpClient->withResponse($request, $response); - $this->client = new Client($this->httpFactory, $this->httpClient, $this->gitHubConfig); - - // Act - $result = $this->client->request($method, $uri); - - // Assert - self::assertSame($response, $result); - } - - public function testDoesNotThrowExceptionForNonRateLimitError(): void - { - // Arrange - $method = 'GET'; - $uri = $this->createMock(UriInterface::class); - $request = $this->createMock(RequestInterface::class); - $forbiddenResponse = ResponseStub::githubForbidden(); - - $this->httpFactory = $this->httpFactory->withRequest($method, $uri, $request); - $this->httpClient = $this->httpClient->withResponse($request, $forbiddenResponse); + $this->httpClient = $this->httpClient->withResponse($request, ResponseStub::githubRateLimit()); $this->client = new Client($this->httpFactory, $this->httpClient, $this->gitHubConfig); // Act - $result = $this->client->request($method, $uri); - - // Assert - self::assertSame($forbiddenResponse, $result); + try { + $this->client->sendRequest($request); + self::fail('RateLimitException is expected.'); + } catch (RateLimitException $e) { + // Assert + self::assertStringContainsString('GITHUB_TOKEN', $e->getMessage()); + self::assertStringContainsString('No API token is configured', $e->getMessage()); + } } - public function testDoesNotThrowExceptionForInvalidJsonResponse(): void + public function testAuthenticationMessageMentionsConfiguredToken(): void { // Arrange - $method = 'GET'; - $uri = $this->createMock(UriInterface::class); $request = $this->createMock(RequestInterface::class); - $invalidJsonResponse = ResponseStub::invalidJson(); + $response = new ResponseStub(401, [], \json_encode(['message' => 'Bad credentials'])); - $this->httpFactory = $this->httpFactory->withRequest($method, $uri, $request); - $this->httpClient = $this->httpClient->withResponse($request, $invalidJsonResponse); - $this->client = new Client($this->httpFactory, $this->httpClient, $this->gitHubConfig); + $this->httpClient = $this->httpClient->withResponse($request, $response); + $client = new Client($this->httpFactory, $this->httpClient, GitHubConfigStub::withToken('invalid-token')); // Act - $result = $this->client->request($method, $uri); - - // Assert - self::assertSame($invalidJsonResponse, $result); + try { + $client->sendRequest($request); + self::fail('AuthenticationException is expected.'); + } catch (AuthenticationException $e) { + // Assert + self::assertStringContainsString('Bad credentials', $e->getMessage()); + self::assertStringContainsString('invalid, expired or revoked', $e->getMessage()); + } } public function testSendRequestDelegatesToHttpClient(): void @@ -231,23 +261,7 @@ public function testSendRequestDelegatesToHttpClient(): void self::assertSame($response, $result); } - public function testSendRequestThrowsRateLimitExceptionOn403WithRateLimitJson(): void - { - // Arrange - $request = $this->createMock(RequestInterface::class); - $rateLimitResponse = ResponseStub::githubRateLimit(); - - $this->httpClient = $this->httpClient->withResponse($request, $rateLimitResponse); - $this->client = new Client($this->httpFactory, $this->httpClient, $this->gitHubConfig); - - // Assert (before Act for exceptions) - $this->expectException(GitHubRateLimitException::class); - - // Act - $this->client->sendRequest($request); - } - - public function testSendRequestPropagatesClientExceptions(): void + public function testSendRequestWrapsClientExceptionsIntoApiException(): void { // Arrange $request = $this->createMock(RequestInterface::class); @@ -256,11 +270,15 @@ public function testSendRequestPropagatesClientExceptions(): void $this->httpClient = $this->httpClient->withException($request, $clientException); $this->client = new Client($this->httpFactory, $this->httpClient, $this->gitHubConfig); - // Assert (before Act for exceptions) - $this->expectException(ClientExceptionInterface::class); - // Act - $this->client->sendRequest($request); + try { + $this->client->sendRequest($request); + self::fail('ApiException is expected.'); + } catch (ApiException $e) { + // Assert + self::assertStringContainsString('Failed to reach GitHub API', $e->getMessage()); + self::assertSame($clientException, $e->getPrevious()); + } } #[DataProvider('provideRequestHeaders')] @@ -283,29 +301,31 @@ public function testRequestMergesHeadersCorrectly(array $additionalHeaders): voi self::assertSame($response, $result); } - #[DataProvider('provideRateLimitScenarios')] - public function testRateLimitDetectionScenarios( + /** + * @param array $headers + * @param class-string<\Throwable>|null $expectedException + */ + #[DataProvider('provideErrorScenarios')] + public function testUnsuccessfulResponsesAreConvertedIntoExceptions( int $statusCode, string $responseBody, - bool $shouldThrowException, + array $headers, + ?string $expectedException, ): void { // Arrange $request = $this->createMock(RequestInterface::class); - $response = new ResponseStub($statusCode, [], $responseBody); + $response = new ResponseStub($statusCode, $headers, $responseBody); $this->httpClient = $this->httpClient->withResponse($request, $response); $this->client = new Client($this->httpFactory, $this->httpClient, $this->gitHubConfig); - if ($shouldThrowException) { - $this->expectException(GitHubRateLimitException::class); - } + $expectedException === null or $this->expectException($expectedException); - // Act & Assert + // Act $result = $this->client->sendRequest($request); - if (!$shouldThrowException) { - self::assertSame($response, $result); - } + // Assert + self::assertSame($response, $result); } protected function setUp(): void diff --git a/tests/Unit/Module/Repository/Internal/GitHub/Api/ResponseValidatorTest.php b/tests/Unit/Module/Repository/Internal/GitHub/Api/ResponseValidatorTest.php new file mode 100644 index 0000000..d095510 --- /dev/null +++ b/tests/Unit/Module/Repository/Internal/GitHub/Api/ResponseValidatorTest.php @@ -0,0 +1,200 @@ +validate(self::releasesRequest(), ResponseStub::ok('[]')); + + // Assert + self::assertTrue(true, 'Successful responses must not throw.'); + } + + public function testRateLimitWithoutTokenExplainsAnonymousLimit(): void + { + // Arrange + $validator = new ResponseValidator(authenticated: false); + $response = new ResponseStub( + 403, + ['x-ratelimit-remaining' => ['0'], 'x-ratelimit-limit' => ['60']], + \json_encode(['message' => 'API rate limit exceeded for 1.2.3.4.']), + ); + + // Assert (before Act for exceptions) + $this->expectException(RateLimitException::class); + $this->expectExceptionMessage('60 requests per hour'); + + // Act + $validator->validate(self::releasesRequest(), $response); + } + + public function testRateLimitWithTokenReportsSpentQuotaAndResetTime(): void + { + // Arrange + $validator = new ResponseValidator(authenticated: true); + $resetsAt = \time() + 600; + $response = new ResponseStub( + 429, + ['x-ratelimit-remaining' => ['0'], 'x-ratelimit-reset' => [(string) $resetsAt]], + \json_encode(['message' => 'API rate limit exceeded']), + ); + + // Act + try { + $validator->validate(self::releasesRequest(), $response); + self::fail('RateLimitException is expected.'); + } catch (RateLimitException $e) { + // Assert + self::assertStringContainsString('spent its quota', $e->getMessage()); + self::assertStringContainsString('GITHUB_TOKEN', $e->getMessage()); + self::assertNotNull($e->resetAt); + self::assertSame($resetsAt, $e->resetAt->getTimestamp()); + } + } + + public function testSecondaryRateLimitIsRecognized(): void + { + // Arrange + $validator = new ResponseValidator(authenticated: true); + $response = new ResponseStub( + 403, + ['retry-after' => ['60']], + \json_encode(['message' => 'You have exceeded a secondary rate limit. Please wait a few minutes.']), + ); + + // Assert (before Act for exceptions) + $this->expectException(RateLimitException::class); + $this->expectExceptionMessage('secondary rate limit exceeded'); + + // Act + $validator->validate(self::releasesRequest(), $response); + } + + public function testForbiddenResponseMentionsRepositoryAndToken(): void + { + // Arrange + $validator = new ResponseValidator(authenticated: true); + $response = new ResponseStub(403, [], \json_encode(['message' => 'Resource not accessible by integration'])); + + // Act + try { + $validator->validate(self::releasesRequest(), $response); + self::fail('AccessDeniedException is expected.'); + } catch (AccessDeniedException $e) { + // Assert + self::assertSame('owner/repo', $e->repository); + self::assertStringContainsString('Resource not accessible by integration', $e->getMessage()); + self::assertStringContainsString('no read access to this repository', $e->getMessage()); + } + } + + public function testNotFoundResponseSuggestsCheckingRepositoryAddress(): void + { + // Arrange + $validator = new ResponseValidator(authenticated: false); + $response = new ResponseStub(404, [], \json_encode(['message' => 'Not Found'])); + + // Act + try { + $validator->validate(self::releasesRequest(), $response); + self::fail('RepositoryNotFoundException is expected.'); + } catch (RepositoryNotFoundException $e) { + // Assert + self::assertStringContainsString('repository `owner/repo`', $e->getMessage()); + self::assertStringContainsString('GITHUB_TOKEN', $e->getMessage()); + } + } + + public function testServerErrorIsReportedAsTemporaryFailure(): void + { + // Arrange + $validator = new ResponseValidator(authenticated: false); + $response = new ResponseStub(503, [], 'Service Unavailable', 'Service Unavailable'); + + // Assert (before Act for exceptions) + $this->expectException(ApiException::class); + $this->expectExceptionMessage('GitHub API is unavailable: HTTP 503'); + + // Act + $validator->validate(self::releasesRequest(), $response); + } + + public function testRepositoryIsResolvedFromAssetDownloadUrl(): void + { + // Arrange + $validator = new ResponseValidator(authenticated: false); + $request = new Request('GET', 'https://github.com/owner/repo/releases/download/v1.0.0/asset.zip'); + $response = new ResponseStub(404, [], \json_encode(['message' => 'Not Found'])); + + // Act + try { + $validator->validate($request, $response); + self::fail('RepositoryNotFoundException is expected.'); + } catch (RepositoryNotFoundException $e) { + // Assert + self::assertSame('owner/repo', $e->repository); + } + } + + public function testTransportFailureKeepsTheOriginalError(): void + { + // Arrange + $validator = new ResponseValidator(authenticated: false); + $original = new \RuntimeException('Could not resolve host: api.github.com'); + + // Act + $exception = $validator->transportFailure(self::releasesRequest(), $original); + + // Assert + self::assertStringContainsString('Failed to reach GitHub API', $exception->getMessage()); + self::assertStringContainsString('Could not resolve host', $exception->getMessage()); + self::assertSame($original, $exception->getPrevious()); + self::assertSame('owner/repo', $exception->repository); + } + + public function testLongApiMessageIsTruncatedWithoutBreakingUtf8(): void + { + // Arrange + $validator = new ResponseValidator(authenticated: false); + // An ASCII prefix shifts the byte-based cut into the middle of a multibyte character + $apiMessage = 'x' . \str_repeat('я', 400); + $response = new ResponseStub(422, [], \json_encode(['message' => $apiMessage])); + + // Act + try { + $validator->validate(self::releasesRequest(), $response); + self::fail('ApiException is expected.'); + } catch (ApiException $e) { + // Assert + $message = $e->getMessage(); + self::assertSame(1, \preg_match('//u', $message), 'The message must stay valid UTF-8.'); + self::assertStringContainsString('…', $message); + } + } + + private static function releasesRequest(): RequestInterface + { + return new Request('GET', 'https://api.github.com/repos/owner/repo/releases?page=1'); + } +} diff --git a/tests/Unit/Module/Repository/Internal/GitHub/Stub/ClientStub.php b/tests/Unit/Module/Repository/Internal/GitHub/Stub/ClientStub.php index 4d6aceb..0d211fb 100644 --- a/tests/Unit/Module/Repository/Internal/GitHub/Stub/ClientStub.php +++ b/tests/Unit/Module/Repository/Internal/GitHub/Stub/ClientStub.php @@ -4,6 +4,7 @@ namespace Internal\DLoad\Tests\Unit\Module\Repository\Internal\GitHub\Stub; +use Internal\DLoad\Tests\Unit\Module\Repository\Stub\ResponseStub; use Psr\Http\Client\ClientExceptionInterface; use Psr\Http\Client\ClientInterface; use Psr\Http\Message\RequestInterface; diff --git a/tests/Unit/Module/Repository/Internal/GitLab/Api/ResponseValidatorTest.php b/tests/Unit/Module/Repository/Internal/GitLab/Api/ResponseValidatorTest.php new file mode 100644 index 0000000..2be6501 --- /dev/null +++ b/tests/Unit/Module/Repository/Internal/GitLab/Api/ResponseValidatorTest.php @@ -0,0 +1,55 @@ + '404 Project Not Found'])); + + // Act + try { + $validator->validate($request, $response); + self::fail('RepositoryNotFoundException is expected.'); + } catch (RepositoryNotFoundException $e) { + // Assert + self::assertSame('group/project', $e->repository); + self::assertStringContainsString('project `group/project`', $e->getMessage()); + self::assertStringContainsString('GITLAB_TOKEN', $e->getMessage()); + } + } + + public function testTooManyRequestsIsReportedAsRateLimit(): void + { + // Arrange + $validator = new ResponseValidator(authenticated: false); + $request = new Request('GET', 'https://gitlab.com/api/v4/projects/group%2Fproject/releases'); + $response = new ResponseStub(429, ['retry-after' => ['30']], ''); + + // Act + try { + $validator->validate($request, $response); + self::fail('RateLimitException is expected.'); + } catch (RateLimitException $e) { + // Assert + self::assertStringContainsString('GitLab API rate limit exceeded', $e->getMessage()); + self::assertStringContainsString('GITLAB_TOKEN', $e->getMessage()); + self::assertNotNull($e->resetAt); + } + } +} diff --git a/tests/Unit/Module/Repository/Internal/GitHub/Stub/ResponseStub.php b/tests/Unit/Module/Repository/Stub/ResponseStub.php similarity index 97% rename from tests/Unit/Module/Repository/Internal/GitHub/Stub/ResponseStub.php rename to tests/Unit/Module/Repository/Stub/ResponseStub.php index 78b88c7..3da8e71 100644 --- a/tests/Unit/Module/Repository/Internal/GitHub/Stub/ResponseStub.php +++ b/tests/Unit/Module/Repository/Stub/ResponseStub.php @@ -2,13 +2,13 @@ declare(strict_types=1); -namespace Internal\DLoad\Tests\Unit\Module\Repository\Internal\GitHub\Stub; +namespace Internal\DLoad\Tests\Unit\Module\Repository\Stub; use Psr\Http\Message\ResponseInterface; use Psr\Http\Message\StreamInterface; /** - * HTTP Response stub for GitHub API tests. + * HTTP Response stub for repository API tests. * * Provides controllable response data for testing. */