From b845874ae90843875cc1da6c0909138c9023d315 Mon Sep 17 00:00:00 2001 From: Gerrod Ubben Date: Thu, 29 Jan 2026 18:08:42 -0500 Subject: [PATCH] Add more useful PulpExceptions for general tasks fixes: #7270 Assisted-by: claude-4.5-sonnet --- CHANGES/plugin_api/7270.feature | 1 + pulp_file/app/tasks/synchronizing.py | 16 +- pulp_file/pytest_plugin.py | 8 + pulp_file/tests/functional/api/test_sync.py | 33 +++- pulpcore/app/models/content.py | 16 +- pulpcore/app/models/repository.py | 11 +- pulpcore/app/tasks/base.py | 21 ++- pulpcore/app/tasks/export.py | 19 +- pulpcore/app/tasks/importer.py | 21 ++- pulpcore/app/tasks/migrate.py | 2 +- pulpcore/app/tasks/purge.py | 3 +- pulpcore/app/tasks/replica.py | 3 +- pulpcore/app/tasks/upload.py | 7 +- pulpcore/app/tasks/vulnerability_report.py | 3 +- pulpcore/download/http.py | 8 +- pulpcore/exceptions/__init__.py | 8 + pulpcore/exceptions/base.py | 185 +++++++++++++++++--- pulpcore/exceptions/plugin.py | 3 +- pulpcore/exceptions/validation.py | 54 ++++-- pulpcore/plugin/exceptions.py | 12 ++ pulpcore/plugin/stages/artifact_stages.py | 13 +- pulpcore/plugin/stages/content_stages.py | 7 +- pulpcore/tests/unit/test_chunked_file.py | 6 +- pulpcore/tests/unit/test_import_checks.py | 11 +- pyproject.toml | 4 +- 25 files changed, 369 insertions(+), 106 deletions(-) create mode 100644 CHANGES/plugin_api/7270.feature diff --git a/CHANGES/plugin_api/7270.feature b/CHANGES/plugin_api/7270.feature new file mode 100644 index 00000000000..c9b7fbbac8a --- /dev/null +++ b/CHANGES/plugin_api/7270.feature @@ -0,0 +1 @@ +Added more useful error exceptions for general tasks. diff --git a/pulp_file/app/tasks/synchronizing.py b/pulp_file/app/tasks/synchronizing.py index 2f237da5133..6d739dd40b6 100644 --- a/pulp_file/app/tasks/synchronizing.py +++ b/pulp_file/app/tasks/synchronizing.py @@ -4,8 +4,10 @@ from gettext import gettext as _ from urllib.parse import quote, urlparse, urlunparse +import aiohttp.client_exceptions from django.core.files import File +from pulpcore.plugin.exceptions import SyncError from pulpcore.plugin.models import Artifact, ProgressReport, Remote, PublishedMetadata from pulpcore.plugin.serializers import RepositoryVersionSerializer from pulpcore.plugin.stages import ( @@ -38,14 +40,14 @@ def synchronize(remote_pk, repository_pk, mirror, url=None): url (str): The url to synchronize. If omitted, the url of the remote is used. Raises: - ValueError: If the remote does not specify a URL to sync. + SyncError: If the remote does not specify a URL to sync. """ remote = FileRemote.objects.get(pk=remote_pk) repository = FileRepository.objects.get(pk=repository_pk) if not remote.url: - raise ValueError(_("A remote must have a url specified to synchronize.")) + raise SyncError(_("A remote must have a url specified to synchronize.")) first_stage = FileFirstStage(remote, url) dv = DeclarativeVersion(first_stage, repository, mirror=mirror, acs=True) @@ -103,7 +105,10 @@ async def run(self): parsed_url = urlparse(self.url) root_dir = os.path.dirname(parsed_url.path) downloader = self.remote.get_downloader(url=self.url) - result = await downloader.run() + try: + result = await downloader.run() + except aiohttp.client_exceptions.ClientResponseError as e: + raise SyncError(_("Error downloading manifest file: {error}").format(error=e)) await pb.aincrement() metadata_files.append((result.path, self.url.split("/")[-1])) @@ -111,7 +116,10 @@ async def run(self): message="Parsing Metadata Lines", code="sync.parsing.metadata" ) as pb: manifest = Manifest(result.path) - entries = list(manifest.read()) + try: + entries = list(manifest.read()) + except ValueError as e: + raise SyncError(_("Error parsing manifest file: {error}").format(error=e)) pb.total = len(entries) await pb.asave() diff --git a/pulp_file/pytest_plugin.py b/pulp_file/pytest_plugin.py index c1a559c9030..bf451a64c66 100644 --- a/pulp_file/pytest_plugin.py +++ b/pulp_file/pytest_plugin.py @@ -171,6 +171,14 @@ def duplicate_filename_paths(write_3_iso_file_fixture_data_factory): ) +@pytest.fixture(scope="class") +def missing_file_path(file_fixtures_root): + file_fixtures_root.joinpath("missing_file").mkdir() + file = {"name": "missing_file/1.iso", "digest": "1234567890", "size": 100} + generate_manifest(file_fixtures_root.joinpath("missing_file/PULP_MANIFEST"), [file]) + return "/missing_file/PULP_MANIFEST" + + @pytest.fixture(scope="class") def file_fixture_server_ssl_client_cert_req( ssl_ctx_req_client_auth, file_fixtures_root, gen_fixture_server diff --git a/pulp_file/tests/functional/api/test_sync.py b/pulp_file/tests/functional/api/test_sync.py index 6bba4f4c934..d1103760e9d 100644 --- a/pulp_file/tests/functional/api/test_sync.py +++ b/pulp_file/tests/functional/api/test_sync.py @@ -79,8 +79,9 @@ def test_invalid_url(file_bindings, file_repo, gen_object_with_cleanup, monitor_ remote = gen_object_with_cleanup(file_bindings.RemotesFileApi, remote_kwargs) body = RepositorySyncURL(remote=remote.pulp_href) - with pytest.raises(PulpTaskError): + with pytest.raises(PulpTaskError) as e: monitor_task(file_bindings.RepositoriesFileApi.sync(file_repo.pulp_href, body).task) + assert "[PLP0008] URL lookup failed." in e.value.task.error["description"] @pytest.mark.parallel @@ -90,8 +91,36 @@ def test_invalid_file( """Sync a repository using an invalid file repository.""" remote = file_remote_factory(manifest_path=invalid_manifest_path, policy="immediate") body = RepositorySyncURL(remote=remote.pulp_href) - with pytest.raises(PulpTaskError): + with pytest.raises(PulpTaskError) as e: monitor_task(file_bindings.RepositoriesFileApi.sync(file_repo.pulp_href, body).task) + assert "[PLP0003]" in e.value.task.error["description"] + + +@pytest.mark.parallel +def test_missing_file( + file_repo, file_bindings, missing_file_path, file_remote_factory, monitor_task +): + """Sync a repository using a manifest file with a missing file.""" + remote = file_remote_factory(manifest_path=missing_file_path, policy="immediate") + body = RepositorySyncURL(remote=remote.pulp_href) + with pytest.raises(PulpTaskError) as e: + monitor_task(file_bindings.RepositoriesFileApi.sync(file_repo.pulp_href, body).task) + assert "[PLP0013] Sync failed: Error downloading artifact" in e.value.task.error["description"] + + +@pytest.mark.parallel +def test_missing_manifest( + file_repo, file_bindings, basic_manifest_path, file_remote_factory, monitor_task +): + """Sync a repository using a missing manifest file.""" + remote = file_remote_factory(manifest_path="/MISSING_MANIFEST", policy="immediate") + body = RepositorySyncURL(remote=remote.pulp_href) + with pytest.raises(PulpTaskError) as e: + monitor_task(file_bindings.RepositoriesFileApi.sync(file_repo.pulp_href, body).task) + assert ( + "[PLP0013] Sync failed: Error downloading manifest file" + in e.value.task.error["description"] + ) @pytest.mark.parallel diff --git a/pulpcore/app/models/content.py b/pulpcore/app/models/content.py index 4a6cf2bc36c..58628c229a7 100644 --- a/pulpcore/app/models/content.py +++ b/pulpcore/app/models/content.py @@ -7,6 +7,7 @@ import asyncio import datetime import json +import logging import os import tempfile import shutil @@ -34,8 +35,11 @@ SizeValidationError, MissingDigestValidationError, UnsupportedDigestValidationError, + ExternalServiceError, ) +logger = logging.getLogger(__name__) + # All available digest fields ordered by algorithm strength. _DIGEST_FIELDS = [] for alg in ("sha512", "sha384", "sha256", "sha224", "sha1", "md5"): @@ -821,7 +825,7 @@ def sign(self, filename, env_vars=None): env_vars (dict): dictionary of environment variables Raises: - RuntimeError: If the return code of the script is not equal to 0. + ExternalServiceError: If the return code of the script is not equal to 0. Returns: A dictionary as validated by the validate() method. @@ -834,12 +838,13 @@ def sign(self, filename, env_vars=None): ) if completed_process.returncode != 0: - raise RuntimeError(str(completed_process.stderr)) + logger.info("Signing service script failed: {}".format(str(completed_process.stderr))) + raise ExternalServiceError("Signing service script") try: return_value = json.loads(completed_process.stdout) except json.JSONDecodeError: - raise RuntimeError("The signing service script did not return valid JSON!") + raise ExternalServiceError("Signing service script", "did not return valid JSON") return return_value @@ -855,12 +860,13 @@ async def asign(self, filename, env_vars=None): stdout, stderr = await process.communicate() if process.returncode != 0: - raise RuntimeError(str(stderr)) + logger.info("Signing service script failed: {}".format(str(stderr))) + raise ExternalServiceError("Signing service script") try: return_value = json.loads(stdout) except json.JSONDecodeError: - raise RuntimeError("The signing service script did not return valid JSON!") + raise ExternalServiceError("Signing service script", "did not return valid JSON") return return_value diff --git a/pulpcore/app/models/repository.py b/pulpcore/app/models/repository.py index b70cdcd4dba..3a968cf0df3 100644 --- a/pulpcore/app/models/repository.py +++ b/pulpcore/app/models/repository.py @@ -16,7 +16,6 @@ from django.db import models, transaction from django.db.models import F, Func, Q, Value from django_lifecycle import AFTER_UPDATE, BEFORE_CREATE, BEFORE_DELETE, hook -from rest_framework.exceptions import APIException from pulpcore.app.util import ( batch_qs, @@ -28,7 +27,7 @@ ) from pulpcore.constants import ALL_KNOWN_CONTENT_CHECKSUMS, PROTECTED_REPO_VERSION_MESSAGE from pulpcore.download.factory import DownloaderFactory -from pulpcore.exceptions import ResourceImmutableError +from pulpcore.exceptions import ResourceImmutableError, RepositoryVersionDeleteError from pulpcore.cache import Cache @@ -1249,7 +1248,7 @@ def _squash(self, repo_relations, next_version): def check_protected(self): """Check if a repo version is protected before trying to delete it.""" if self in self.repository.protected_versions(): - raise Exception(PROTECTED_REPO_VERSION_MESSAGE) + raise RepositoryVersionDeleteError(PROTECTED_REPO_VERSION_MESSAGE) def delete(self, **kwargs): """ @@ -1263,7 +1262,7 @@ def delete(self, **kwargs): """ if self.complete: if self.repository.versions.complete().count() <= 1: - raise APIException(_("Attempt to delete the last remaining version.")) + raise RepositoryVersionDeleteError() if settings.CACHE_ENABLED: base_paths = self.distribution_set.values_list("base_path", flat=True) if base_paths: @@ -1335,9 +1334,7 @@ def __enter__(self): RepositoryVersion: self """ if self.complete: - raise RuntimeError( - _("This Repository version is complete. It cannot be modified further.") - ) + raise ResourceImmutableError(self) repository = self.repository.cast() repository.initialize_new_version(self) return self diff --git a/pulpcore/app/tasks/base.py b/pulpcore/app/tasks/base.py index fda739e4603..f9f528650e1 100644 --- a/pulpcore/app/tasks/base.py +++ b/pulpcore/app/tasks/base.py @@ -1,9 +1,11 @@ from django.db import transaction +from rest_framework.exceptions import ValidationError as DRFValidationError from pulpcore.app.apps import get_plugin_config from pulpcore.app.models import CreatedResource from pulpcore.app.loggers import deprecation_logger from pulpcore.plugin.models import MasterModel +from pulpcore.exceptions import ValidationError from asgiref.sync import sync_to_async @@ -28,7 +30,7 @@ def general_create(app_label, serializer_name, *args, **kwargs): Create a model instance. Raises: - ValidationError: If the serializer is not valid + SerializerValidationError: If the serializer is not valid """ data = kwargs.pop("data", None) @@ -38,7 +40,10 @@ def general_create(app_label, serializer_name, *args, **kwargs): serializer_class = get_plugin_config(app_label).named_serializers[serializer_name] serializer = serializer_class(data=data, context=context) - serializer.is_valid(raise_exception=True) + try: + serializer.is_valid(raise_exception=True) + except DRFValidationError as e: + raise ValidationError(e.detail) instance = serializer.save() if isinstance(instance, MasterModel): instance = instance.cast() @@ -65,7 +70,7 @@ def general_update(instance_id, app_label, serializer_name, *args, **kwargs): their values are updated as such. Raises: - [rest_framework.exceptions.ValidationError][]: When serializer instance can't be saved + SerializerValidationError: When serializer instance can't be saved due to validation error. This theoretically should never occur since validation is performed before the task is dispatched. """ @@ -80,7 +85,10 @@ def general_update(instance_id, app_label, serializer_name, *args, **kwargs): if isinstance(instance, MasterModel): instance = instance.cast() serializer = serializer_class(instance, data=data, partial=partial) - serializer.is_valid(raise_exception=True) + try: + serializer.is_valid(raise_exception=True) + except DRFValidationError as e: + raise ValidationError(e.detail) serializer.save() @@ -141,7 +149,10 @@ async def ageneral_update(instance_id, app_label, serializer_name, *args, **kwar if isinstance(instance, MasterModel): instance = await instance.acast() serializer = serializer_class(instance, data=data, partial=partial, context=context) - await sync_to_async(serializer.is_valid)(raise_exception=True) + try: + await sync_to_async(serializer.is_valid)(raise_exception=True) + except DRFValidationError as e: + raise ValidationError(e.detail) await sync_to_async(serializer.save)() return await sync_to_async(lambda: serializer.data)() diff --git a/pulpcore/app/tasks/export.py b/pulpcore/app/tasks/export.py index 4bbca2ca092..666670e138c 100644 --- a/pulpcore/app/tasks/export.py +++ b/pulpcore/app/tasks/export.py @@ -34,6 +34,8 @@ export_content, ) from pulpcore.constants import FS_EXPORT_METHODS +from pulpcore.exceptions import ExportError +from rest_framework.exceptions import ValidationError as DRFValidationError log = logging.getLogger(__name__) @@ -66,14 +68,14 @@ def _export_to_file_system(path, relative_paths_to_artifacts, method=FS_EXPORT_M relative_paths_to_artifacts: A dict with {relative_path: artifact} mapping Raises: - ValidationError: When path is not in the ALLOWED_EXPORT_PATHS setting + ExportError: When path is not in the ALLOWED_EXPORT_PATHS setting """ using_filesystem_storage = ( settings.STORAGES["default"]["BACKEND"] == "pulpcore.app.models.storage.FileSystem" ) if method != FS_EXPORT_METHODS.WRITE and not using_filesystem_storage: - raise RuntimeError(_("Only write is supported for non-filesystem storage.")) + raise ExportError(_("Only write is supported for non-filesystem storage.")) os.makedirs(path) export_not_on_same_filesystem = ( @@ -101,7 +103,7 @@ def _export_to_file_system(path, relative_paths_to_artifacts, method=FS_EXPORT_M for chunk in af.chunks(1024 * 1024): f.write(chunk) else: - raise RuntimeError(_("Unsupported export method '{}'.").format(method)) + raise ExportError(_("Unsupported export method '{}'.").format(method)) def _export_publication_to_file_system( @@ -218,7 +220,7 @@ def fs_publication_export(exporter_pk, publication_pk, start_repo_version_pk=Non ) if not _export_location_is_clean(exporter.path): - raise RuntimeError(_("Cannot export to directories that contain existing data.")) + raise ExportError(_("Cannot export to directories that contain existing data.")) _export_publication_to_file_system( exporter.path, publication, start_repo_version=start_repo_version, method=exporter.method @@ -369,12 +371,15 @@ def pulp_export(exporter_pk, params): params (dict): request data Raises: - ValidationError: When path is not in the ALLOWED_EXPORT_PATHS setting, + ExportError: When path is not in the ALLOWED_EXPORT_PATHS setting, OR path exists and is not a directory """ pulp_exporter = PulpExporter.objects.get(pk=exporter_pk) serializer = PulpExportSerializer(data=params, context={"exporter": pulp_exporter}) - serializer.is_valid(raise_exception=True) + try: + serializer.is_valid(raise_exception=True) + except DRFValidationError as e: + raise ExportError(e.detail) the_export = PulpExport.objects.create(exporter=pulp_exporter, params=params) the_export.validated_versions = serializer.validated_data.get("versions", None) the_export.validated_start_versions = serializer.validated_data.get("start_versions", None) @@ -470,7 +475,7 @@ def _do_export(pulp_exporter, tar, the_export): # an on_demand repo content_artifacts = ContentArtifact.objects.filter(content__in=version.content) if content_artifacts.filter(artifact=None).exists(): - raise RuntimeError(_("Remote artifacts cannot be exported.")) + raise ExportError(_("Remote artifacts cannot be exported.")) if do_incremental: vers_artifacts = version.artifacts.difference(vers_match[version].artifacts) diff --git a/pulpcore/app/tasks/importer.py b/pulpcore/app/tasks/importer.py index 3480348150c..86d64437195 100644 --- a/pulpcore/app/tasks/importer.py +++ b/pulpcore/app/tasks/importer.py @@ -12,10 +12,9 @@ from django.core.files.storage import default_storage from django.db.models import F from io import StringIO -from rest_framework.serializers import ValidationError from tablib import Dataset -from pulpcore.exceptions.plugin import MissingPlugin +from pulpcore.exceptions import MissingPlugin, ImportError from pulpcore.app.apps import get_plugin_config from pulpcore.app.models import ( AppStatus, @@ -78,7 +77,7 @@ def __init__(self, toc_path): with open(toc_path, "r") as toc_file: self.toc = json.load(toc_file) if "files" not in self.toc or "meta" not in self.toc: - raise ValidationError(_("Missing 'files' or 'meta' keys in table-of-contents!")) + raise ImportError(_("Missing 'files' or 'meta' keys in table-of-contents!")) toc_dir = os.path.dirname(toc_path) # sorting-by-filename is REALLY IMPORTANT here @@ -126,7 +125,7 @@ def read(self, size): if read_size < current_size: # Reached EOF (should only happen on the last chunk) if self.chunk != len(self.chunks) - 1: - raise Exception(f"Short read from chunk {self.chunk}.") + raise ImportError(f"Short read from chunk {self.chunk}.") return data if remaining_size == 0: return data @@ -154,7 +153,7 @@ def validate_chunks(self): * point to chunks whose checksums match the checksums stored in the 'toc' file Raises: - ValidationError: When toc points to chunked-export-files that can't be found in the + ImportError: When toc points to chunked-export-files that can't be found in the same directory as the toc-file, or the checksums of the chunks do not match the checksums stored in toc. """ @@ -164,7 +163,7 @@ def validate_chunks(self): if not os.path.isfile(chunk_path): missing_files.append(chunk_path) if missing_files: - raise ValidationError( + raise ImportError( _( "Missing import-chunks named in table-of-contents: {}.".format( str(missing_files) @@ -190,7 +189,7 @@ def validate_chunks(self): # if there are any errors, report and fail if errs: - raise ValidationError(_("Import chunk hash mismatch: {}).").format(str(errs))) + raise ImportError(_("Import chunk hash mismatch: {}).").format(str(errs))) def _get_destination_repo_name(importer, source_repo_name): @@ -293,7 +292,7 @@ def _check_versions(version_json): Compare the export version_json to the installed components. An upstream whose db-metadata doesn't match the downstream won't import successfully; check - for compatibility and raise a ValidationError if incompatible versions are found. + for compatibility and raise a ImportError if incompatible versions are found. """ error_messages = [] for component in version_json: @@ -320,7 +319,7 @@ def _check_versions(version_json): ) if error_messages: - raise ValidationError((" ".join(error_messages))) + raise ImportError((" ".join(error_messages))) def import_repository_version( @@ -366,7 +365,7 @@ def import_repository_version( tar.extract(mem, path=temp_dir) if not rv_name: - raise ValidationError(_("No RepositoryVersion found for {}").format(rv_name)) + raise ImportError(_("No RepositoryVersion found for {}").format(rv_name)) rv_path = os.path.join(temp_dir, rv_name) @@ -478,7 +477,7 @@ def safe_extract(tar, path=".", members=None, *, numeric_owner=False): for member in tar.getmembers(): member_path = os.path.join(path, member.name) if not is_within_directory(path, member_path): - raise Exception("Attempted Path Traversal in Tar File") + raise ImportError("Attempted Path Traversal in Tar File") tar.extractall(path, members, numeric_owner=numeric_owner) diff --git a/pulpcore/app/tasks/migrate.py b/pulpcore/app/tasks/migrate.py index e556a4185c5..b77bec9c202 100644 --- a/pulpcore/app/tasks/migrate.py +++ b/pulpcore/app/tasks/migrate.py @@ -2,7 +2,7 @@ from gettext import gettext as _ from django.utils.timezone import now -from rest_framework.serializers import ValidationError +from pulpcore.exceptions import ValidationError from pulpcore.app.models import Artifact, storage, ProgressReport from pulpcore.app.serializers import DomainBackendMigratorSerializer from pulpcore.app.util import get_domain diff --git a/pulpcore/app/tasks/purge.py b/pulpcore/app/tasks/purge.py index 69263e934e6..c98a5d10594 100644 --- a/pulpcore/app/tasks/purge.py +++ b/pulpcore/app/tasks/purge.py @@ -13,6 +13,7 @@ from pulpcore.app.role_util import get_objects_for_user from pulpcore.app.util import get_domain, get_current_authenticated_user from pulpcore.constants import TASK_STATES, TASK_FINAL_STATES +from pulpcore.exceptions import SystemStateError log = getLogger(__name__) @@ -107,7 +108,7 @@ def purge(finished_before=None, states=None, **kwargs): if not scheduled: current_user = get_current_authenticated_user() if current_user is None: - raise RuntimeError( + raise SystemStateError( "This task should have been dispatched by a user. Cannot find it though. " "Maybe it got deleted." ) diff --git a/pulpcore/app/tasks/replica.py b/pulpcore/app/tasks/replica.py index 137239a49f1..46d67f3023b 100644 --- a/pulpcore/app/tasks/replica.py +++ b/pulpcore/app/tasks/replica.py @@ -9,6 +9,7 @@ from pulpcore.app.apps import pulp_plugin_configs, PulpAppConfig from pulpcore.app.models import UpstreamPulp, Task, TaskGroup from pulpcore.app.replica import ReplicaContext +from pulpcore.exceptions import ReplicateError from pulpcore.tasking.tasks import dispatch from pulp_glue.common import __version__ as pulp_glue_version @@ -119,7 +120,7 @@ def finalize_replication(server_pk): task_group = TaskGroup.current() server = UpstreamPulp.objects.get(pk=server_pk) if task_group.tasks.exclude(pk=task.pk).exclude(state=TASK_STATES.COMPLETED).exists(): - raise Exception("Replication failed.") + raise ReplicateError() # Record timestamp of last successful replication. started_at = task_group.tasks.aggregate(Min("started_at"))["started_at__min"] diff --git a/pulpcore/app/tasks/upload.py b/pulpcore/app/tasks/upload.py index 8fb3eb6e463..c8a1c1e1f6c 100644 --- a/pulpcore/app/tasks/upload.py +++ b/pulpcore/app/tasks/upload.py @@ -1,9 +1,11 @@ from gettext import gettext as _ from logging import getLogger from tempfile import NamedTemporaryFile +from rest_framework.exceptions import ValidationError as DRFValidationError from pulpcore.app import files, models from pulpcore.app.serializers import ArtifactSerializer +from pulpcore.exceptions import ValidationError log = getLogger(__name__) @@ -34,7 +36,10 @@ def commit(upload_id, sha256): data = {"file": file, "sha256": sha256} serializer = ArtifactSerializer(data=data) - serializer.is_valid(raise_exception=True) + try: + serializer.is_valid(raise_exception=True) + except DRFValidationError as e: + raise ValidationError(e.detail) artifact = serializer.save() resource = models.CreatedResource(content_object=artifact) diff --git a/pulpcore/app/tasks/vulnerability_report.py b/pulpcore/app/tasks/vulnerability_report.py index bcac429a7b7..a1e5334e63c 100644 --- a/pulpcore/app/tasks/vulnerability_report.py +++ b/pulpcore/app/tasks/vulnerability_report.py @@ -9,6 +9,7 @@ from pulpcore.app.models.progress import ProgressReport from pulpcore.app.models.vulnerability_report import VulnerabilityReport from pulpcore.constants import OSV_QUERY_URL, TASK_STATES +from pulpcore.exceptions import ExternalServiceError class VulnerabilityReportScanner: @@ -118,7 +119,7 @@ async def _query_osv_api(self, osv_data): try: return await response.json() except aiohttp.ContentTypeError: - raise RuntimeError("Vuln report task failed to query osv.dev data.") + raise ExternalServiceError("osv.dev", "Failed to query vulnerability data.") async def _save_vulnerability_report(self, vulns, content, repo_version): """ diff --git a/pulpcore/download/http.py b/pulpcore/download/http.py index 795ed21c2fe..f0057ee468f 100644 --- a/pulpcore/download/http.py +++ b/pulpcore/download/http.py @@ -3,7 +3,6 @@ import aiohttp import asyncio import backoff -import socket from .base import BaseDownloader, DownloadResult from pulpcore.exceptions import ( @@ -300,11 +299,8 @@ async def _run(self, extra_data=None): self.raise_for_status(response) to_return = await self._handle_response(response) await response.release() - except aiohttp.ClientConnectorError as e: - # Check if this is a DNS error - if isinstance(e.os_error, socket.gaierror): - raise DnsDomainNameException(self.url) - raise + except aiohttp.ClientConnectorDNSError: + raise DnsDomainNameException(self.url) if self._close_session_on_finalize: await self.session.close() return to_return diff --git a/pulpcore/exceptions/__init__.py b/pulpcore/exceptions/__init__.py index c3bee9140a2..a5343971ab8 100644 --- a/pulpcore/exceptions/__init__.py +++ b/pulpcore/exceptions/__init__.py @@ -9,6 +9,13 @@ ProxyAuthenticationError, InternalErrorException, RepositoryVersionDeleteError, + ExternalServiceError, + ExportError, + ImportError, + SystemStateError, + ReplicateError, + SyncError, + PublishError, ) from .validation import ( DigestValidationError, @@ -18,3 +25,4 @@ MissingDigestValidationError, UnsupportedDigestValidationError, ) +from .plugin import MissingPlugin diff --git a/pulpcore/exceptions/base.py b/pulpcore/exceptions/base.py index 7adb5f6a45b..59a3ee598fc 100644 --- a/pulpcore/exceptions/base.py +++ b/pulpcore/exceptions/base.py @@ -8,15 +8,11 @@ class PulpException(Exception): """ http_status_code = http.client.INTERNAL_SERVER_ERROR + error_code = None - def __init__(self, error_code): - """ - :param error_code: unique error code - :type error_code: str - """ - if not isinstance(error_code, str): - raise TypeError(_("Error code must be an instance of str.")) - self.error_code = error_code + def __init__(self): + if not isinstance(self.error_code, str): + raise NotImplementedError("ABC error. Subclass must define a unique error code.") def __str__(self): """ @@ -39,7 +35,10 @@ def exception_to_dict(exc): :return: dictionary representing the Exception :rtype: dict """ - return {"description": str(exc)} + dic = {"description": str(exc)} + if isinstance(exc, PulpException): + dic["error_code"] = exc.error_code + return dic class InternalErrorException(PulpException): @@ -47,8 +46,7 @@ class InternalErrorException(PulpException): Exception to signal that an unexpected internal error occurred. """ - def __init__(self): - super().__init__("PLP0000") + error_code = "PLP0000" def __str__(self): return f"[{self.error_code}] " + _("An internal error occurred.") @@ -59,12 +57,13 @@ class ResourceImmutableError(PulpException): Exceptions that are raised due to trying to update an immutable resource """ + error_code = "PLP0006" + def __init__(self, model): """ Args: model (pulpcore.app.models.Model): that the user is trying to update """ - super().__init__("PLP0006") self.model = model def __str__(self): @@ -79,12 +78,13 @@ class TimeoutException(PulpException): Exception to signal timeout error. """ + error_code = "PLP0005" + def __init__(self, url): """ :param url: the url the download for timed out :type url: str """ - super().__init__("PLP0005") self.url = url def __str__(self): @@ -99,8 +99,7 @@ class DomainProtectedError(PulpException): repositories with content. """ - def __init__(self): - super().__init__("PLP0007") + error_code = "PLP0007" def __str__(self): return f"[{self.error_code}] " + _( @@ -113,12 +112,13 @@ class DnsDomainNameException(PulpException): Exception to signal that dns could not resolve the domain name for specified url. """ + error_code = "PLP0008" + def __init__(self, url): """ :param url: the url that dns could not resolve :type url: str """ - super().__init__("PLP0008") self.url = url def __str__(self): @@ -131,12 +131,13 @@ class UrlSchemeNotSupportedError(PulpException): Pulp does not have a registered handler for. """ + error_code = "PLP0009" + def __init__(self, url): """ :param url: The full URL that failed validation. :type url: str """ - super().__init__("PLP0009") self.url = url def __str__(self): @@ -149,12 +150,13 @@ class ProxyAuthenticationError(PulpException): but it was not provided or is invalid """ + error_code = "PLP0010" + def __init__(self, proxy_url): """ :param proxy_url: The URL of the proxy server. :type proxy_url: str """ - super().__init__("PLP0010") self.proxy_url = proxy_url def __str__(self): @@ -168,11 +170,148 @@ class RepositoryVersionDeleteError(PulpException): Raised when attempting to delete a repository version that cannot be deleted """ - def __init__(self): - super().__init__("PLP0011") + http_status_code = http.client.BAD_REQUEST + error_code = "PLP0011" + + def __init__(self, message=None): + """ + :param message: Description of the repository version delete error + :type message: str + """ + self.message = message or _( + "Cannot delete repository version. Repositories must have at least one repository " + "version." + ) def __str__(self): - return f"[{self.error_code}] " + _( - "Cannot delete repository version. Repositories must have at least one " - "repository version." + return f"[{self.error_code}] " + self.message + + +class PublishError(PulpException): + """ + Raised when a publish operation fails. + """ + + error_code = "PLP0012" + + def __init__(self, message=None): + """ + :param message: Description of the publish error + :type message: str + """ + self.message = message + + def __str__(self): + return f"[{self.error_code}] " + _("Publish failed: {message}").format(message=self.message) + + +class SyncError(PulpException): + """ + Raised when a sync operation fails. + """ + + error_code = "PLP0013" + + def __init__(self, message): + """ + :param message: Description of the sync error + :type message: str + """ + self.message = message + + def __str__(self): + return f"[{self.error_code}] " + _("Sync failed: {message}").format(message=self.message) + + +class ExternalServiceError(PulpException): + """ + Raised when an external API or service fails. + """ + + http_status_code = http.client.BAD_GATEWAY + error_code = "PLP0014" + + def __init__(self, service_name, details=None): + """ + :param service_name: Name of the external service + :type service_name: str + :param details: Additional details about the failure + :type details: str or None + """ + self.service_name = service_name + self.details = details + + def __str__(self): + msg = _("External service '{service}' failed").format(service=self.service_name) + if self.details: + msg += f": {self.details}" + return f"[{self.error_code}] {msg}" + + +class ExportError(PulpException): + """ + Raised when export operation fails due to configuration or preconditions. + """ + + http_status_code = http.client.BAD_REQUEST + error_code = "PLP0015" + + def __init__(self, message): + """ + :param message: Description of the export error + :type message: str + """ + self.message = message + + def __str__(self): + return f"[{self.error_code}] " + _("Export failed: {message}").format(message=self.message) + + +class ImportError(PulpException): + """ + Raised when an import operation fails due to configuration or preconditions. + """ + + http_status_code = http.client.BAD_REQUEST + error_code = "PLP0016" + + def __init__(self, message): + """ + :param message: Description of the import error + :type message: str + """ + self.message = message + + def __str__(self): + return f"[{self.error_code}] " + _("Import failed: {message}").format(message=self.message) + + +class SystemStateError(PulpException): + """ + Raised when system is in an unexpected state. + """ + + error_code = "PLP0017" + + def __init__(self, message): + """ + :param message: Description of the system state error + :type message: str + """ + self.message = message + + def __str__(self): + return f"[{self.error_code}] " + _("System state error: {message}").format( + message=self.message ) + + +class ReplicateError(PulpException): + """ + Raised when a replicate operation fails. + """ + + error_code = "PLP0018" + + def __str__(self): + return f"[{self.error_code}] " + _("Replication failed") diff --git a/pulpcore/exceptions/plugin.py b/pulpcore/exceptions/plugin.py index 528d04bcb40..e4a40ca28e9 100644 --- a/pulpcore/exceptions/plugin.py +++ b/pulpcore/exceptions/plugin.py @@ -10,12 +10,13 @@ class MissingPlugin(PulpException): Exception that is raised when a requested plugin is not installed. """ + error_code = "PLP0002" + def __init__(self, plugin_app_label): """ :param resources: keyword arguments of resource_type=resource_id :type resources: dict """ - super().__init__("PLP0002") self.plugin_app_label = plugin_app_label def __str__(self): diff --git a/pulpcore/exceptions/validation.py b/pulpcore/exceptions/validation.py index 9a0ba045cda..953f54037a5 100644 --- a/pulpcore/exceptions/validation.py +++ b/pulpcore/exceptions/validation.py @@ -1,5 +1,5 @@ from gettext import gettext as _ - +import http.client from pulpcore.exceptions import PulpException @@ -8,7 +8,14 @@ class ValidationError(PulpException): A base class for all Validation Errors. """ - pass + http_status_code = http.client.BAD_REQUEST + error_code = "PLP0001" + + def __init__(self, msg): + self.msg = msg + + def __str__(self): + return f"[{self.error_code}] Validation Error: {self.msg}" class DigestValidationError(ValidationError): @@ -16,8 +23,9 @@ class DigestValidationError(ValidationError): Raised when a file fails to validate a digest checksum. """ + error_code = "PLP0003" + def __init__(self, actual, expected, *args, url=None, **kwargs): - super().__init__("PLP0003") self.url = url self.actual = actual self.expected = expected @@ -43,8 +51,9 @@ class SizeValidationError(ValidationError): Raised when a file fails to validate a size checksum. """ + error_code = "PLP0004" + def __init__(self, actual, expected, *args, url=None, **kwargs): - super().__init__("PLP0004") self.url = url self.actual = actual self.expected = expected @@ -65,27 +74,48 @@ def __str__(self): return f"[{self.error_code}] " + msg.format(expected=self.expected, actual=self.actual) -class MissingDigestValidationError(Exception): +class MissingDigestValidationError(ValidationError): """ Raised when attempting to save() an Artifact with an incomplete set of checksums. """ - pass + error_code = "PLP0019" + def __init__(self, message=None): + self.message = message or _("Artifact is missing required checksums.") + + def __str__(self): + return f"[{self.error_code}] {self.message}" -class UnsupportedDigestValidationError(Exception): + +class UnsupportedDigestValidationError(ValidationError): """ Raised when an attempt is made to use a checksum-type that is not enabled/available. """ - pass + error_code = "PLP0020" + + def __init__(self, digest_name=None): + self.digest_name = digest_name + + def __str__(self): + if self.digest_name: + return f"[{self.error_code}] " + _( + "Checksum type '{digest}' is not supported or enabled." + ).format(digest=self.digest_name) + return f"[{self.error_code}] " + _("Unsupported checksum type.") -class InvalidSignatureError(RuntimeError): +class InvalidSignatureError(ValidationError): """ Raised when a signature could not be verified by the GnuPG utility. """ - def __init__(self, *args, **kwargs): - self.verified = kwargs.pop("verified", None) - super().__init__(*args, **kwargs) + error_code = "PLP0021" + + def __init__(self, message=None, verified=None): + self.message = message or _("Signature verification failed.") + self.verified = verified + + def __str__(self): + return f"[{self.error_code}] {self.message}" diff --git a/pulpcore/plugin/exceptions.py b/pulpcore/plugin/exceptions.py index 59870dc6244..c86f93932d4 100644 --- a/pulpcore/plugin/exceptions.py +++ b/pulpcore/plugin/exceptions.py @@ -1,4 +1,5 @@ from pulpcore.exceptions import ( + ValidationError, DigestValidationError, InvalidSignatureError, PulpException, @@ -6,10 +7,16 @@ MissingDigestValidationError, TimeoutException, UnsupportedDigestValidationError, + PublishError, + SyncError, + ExternalServiceError, + SystemStateError, + ReplicateError, ) __all__ = [ + "ValidationError", "DigestValidationError", "InvalidSignatureError", "PulpException", @@ -17,4 +24,9 @@ "MissingDigestValidationError", "TimeoutException", "UnsupportedDigestValidationError", + "PublishError", + "SyncError", + "ExternalServiceError", + "SystemStateError", + "ReplicateError", ] diff --git a/pulpcore/plugin/stages/artifact_stages.py b/pulpcore/plugin/stages/artifact_stages.py index f67664ce872..82c89e56658 100644 --- a/pulpcore/plugin/stages/artifact_stages.py +++ b/pulpcore/plugin/stages/artifact_stages.py @@ -4,12 +4,12 @@ import logging from django.conf import settings - +import aiohttp.client_exceptions from aiofiles import os as aos from asgiref.sync import sync_to_async from django.db.models import Prefetch, prefetch_related_objects -from pulpcore.plugin.exceptions import UnsupportedDigestValidationError +from pulpcore.plugin.exceptions import UnsupportedDigestValidationError, SyncError from pulpcore.plugin.models import ( Artifact, ContentArtifact, @@ -240,7 +240,10 @@ async def _handle_content_unit(self, d_content): and not d_artifact.artifact.file ] if downloaders_for_content: - await asyncio.gather(*downloaders_for_content) + try: + await asyncio.gather(*downloaders_for_content) + except aiohttp.client_exceptions.ClientResponseError as e: + raise SyncError(_("Error downloading artifact: {error}").format(error=e)) await self.put(d_content) return len(downloaders_for_content) @@ -407,7 +410,7 @@ async def _handle_remote_artifacts(self, batch): "No declared artifact with relative path '{rp}' for content '{c}'" " from remote '{rname}', and no paths available." ) - raise ValueError( + raise SyncError( msg.format( rp=d_artifact.relative_path, c=d_content.content.natural_key(), @@ -416,7 +419,7 @@ async def _handle_remote_artifacts(self, batch): ) else: msg = _('No declared artifact with relative path "{rp}" for content "{c}"') - raise ValueError( + raise SyncError( msg.format(rp=d_artifact.relative_path, c=d_content.content) ) diff --git a/pulpcore/plugin/stages/content_stages.py b/pulpcore/plugin/stages/content_stages.py index 5766b4e404a..6070c30bfc9 100644 --- a/pulpcore/plugin/stages/content_stages.py +++ b/pulpcore/plugin/stages/content_stages.py @@ -1,3 +1,4 @@ +from gettext import gettext as _ from collections import defaultdict from asgiref.sync import sync_to_async @@ -7,6 +8,7 @@ from pulpcore.plugin.sync import sync_to_async_iterable +from pulpcore.plugin.exceptions import SyncError from pulpcore.plugin.models import Content, ContentArtifact, ProgressReport from .api import Stage @@ -115,13 +117,14 @@ def process_batch(): try: with transaction.atomic(): d_content.content.save() - except IntegrityError as e: + except IntegrityError: try: d_content.content = d_content.content.__class__.objects.get( d_content.content.q() ) except ObjectDoesNotExist: - raise e + msg = _('Content not found during save "{c}"') + raise SyncError(msg.format(c=d_content.content.natural_key())) else: for d_artifact in d_content.d_artifacts: if not d_artifact.artifact._state.adding: diff --git a/pulpcore/tests/unit/test_chunked_file.py b/pulpcore/tests/unit/test_chunked_file.py index a7e0fc55396..649cc3563f7 100644 --- a/pulpcore/tests/unit/test_chunked_file.py +++ b/pulpcore/tests/unit/test_chunked_file.py @@ -3,11 +3,11 @@ from pathlib import Path import pytest -from rest_framework.exceptions import ValidationError from pulpcore.app.models import ProgressReport from pulpcore.app.tasks.importer import ChunkedFile from pulpcore.app.util import Crc32Hasher, compute_file_hash +from pulpcore.exceptions import ImportError def write_chunk_files(tmp_path: Path, data_chunks: t.List[t.ByteString]): @@ -131,7 +131,7 @@ def test_chunked_file_validate_raises(tmp_path, monkeypatch): tmp_path, data_chunks=chunks_list, chunk_size=chunk_size, corrupted=True ) chunked_file = ChunkedFile(toc_path) - with pytest.raises(ValidationError, match="Import chunk hash mismatch.*"): + with pytest.raises(ImportError, match="Import chunk hash mismatch.*"): chunked_file.validate_chunks() @@ -144,7 +144,7 @@ def test_chunked_file_shortread_exception(tmp_path): toc_path = create_tocfile(tmp_path, data_chunks=malformed_chunks_list, chunk_size=chunk_size) chunked_file = ChunkedFile(toc_path) - with pytest.raises(Exception, match=r"Short read from chunk \d*."): + with pytest.raises(ImportError, match=r"Short read from chunk \d*."): with chunked_file as fp: content_size = len(contiguous_data) fp.read(content_size) diff --git a/pulpcore/tests/unit/test_import_checks.py b/pulpcore/tests/unit/test_import_checks.py index 0a40f6480ef..cf833f9e0b0 100644 --- a/pulpcore/tests/unit/test_import_checks.py +++ b/pulpcore/tests/unit/test_import_checks.py @@ -1,9 +1,8 @@ import pytest -from rest_framework.serializers import ValidationError - import pulpcore.app.apps from pulpcore.app.tasks.importer import _check_versions +from pulpcore.exceptions import ImportError def _pulp_plugin_configs(): @@ -34,20 +33,20 @@ def test_vers_check(monkeypatch): _check_versions(export_json) export_json = [{"component": "123", "version": "1.4.3"}] - with pytest.raises(ValidationError): + with pytest.raises(ImportError): _check_versions(export_json) export_json = [{"component": "123", "version": "2.2.3"}] - with pytest.raises(ValidationError): + with pytest.raises(ImportError): _check_versions(export_json) export_json = [{"component": "non_existent", "version": "1.2.3"}] - with pytest.raises(ValidationError): + with pytest.raises(ImportError): _check_versions(export_json) export_json = [ {"component": "123", "version": "1.2.3"}, {"component": "non_existent", "version": "1.2.3"}, ] - with pytest.raises(ValidationError): + with pytest.raises(ImportError): _check_versions(export_json) diff --git a/pyproject.toml b/pyproject.toml index ac1acf902b2..cb08059b68e 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -27,7 +27,7 @@ requires-python = ">=3.11" dependencies = [ "aiodns>=3.3.0,<3.7", # Looks like only bugfixes in z-Stream. "aiofiles>=22.1,<=25.1.0", # Uses sort of CalVer, luckily not released often https://github.com/Tinche/aiofiles/issues/144 . - "aiohttp>=3.8.3,<3.14", # SemVer https://docs.aiohttp.org/en/stable/faq.html#what-is-the-api-stability-and-deprecation-policy . + "aiohttp>=3.10.10,<3.14", # SemVer https://docs.aiohttp.org/en/stable/faq.html#what-is-the-api-stability-and-deprecation-policy . "asyncio-throttle>=1.0,<=1.0.2", # Unsure about versioning, but not released often anyway. "backoff>=2.1.2,<2.3", # Looks like only bugfixes in z-Stream. "click>=8.1.0,<8.4", # Uses milestone.feature.fix https://palletsprojects.com/versions . @@ -63,7 +63,7 @@ dependencies = [ "url-normalize>=1.4.3,<2.3", # SemVer. https://github.com/niksite/url-normalize/blob/master/CHANGELOG.md#changelog "uuid6>=2023.5.2,<=2025.0.1", "whitenoise>=5.0,<6.12.0", - "yarl>=1.9.1,<1.23", # Seems to follow SemVer https://yarl.aio-libs.org/en/latest/contributing/release_guide/#pre-release-activities . + "yarl>=1.12.0,<1.23", # Seems to follow SemVer https://yarl.aio-libs.org/en/latest/contributing/release_guide/#pre-release-activities . ] [project.optional-dependencies]