From 8e37dbdf1244ac840efbf2ed6a96ca418ee45afc Mon Sep 17 00:00:00 2001 From: Patrick Ogenstad Date: Fri, 18 Sep 2026 16:48:17 +0200 Subject: [PATCH 1/3] feat: add the generated error catalogue bindings Generated by `uv run invoke backend.generate` from the Infrahub branch that adds the generator. Nine exception classes, fifteen payload models, and a dispatch that resolves a catalogue code and its payload to a concrete exception. The three codes the server reports as 401 or 403 get a payload model but no class: the SDK routes those by the response it saw. This file is generated and must not be hand-edited. Infrahub owns it, the same way it owns the schema models and the protocols. --- infrahub_sdk/exceptions/catalogue.py | 640 +++++++++++++++++++++++++++ 1 file changed, 640 insertions(+) create mode 100644 infrahub_sdk/exceptions/catalogue.py diff --git a/infrahub_sdk/exceptions/catalogue.py b/infrahub_sdk/exceptions/catalogue.py new file mode 100644 index 00000000..d2b8f0a7 --- /dev/null +++ b/infrahub_sdk/exceptions/catalogue.py @@ -0,0 +1,640 @@ +# Generated from schema/error-catalogue.json in the opsmill/infrahub repository - DO NOT EDIT. +# Catalogue version: 1 +# Regenerate there with: uv run invoke backend.generate +# +# Stability: a "stable" code keeps its payload shape; an "evolving" code may still gain fields. +from __future__ import annotations + +from collections.abc import Callable, Mapping +from datetime import datetime +from typing import Any, ClassVar + +from pydantic import BaseModel, ConfigDict +from typing_extensions import Self + +from .base import BranchNotFoundError, GraphQLError, NodeNotFoundError, SchemaNotFoundError + +__all__ = [ + "CODE_TO_DATA_MODEL", + "CODE_TO_EXCEPTION", + "AttributeConstraintViolationData", + "AttributeConstraintViolationError", + "AttributeInvalidTypeData", + "AttributeInvalidTypeError", + "AttributeRequiredData", + "AttributeRequiredError", + "AuthenticationRequiredData", + "BranchAlreadyMergedData", + "BranchAlreadyMergedError", + "BranchNeedsRebaseData", + "BranchNeedsRebaseError", + "BranchNotFoundData", + "MergeInProgressData", + "MergeInProgressError", + "MergeRecoveryRequiredData", + "MergeRecoveryRequiredError", + "NodeNotFoundData", + "PermissionDeniedData", + "SchemaNotFoundData", + "TokenExpiredData", + "UndefinedError", + "UndefinedErrorData", + "UniquenessViolationData", + "UniquenessViolationError", + "exception_from_payload", +] + + +class AttributeConstraintViolationData(BaseModel): + """Payload a server-reported ATTRIBUTE_CONSTRAINT_VIOLATION carries.""" + + # A newer server may add fields this SDK has never heard of; ignoring them keeps the code + # resolvable rather than failing validation. + model_config = ConfigDict(extra="ignore") + + node_kind: str + field_name: str + constraint: str + detail: str | None = None + + +class AttributeInvalidTypeData(BaseModel): + """Payload a server-reported ATTRIBUTE_INVALID_TYPE carries.""" + + # A newer server may add fields this SDK has never heard of; ignoring them keeps the code + # resolvable rather than failing validation. + model_config = ConfigDict(extra="ignore") + + node_kind: str + field_name: str + expected_type: str + received_type: str + + +class AttributeRequiredData(BaseModel): + """Payload a server-reported ATTRIBUTE_REQUIRED carries.""" + + # A newer server may add fields this SDK has never heard of; ignoring them keeps the code + # resolvable rather than failing validation. + model_config = ConfigDict(extra="ignore") + + node_kind: str + field_name: str + + +class AuthenticationRequiredData(BaseModel): + """Payload a server-reported AUTHENTICATION_REQUIRED carries.""" + + # A newer server may add fields this SDK has never heard of; ignoring them keeps the code + # resolvable rather than failing validation. + model_config = ConfigDict(extra="ignore") + + +class BranchAlreadyMergedData(BaseModel): + """Payload a server-reported BRANCH_ALREADY_MERGED carries.""" + + # A newer server may add fields this SDK has never heard of; ignoring them keeps the code + # resolvable rather than failing validation. + model_config = ConfigDict(extra="ignore") + + branch_name: str + + +class BranchNeedsRebaseData(BaseModel): + """Payload a server-reported BRANCH_NEEDS_REBASE carries.""" + + # A newer server may add fields this SDK has never heard of; ignoring them keeps the code + # resolvable rather than failing validation. + model_config = ConfigDict(extra="ignore") + + branch_name: str + + +class BranchNotFoundData(BaseModel): + """Payload a server-reported BRANCH_NOT_FOUND carries.""" + + # A newer server may add fields this SDK has never heard of; ignoring them keeps the code + # resolvable rather than failing validation. + model_config = ConfigDict(extra="ignore") + + branch_name: str + + +class MergeInProgressData(BaseModel): + """Payload a server-reported MERGE_IN_PROGRESS carries.""" + + # A newer server may add fields this SDK has never heard of; ignoring them keeps the code + # resolvable rather than failing validation. + model_config = ConfigDict(extra="ignore") + + branch_name: str + merging_branch: str + + +class MergeRecoveryRequiredData(BaseModel): + """Payload a server-reported MERGE_RECOVERY_REQUIRED carries.""" + + # A newer server may add fields this SDK has never heard of; ignoring them keeps the code + # resolvable rather than failing validation. + model_config = ConfigDict(extra="ignore") + + branch_name: str + merging_branch: str + + +class NodeNotFoundData(BaseModel): + """Payload a server-reported NODE_NOT_FOUND carries.""" + + # A newer server may add fields this SDK has never heard of; ignoring them keeps the code + # resolvable rather than failing validation. + model_config = ConfigDict(extra="ignore") + + node_kind: str + identifier: str + + +class PermissionDeniedData(BaseModel): + """Payload a server-reported PERMISSION_DENIED carries.""" + + # A newer server may add fields this SDK has never heard of; ignoring them keeps the code + # resolvable rather than failing validation. + model_config = ConfigDict(extra="ignore") + + action: str | None = None + resource_kind: str | None = None + + +class SchemaNotFoundData(BaseModel): + """Payload a server-reported SCHEMA_NOT_FOUND carries.""" + + # A newer server may add fields this SDK has never heard of; ignoring them keeps the code + # resolvable rather than failing validation. + model_config = ConfigDict(extra="ignore") + + kind: str + + +class TokenExpiredData(BaseModel): + """Payload a server-reported TOKEN_EXPIRED carries.""" + + # A newer server may add fields this SDK has never heard of; ignoring them keeps the code + # resolvable rather than failing validation. + model_config = ConfigDict(extra="ignore") + + expired_at: datetime | None = None + + +class UndefinedErrorData(BaseModel): + """Payload a server-reported UNDEFINED_ERROR carries.""" + + # A newer server may add fields this SDK has never heard of; ignoring them keeps the code + # resolvable rather than failing validation. + model_config = ConfigDict(extra="ignore") + + +class UniquenessViolationData(BaseModel): + """Payload a server-reported UNIQUENESS_VIOLATION carries.""" + + # A newer server may add fields this SDK has never heard of; ignoring them keeps the code + # resolvable rather than failing validation. + model_config = ConfigDict(extra="ignore") + + node_kind: str + fields: list[str] + + +class AttributeConstraintViolationError(GraphQLError): + """Raised when the server reports ATTRIBUTE_CONSTRAINT_VIOLATION. + + A node attribute value failed a schema-defined constraint (e.g. regex, length, range). + + Stability: evolving. + """ + + CODE: ClassVar[str | None] = "ATTRIBUTE_CONSTRAINT_VIOLATION" + DATA_MODEL: ClassVar[type[BaseModel]] = AttributeConstraintViolationData + + code: str | None = "ATTRIBUTE_CONSTRAINT_VIOLATION" + http_status: int | None = 422 + + def __init__( + self, + *, + node_kind: str, + field_name: str, + constraint: str, + detail: str | None = None, + errors: list[dict[str, Any]] | None = None, + query: str | None = None, + variables: dict | None = None, + message: str | None = None, + ) -> None: + self.node_kind = node_kind + self.field_name = field_name + self.constraint = constraint + self.detail = detail + super().__init__(errors=errors or [], query=query, variables=variables, message=message) + + @classmethod + def from_payload(cls, payload: AttributeConstraintViolationData) -> Self: + """Build the exception from the validated payload of a server-reported failure.""" + return cls( + node_kind=payload.node_kind, + field_name=payload.field_name, + constraint=payload.constraint, + detail=payload.detail, + ) + + +class AttributeInvalidTypeError(GraphQLError): + """Raised when the server reports ATTRIBUTE_INVALID_TYPE. + + A node attribute received a value that does not match its declared type. + + Stability: stable. + """ + + CODE: ClassVar[str | None] = "ATTRIBUTE_INVALID_TYPE" + DATA_MODEL: ClassVar[type[BaseModel]] = AttributeInvalidTypeData + + code: str | None = "ATTRIBUTE_INVALID_TYPE" + http_status: int | None = 422 + + def __init__( + self, + *, + node_kind: str, + field_name: str, + expected_type: str, + received_type: str, + errors: list[dict[str, Any]] | None = None, + query: str | None = None, + variables: dict | None = None, + message: str | None = None, + ) -> None: + self.node_kind = node_kind + self.field_name = field_name + self.expected_type = expected_type + self.received_type = received_type + super().__init__(errors=errors or [], query=query, variables=variables, message=message) + + @classmethod + def from_payload(cls, payload: AttributeInvalidTypeData) -> Self: + """Build the exception from the validated payload of a server-reported failure.""" + return cls( + node_kind=payload.node_kind, + field_name=payload.field_name, + expected_type=payload.expected_type, + received_type=payload.received_type, + ) + + +class AttributeRequiredError(GraphQLError): + """Raised when the server reports ATTRIBUTE_REQUIRED. + + A mandatory node attribute was not provided. + + Stability: stable. + """ + + CODE: ClassVar[str | None] = "ATTRIBUTE_REQUIRED" + DATA_MODEL: ClassVar[type[BaseModel]] = AttributeRequiredData + + code: str | None = "ATTRIBUTE_REQUIRED" + http_status: int | None = 422 + + def __init__( + self, + *, + node_kind: str, + field_name: str, + errors: list[dict[str, Any]] | None = None, + query: str | None = None, + variables: dict | None = None, + message: str | None = None, + ) -> None: + self.node_kind = node_kind + self.field_name = field_name + super().__init__(errors=errors or [], query=query, variables=variables, message=message) + + @classmethod + def from_payload(cls, payload: AttributeRequiredData) -> Self: + """Build the exception from the validated payload of a server-reported failure.""" + return cls(node_kind=payload.node_kind, field_name=payload.field_name) + + +class BranchAlreadyMergedError(GraphQLError): + """Raised when the server reports BRANCH_ALREADY_MERGED. + + The target branch has been merged and is permanently read-only. + + Stability: stable. + """ + + CODE: ClassVar[str | None] = "BRANCH_ALREADY_MERGED" + DATA_MODEL: ClassVar[type[BaseModel]] = BranchAlreadyMergedData + + code: str | None = "BRANCH_ALREADY_MERGED" + http_status: int | None = 400 + + def __init__( + self, + *, + branch_name: str, + errors: list[dict[str, Any]] | None = None, + query: str | None = None, + variables: dict | None = None, + message: str | None = None, + ) -> None: + self.branch_name = branch_name + super().__init__(errors=errors or [], query=query, variables=variables, message=message) + + @classmethod + def from_payload(cls, payload: BranchAlreadyMergedData) -> Self: + """Build the exception from the validated payload of a server-reported failure.""" + return cls(branch_name=payload.branch_name) + + +class BranchNeedsRebaseError(GraphQLError): + """Raised when the server reports BRANCH_NEEDS_REBASE. + + The target branch must be rebased before it accepts new changes. + + Stability: stable. + """ + + CODE: ClassVar[str | None] = "BRANCH_NEEDS_REBASE" + DATA_MODEL: ClassVar[type[BaseModel]] = BranchNeedsRebaseData + + code: str | None = "BRANCH_NEEDS_REBASE" + http_status: int | None = 400 + + def __init__( + self, + *, + branch_name: str, + errors: list[dict[str, Any]] | None = None, + query: str | None = None, + variables: dict | None = None, + message: str | None = None, + ) -> None: + self.branch_name = branch_name + super().__init__(errors=errors or [], query=query, variables=variables, message=message) + + @classmethod + def from_payload(cls, payload: BranchNeedsRebaseData) -> Self: + """Build the exception from the validated payload of a server-reported failure.""" + return cls(branch_name=payload.branch_name) + + +class MergeInProgressError(GraphQLError): + """Raised when the server reports MERGE_IN_PROGRESS. + + The write was rejected because a branch merge is in progress. The block is transient; retry once the + merge completes. + + Stability: evolving. + """ + + CODE: ClassVar[str | None] = "MERGE_IN_PROGRESS" + DATA_MODEL: ClassVar[type[BaseModel]] = MergeInProgressData + + code: str | None = "MERGE_IN_PROGRESS" + http_status: int | None = 423 + + def __init__( + self, + *, + branch_name: str, + merging_branch: str, + errors: list[dict[str, Any]] | None = None, + query: str | None = None, + variables: dict | None = None, + message: str | None = None, + ) -> None: + self.branch_name = branch_name + self.merging_branch = merging_branch + super().__init__(errors=errors or [], query=query, variables=variables, message=message) + + @classmethod + def from_payload(cls, payload: MergeInProgressData) -> Self: + """Build the exception from the validated payload of a server-reported failure.""" + return cls(branch_name=payload.branch_name, merging_branch=payload.merging_branch) + + +class MergeRecoveryRequiredError(GraphQLError): + """Raised when the server reports MERGE_RECOVERY_REQUIRED. + + The write was rejected because a previous branch merge failed and left the default branch protected. + Recovery is required: an administrator must run `infrahub recover merge`. Unlike MERGE_IN_PROGRESS + this is not retryable. + + Stability: evolving. + """ + + CODE: ClassVar[str | None] = "MERGE_RECOVERY_REQUIRED" + DATA_MODEL: ClassVar[type[BaseModel]] = MergeRecoveryRequiredData + + code: str | None = "MERGE_RECOVERY_REQUIRED" + http_status: int | None = 423 + + def __init__( + self, + *, + branch_name: str, + merging_branch: str, + errors: list[dict[str, Any]] | None = None, + query: str | None = None, + variables: dict | None = None, + message: str | None = None, + ) -> None: + self.branch_name = branch_name + self.merging_branch = merging_branch + super().__init__(errors=errors or [], query=query, variables=variables, message=message) + + @classmethod + def from_payload(cls, payload: MergeRecoveryRequiredData) -> Self: + """Build the exception from the validated payload of a server-reported failure.""" + return cls(branch_name=payload.branch_name, merging_branch=payload.merging_branch) + + +class UndefinedError(GraphQLError): + """Raised when the server reports UNDEFINED_ERROR. + + An error not yet covered by the catalogue. Its occurrence indicates a catalogue gap and should be + triaged. + + Stability: stable. + """ + + CODE: ClassVar[str | None] = "UNDEFINED_ERROR" + DATA_MODEL: ClassVar[type[BaseModel]] = UndefinedErrorData + + code: str | None = "UNDEFINED_ERROR" + http_status: int | None = 500 + + def __init__( + self, + *, + errors: list[dict[str, Any]] | None = None, + query: str | None = None, + variables: dict | None = None, + message: str | None = None, + ) -> None: + super().__init__(errors=errors or [], query=query, variables=variables, message=message) + + @classmethod + def from_payload(cls, payload: UndefinedErrorData) -> Self: + """Build the exception from the validated payload of a server-reported failure.""" + del payload + return cls() + + +class UniquenessViolationError(GraphQLError): + """Raised when the server reports UNIQUENESS_VIOLATION. + + The submitted values collide with an existing object on a uniqueness constraint. The constraint members + may be relationships as well as attributes. + + Stability: evolving. + """ + + CODE: ClassVar[str | None] = "UNIQUENESS_VIOLATION" + DATA_MODEL: ClassVar[type[BaseModel]] = UniquenessViolationData + + code: str | None = "UNIQUENESS_VIOLATION" + http_status: int | None = 422 + + def __init__( + self, + *, + node_kind: str, + fields: list[str], + errors: list[dict[str, Any]] | None = None, + query: str | None = None, + variables: dict | None = None, + message: str | None = None, + ) -> None: + self.node_kind = node_kind + self.fields = fields + super().__init__(errors=errors or [], query=query, variables=variables, message=message) + + @classmethod + def from_payload(cls, payload: UniquenessViolationData) -> Self: + """Build the exception from the validated payload of a server-reported failure.""" + return cls(node_kind=payload.node_kind, fields=payload.fields) + + +# Codes that map to a dedicated exception class. Authentication and permission codes are absent: the +# SDK raises a generic class for those and carries the code on the instance. +CODE_TO_EXCEPTION: dict[str, type[GraphQLError]] = { + "ATTRIBUTE_CONSTRAINT_VIOLATION": AttributeConstraintViolationError, + "ATTRIBUTE_INVALID_TYPE": AttributeInvalidTypeError, + "ATTRIBUTE_REQUIRED": AttributeRequiredError, + "BRANCH_ALREADY_MERGED": BranchAlreadyMergedError, + "BRANCH_NEEDS_REBASE": BranchNeedsRebaseError, + "BRANCH_NOT_FOUND": BranchNotFoundError, + "MERGE_IN_PROGRESS": MergeInProgressError, + "MERGE_RECOVERY_REQUIRED": MergeRecoveryRequiredError, + "NODE_NOT_FOUND": NodeNotFoundError, + "SCHEMA_NOT_FOUND": SchemaNotFoundError, + "UNDEFINED_ERROR": UndefinedError, + "UNIQUENESS_VIOLATION": UniquenessViolationError, +} + +# Every code's payload model, including the codes that get no class, so a caller that observes one +# can still validate what it carries. +CODE_TO_DATA_MODEL: dict[str, type[BaseModel]] = { + "ATTRIBUTE_CONSTRAINT_VIOLATION": AttributeConstraintViolationData, + "ATTRIBUTE_INVALID_TYPE": AttributeInvalidTypeData, + "ATTRIBUTE_REQUIRED": AttributeRequiredData, + "AUTHENTICATION_REQUIRED": AuthenticationRequiredData, + "BRANCH_ALREADY_MERGED": BranchAlreadyMergedData, + "BRANCH_NEEDS_REBASE": BranchNeedsRebaseData, + "BRANCH_NOT_FOUND": BranchNotFoundData, + "MERGE_IN_PROGRESS": MergeInProgressData, + "MERGE_RECOVERY_REQUIRED": MergeRecoveryRequiredData, + "NODE_NOT_FOUND": NodeNotFoundData, + "PERMISSION_DENIED": PermissionDeniedData, + "SCHEMA_NOT_FOUND": SchemaNotFoundData, + "TOKEN_EXPIRED": TokenExpiredData, + "UNDEFINED_ERROR": UndefinedErrorData, + "UNIQUENESS_VIOLATION": UniquenessViolationData, +} + + +def _build_attribute_constraint_violation(data: Mapping[str, Any]) -> GraphQLError: + return AttributeConstraintViolationError.from_payload(AttributeConstraintViolationData.model_validate(data)) + + +def _build_attribute_invalid_type(data: Mapping[str, Any]) -> GraphQLError: + return AttributeInvalidTypeError.from_payload(AttributeInvalidTypeData.model_validate(data)) + + +def _build_attribute_required(data: Mapping[str, Any]) -> GraphQLError: + return AttributeRequiredError.from_payload(AttributeRequiredData.model_validate(data)) + + +def _build_branch_already_merged(data: Mapping[str, Any]) -> GraphQLError: + return BranchAlreadyMergedError.from_payload(BranchAlreadyMergedData.model_validate(data)) + + +def _build_branch_needs_rebase(data: Mapping[str, Any]) -> GraphQLError: + return BranchNeedsRebaseError.from_payload(BranchNeedsRebaseData.model_validate(data)) + + +def _build_branch_not_found(data: Mapping[str, Any]) -> GraphQLError: + return BranchNotFoundError.from_payload(BranchNotFoundData.model_validate(data)) + + +def _build_merge_in_progress(data: Mapping[str, Any]) -> GraphQLError: + return MergeInProgressError.from_payload(MergeInProgressData.model_validate(data)) + + +def _build_merge_recovery_required(data: Mapping[str, Any]) -> GraphQLError: + return MergeRecoveryRequiredError.from_payload(MergeRecoveryRequiredData.model_validate(data)) + + +def _build_node_not_found(data: Mapping[str, Any]) -> GraphQLError: + return NodeNotFoundError.from_payload(NodeNotFoundData.model_validate(data)) + + +def _build_schema_not_found(data: Mapping[str, Any]) -> GraphQLError: + return SchemaNotFoundError.from_payload(SchemaNotFoundData.model_validate(data)) + + +def _build_undefined_error(data: Mapping[str, Any]) -> GraphQLError: + return UndefinedError.from_payload(UndefinedErrorData.model_validate(data)) + + +def _build_uniqueness_violation(data: Mapping[str, Any]) -> GraphQLError: + return UniquenessViolationError.from_payload(UniquenessViolationData.model_validate(data)) + + +# Each builder validates against its own code's model, so the payload type is never widened on the +# way to the constructor that consumes it. +_CODE_TO_BUILDER: dict[str, Callable[[Mapping[str, Any]], GraphQLError]] = { + "ATTRIBUTE_CONSTRAINT_VIOLATION": _build_attribute_constraint_violation, + "ATTRIBUTE_INVALID_TYPE": _build_attribute_invalid_type, + "ATTRIBUTE_REQUIRED": _build_attribute_required, + "BRANCH_ALREADY_MERGED": _build_branch_already_merged, + "BRANCH_NEEDS_REBASE": _build_branch_needs_rebase, + "BRANCH_NOT_FOUND": _build_branch_not_found, + "MERGE_IN_PROGRESS": _build_merge_in_progress, + "MERGE_RECOVERY_REQUIRED": _build_merge_recovery_required, + "NODE_NOT_FOUND": _build_node_not_found, + "SCHEMA_NOT_FOUND": _build_schema_not_found, + "UNDEFINED_ERROR": _build_undefined_error, + "UNIQUENESS_VIOLATION": _build_uniqueness_violation, +} + + +def exception_from_payload(code: str, data: Mapping[str, Any]) -> GraphQLError | None: + """Build the exception a catalogued code names, or None where the SDK has no class for it. + + Raises: + pydantic.ValidationError: when the payload does not match what the code declares. + + """ + builder = _CODE_TO_BUILDER.get(code) + return None if builder is None else builder(data) From c1cb9484da5307c4379f846cb7798b3d3583dfba Mon Sep 17 00:00:00 2001 From: Patrick Ogenstad Date: Fri, 18 Sep 2026 16:51:45 +0200 Subject: [PATCH 2/3] feat: re-export the generated exception classes from the package infrahub_sdk.exceptions is the supported import path, so the nine classes the catalogue generates belong on it. They are listed by name rather than star-imported: a wildcard would also promote fifteen payload models, both lookup maps and the dispatch helper to the package surface, where a name is a stability promise. Those stay importable from the catalogue module, which is where the factory wants them. The layering test learns that the generated module sits above base and below factory, and the public-names snapshot gains the nine classes, so the surface stays a deliberate choice rather than a side effect. --- infrahub_sdk/exceptions/__init__.py | 31 ++++++++++++++--- .../error_catalogue/public_names.json | 9 +++++ tests/unit/sdk/test_exceptions_layering.py | 8 +++-- .../unit/sdk/test_exceptions_public_names.py | 33 +++++++++++++++---- 4 files changed, 68 insertions(+), 13 deletions(-) diff --git a/infrahub_sdk/exceptions/__init__.py b/infrahub_sdk/exceptions/__init__.py index 94b56529..a958fbbd 100644 --- a/infrahub_sdk/exceptions/__init__.py +++ b/infrahub_sdk/exceptions/__init__.py @@ -1,9 +1,12 @@ """The supported import path for every SDK exception. -The class list below is written out rather than star-imported, so reading this file tells you what -the package exports. It has to be kept in step with `base.__all__` by hand; the tests in -`tests/unit/sdk/test_exceptions_public_names.py` fail if the two drift apart, or if a class defined -in `base` is left out of either. +The class lists below are written out rather than star-imported, so reading this file tells you what +the package exports. They have to be kept in step by hand with `base.__all__` and with the exception +classes `catalogue` generates; the tests in `tests/unit/sdk/test_exceptions_public_names.py` fail if +they drift apart, or if a class defined in `base` is left out of either. + +Only `catalogue`'s exception classes are re-exported. Its payload models, its lookup maps and its +dispatch helper are the factory's business and stay importable from the module itself. `__all__` is what `import *` hands a caller: the exception classes and nothing else. Without it the wildcard also carries the `base` and `factory` submodule names, which are an artefact of the layout. @@ -48,12 +51,28 @@ VersionNotSupportedError, ) from .base import code_names_the_failure as code_names_the_failure +from .catalogue import ( + AttributeConstraintViolationError, + AttributeInvalidTypeError, + AttributeRequiredError, + BranchAlreadyMergedError, + BranchNeedsRebaseError, + MergeInProgressError, + MergeRecoveryRequiredError, + UndefinedError, + UniquenessViolationError, +) from .factory import authentication_error_from_response as authentication_error_from_response from .factory import graphql_error_from_response as graphql_error_from_response __all__ = [ "ApiError", + "AttributeConstraintViolationError", + "AttributeInvalidTypeError", + "AttributeRequiredError", "AuthenticationError", + "BranchAlreadyMergedError", + "BranchNeedsRebaseError", "BranchNotFoundError", "CircularFragmentError", "DuplicateFragmentError", @@ -68,6 +87,8 @@ "InfrahubTransformNotFoundError", "InvalidResponseError", "JsonDecodeError", + "MergeInProgressError", + "MergeRecoveryRequiredError", "ModuleImportError", "NodeInvalidError", "NodeNotFoundError", @@ -82,7 +103,9 @@ "ServerNotResponsiveError", "TimestampFormatError", "URLNotFoundError", + "UndefinedError", "UninitializedError", + "UniquenessViolationError", "ValidationError", "VersionNotSupportedError", ] diff --git a/tests/fixtures/error_catalogue/public_names.json b/tests/fixtures/error_catalogue/public_names.json index 79a80903..172c49f5 100644 --- a/tests/fixtures/error_catalogue/public_names.json +++ b/tests/fixtures/error_catalogue/public_names.json @@ -1,6 +1,11 @@ [ "ApiError", + "AttributeConstraintViolationError", + "AttributeInvalidTypeError", + "AttributeRequiredError", "AuthenticationError", + "BranchAlreadyMergedError", + "BranchNeedsRebaseError", "BranchNotFoundError", "CircularFragmentError", "DuplicateFragmentError", @@ -15,6 +20,8 @@ "InfrahubTransformNotFoundError", "InvalidResponseError", "JsonDecodeError", + "MergeInProgressError", + "MergeRecoveryRequiredError", "ModuleImportError", "NodeInvalidError", "NodeNotFoundError", @@ -29,7 +36,9 @@ "ServerNotResponsiveError", "TimestampFormatError", "URLNotFoundError", + "UndefinedError", "UninitializedError", + "UniquenessViolationError", "ValidationError", "VersionNotSupportedError" ] diff --git a/tests/unit/sdk/test_exceptions_layering.py b/tests/unit/sdk/test_exceptions_layering.py index e6f71bb4..e7689755 100644 --- a/tests/unit/sdk/test_exceptions_layering.py +++ b/tests/unit/sdk/test_exceptions_layering.py @@ -18,11 +18,13 @@ PACKAGE_DIR = Path(exceptions_package.__file__).parent # base.py sits at the bottom, which is what keeps the hand-written hierarchy independent of anything -# built on top of it. Each layer above may import only from below it. +# built on top of it. Each layer above may import only from below it. The generated catalogue sits +# above base and below factory, so the factory can resolve a code to one of its classes. LAYERS = { "base": 0, - "factory": 1, - "__init__": 2, + "catalogue": 1, + "factory": 2, + "__init__": 3, } diff --git a/tests/unit/sdk/test_exceptions_public_names.py b/tests/unit/sdk/test_exceptions_public_names.py index 5d100e19..c94a4190 100644 --- a/tests/unit/sdk/test_exceptions_public_names.py +++ b/tests/unit/sdk/test_exceptions_public_names.py @@ -3,7 +3,7 @@ import json from infrahub_sdk import exceptions -from infrahub_sdk.exceptions import authentication_error_from_response, base, graphql_error_from_response +from infrahub_sdk.exceptions import authentication_error_from_response, base, catalogue, graphql_error_from_response from tests.helpers.fixtures import read_fixture @@ -36,13 +36,34 @@ def classes_defined_in(module: object) -> set[str]: } -def test_the_facade_lists_every_class_base_declares() -> None: - """The façade writes its exports out by hand, so nothing may drift out of step with `base`. +def catalogue_exception_names() -> set[str]: + """The exception classes the generated module declares, without its payload models or maps.""" + return { + name + for name in catalogue.__all__ + if isinstance(getattr(catalogue, name), type) and issubclass(getattr(catalogue, name), BaseException) + } + + +def test_the_facade_lists_every_class_base_and_catalogue_declare() -> None: + """The façade writes its exports out by hand, so nothing may drift out of step with its sources. - A class added to `base.__all__` has to be added to both lists in - `infrahub_sdk/exceptions/__init__.py`: the import block and `__all__`. + A class added to `base.__all__`, or a new code added to the catalogue, has to be added to both + lists in `infrahub_sdk/exceptions/__init__.py`: the import block and `__all__`. """ - assert set(exceptions.__all__) == set(base.__all__) + assert set(exceptions.__all__) == set(base.__all__) | catalogue_exception_names() + + +def test_the_facade_re_exports_no_catalogue_payload_model_or_lookup() -> None: + """Only the classes a caller catches are promoted to the package surface. + + The payload models, the two lookup maps and the dispatch helper are the factory's business, so + they stay importable from `catalogue` rather than becoming a stability promise of the package. + """ + non_classes = set(catalogue.__all__) - catalogue_exception_names() + leaked = sorted(non_classes & set(exceptions.__all__)) + + assert leaked == [], f"re-exported from catalogue but not an exception class: {leaked}" def test_every_name_the_facade_declares_is_bound() -> None: From d1d10c690fd69d55f51a1019b7774de2d096c4b0 Mon Sep 17 00:00:00 2001 From: Patrick Ogenstad Date: Fri, 18 Sep 2026 17:46:10 +0200 Subject: [PATCH 3/3] feat: raise a typed exception per catalogue code The GraphQL factory now resolves a catalogue code through the generated bindings' own dispatch, which validates the payload against that code's model and hands it to the class's `from_payload`. The hand-written stand-in payload dataclasses and the three-way branch that read them go away with it, so the factory assembles no attribute itself and every catalogued code - not only the three adopted ones - reaches a class carrying its payload as typed attributes. A payload that violates what the catalogue declares falls back to the generic class for the transport, with the code still readable and the raw extensions retained. Which generic class it falls back to follows the transport the SDK observed, never the status the code declares: the GraphQL branch for anything read from an `errors` array, the authentication branch only for a response the SDK saw as 401 or 403. That is the only rule under which the three authentication codes, which deliberately have no class of their own, reach the class an existing `except` clause expects. A declared HTTP status is now assigned only when the envelope carried one, so a generated class keeps the status the catalogue gave it. A class built from its payload alone carries the GraphQL placeholder message; where the failure was not described, the factory replaces that with the message the call site has always produced rather than leaving a sentence naming no query and no errors. Tests cover every code in the bindings, checked against the bindings' own code list so the set cannot fall behind: the class raised, each promoted attribute's value, and the code and status, with no case reading a message. Both clients are driven over both transports and the file-upload path, and two integration cases prove the envelope against a real server. The uniqueness case skips where the server's catalogue has no entry for that failure yet. --- changelog/+typed-per-code-exceptions.added.md | 20 ++ dev/specs/ifc-3034-error-catalogue/tasks.md | 24 +- .../docs/python-sdk/topics/error_handling.mdx | 184 +++++++++++ infrahub_sdk/exceptions/base.py | 12 +- infrahub_sdk/exceptions/factory.py | 126 ++++---- pyproject.toml | 1 + tests/fixtures/error_catalogue/README.md | 7 + .../codes/attribute_constraint_violation.json | 27 ++ .../codes/attribute_invalid_type.json | 27 ++ .../codes/attribute_required.json | 25 ++ .../codes/authentication_required.json | 22 ++ .../codes/branch_already_merged.json | 24 ++ .../codes/branch_needs_rebase.json | 24 ++ .../codes/branch_not_found.json | 21 ++ .../codes/merge_in_progress.json | 25 ++ .../codes/merge_recovery_required.json | 25 ++ .../error_catalogue/codes/node_not_found.json | 25 ++ .../codes/permission_denied.json | 25 ++ .../codes/schema_not_found.json | 21 ++ .../error_catalogue/codes/token_expired.json | 21 ++ .../codes/undefined_error.json | 19 ++ .../codes/uniqueness_violation.json | 27 ++ tests/integration/test_infrahub_client.py | 43 ++- .../integration/test_infrahub_client_sync.py | 43 ++- tests/unit/sdk/test_client.py | 95 +++++- tests/unit/sdk/test_error_catalogue.py | 306 +++++++++++++++++- 26 files changed, 1127 insertions(+), 92 deletions(-) create mode 100644 changelog/+typed-per-code-exceptions.added.md create mode 100644 docs/docs/python-sdk/topics/error_handling.mdx create mode 100644 tests/fixtures/error_catalogue/codes/attribute_constraint_violation.json create mode 100644 tests/fixtures/error_catalogue/codes/attribute_invalid_type.json create mode 100644 tests/fixtures/error_catalogue/codes/attribute_required.json create mode 100644 tests/fixtures/error_catalogue/codes/authentication_required.json create mode 100644 tests/fixtures/error_catalogue/codes/branch_already_merged.json create mode 100644 tests/fixtures/error_catalogue/codes/branch_needs_rebase.json create mode 100644 tests/fixtures/error_catalogue/codes/branch_not_found.json create mode 100644 tests/fixtures/error_catalogue/codes/merge_in_progress.json create mode 100644 tests/fixtures/error_catalogue/codes/merge_recovery_required.json create mode 100644 tests/fixtures/error_catalogue/codes/node_not_found.json create mode 100644 tests/fixtures/error_catalogue/codes/permission_denied.json create mode 100644 tests/fixtures/error_catalogue/codes/schema_not_found.json create mode 100644 tests/fixtures/error_catalogue/codes/token_expired.json create mode 100644 tests/fixtures/error_catalogue/codes/undefined_error.json create mode 100644 tests/fixtures/error_catalogue/codes/uniqueness_violation.json diff --git a/changelog/+typed-per-code-exceptions.added.md b/changelog/+typed-per-code-exceptions.added.md new file mode 100644 index 00000000..22994adb --- /dev/null +++ b/changelog/+typed-per-code-exceptions.added.md @@ -0,0 +1,20 @@ +Every code in Infrahub's error catalogue now has an exception class of its own, importable from `infrahub_sdk.exceptions`, carrying the failure's payload as directly typed attributes. Identifying a specific failure no longer means matching words in a message: + +```python +from infrahub_sdk.exceptions import ApiError, UniquenessViolationError + +try: + await node.save() +except UniquenessViolationError as exc: + print(exc.node_kind, exc.fields) # "TestPerson", ["name"] +except ApiError as exc: + print("some other failure:", exc.code) +``` + +The new classes are `AttributeConstraintViolationError`, `AttributeInvalidTypeError`, `AttributeRequiredError`, `BranchAlreadyMergedError`, `BranchNeedsRebaseError`, `MergeInProgressError`, `MergeRecoveryRequiredError`, `UndefinedError`, and `UniquenessViolationError`. Each is typed exactly as the catalogue declares the payload, so a required field is never optional and needs no guard. They all descend from `GraphQLError`, so no `except` clause stops catching what it catches today. + +`AUTHENTICATION_REQUIRED`, `TOKEN_EXPIRED`, and `PERMISSION_DENIED` deliberately have no class of their own: each of them reaches the SDK on two transports, and which generic class it raises follows the transport the SDK observed rather than the status the code declares. Catch `ApiError` and test `exc.code` to handle one of the three whichever way it arrived. + +A server predating a code, or one whose payload does not match what the catalogue declares for it, still raises the generic class for the transport with `exc.code` readable, so an SDK of any version keeps working against a server of any version. + +`docs/python-sdk/topics/error_handling` covers the hierarchy, the attributes readable on a caught error, and the cross-version guarantees. diff --git a/dev/specs/ifc-3034-error-catalogue/tasks.md b/dev/specs/ifc-3034-error-catalogue/tasks.md index 2ed8af50..1a36d01d 100644 --- a/dev/specs/ifc-3034-error-catalogue/tasks.md +++ b/dev/specs/ifc-3034-error-catalogue/tasks.md @@ -450,39 +450,39 @@ the raised type and the typed attributes, reading no message (quickstart scenari ### Tests for User Story 1 -- [ ] T066 [P] [US1] Add one response-envelope fixture per catalogue code under +- [X] T066 [P] [US1] Add one response-envelope fixture per catalogue code under `tests/fixtures/error_catalogue/`, each a verbatim server response rather than a hand-shaped dict. -- [ ] T067 [US1] Add the exhaustive factory cases to `tests/unit/sdk/test_error_catalogue.py`: one per +- [X] T067 [US1] Add the exhaustive factory cases to `tests/unit/sdk/test_error_catalogue.py`: one per code, asserting the raised class, every promoted attribute's concrete value, and that `exc.code` and `exc.http_status` match the catalogue entry. No case reads a payload object, because there is none. -- [ ] T068 [P] [US1] Add the adopted-class cases to `tests/unit/sdk/test_error_catalogue.py`: a +- [X] T068 [P] [US1] Add the adopted-class cases to `tests/unit/sdk/test_error_catalogue.py`: a server-reported `NODE_NOT_FOUND` populates `node_type` and `identifier`, `BRANCH_NOT_FOUND` and `SCHEMA_NOT_FOUND` populate `identifier`, and `exc.code is not None` distinguishes a server-reported raise from a client-side one. -- [ ] T069 [P] [US1] Add the representative parity set to `tests/unit/sdk/test_client.py`, parametrized +- [X] T069 [P] [US1] Add the representative parity set to `tests/unit/sdk/test_client.py`, parametrized over `["standard", "sync"]` via the `BothClients` fixture, covering both branches, both transports, and the file-upload variant, asserting the same class and the same attributes on each. -- [ ] T070 [P] [US1] Add a `catalogue`-marked case to `tests/integration/test_infrahub_client.py`: saving a +- [X] T070 [P] [US1] Add a `catalogue`-marked case to `tests/integration/test_infrahub_client.py`: saving a node that collides on a unique attribute raises `UniquenessViolationError` with the node kind and colliding fields from the real payload, and deleting a missing node raises `NodeNotFoundError` with its kind and identifier. -- [ ] T071 [P] [US1] Add the same two `catalogue`-marked cases to +- [X] T071 [P] [US1] Add the same two `catalogue`-marked cases to `tests/integration/test_infrahub_client_sync.py`. ### Implementation for User Story 1 -- [ ] T072 [US1] Extend `graphql_error_from_response` in `infrahub_sdk/exceptions/factory.py` to look the +- [X] T072 [US1] Extend `graphql_error_from_response` in `infrahub_sdk/exceptions/factory.py` to look the first error's code up in `CODE_TO_EXCEPTION`, validate `extensions.data` with the class's `DATA_MODEL`, and raise via `cls.from_payload(...)`. The factory never assembles attributes itself. -- [ ] T073 [US1] Implement the validation-failure fallback in `infrahub_sdk/exceptions/factory.py`: an +- [X] T073 [US1] Implement the validation-failure fallback in `infrahub_sdk/exceptions/factory.py`: an invalid payload falls back to the generic class for the observed transport with `exc.code` still readable, the raw `extensions` retained, and a debug log. A pydantic `ValidationError` never escapes a raise path. -- [ ] T074 [US1] Confirm in `infrahub_sdk/exceptions/factory.py` that the fallback follows **the transport +- [X] T074 [US1] Confirm in `infrahub_sdk/exceptions/factory.py` that the fallback follows **the transport the SDK observed** and never the code's declared status: the GraphQL branch for anything read from an `errors` array, the authentication branch only for a response the SDK saw as HTTP 401 or 403. This is the only rule under which the three authentication codes reach the right class at all. -- [ ] T075 [US1] Add a test to `tests/unit/sdk/test_error_catalogue.py` asserting the first error governs +- [X] T075 [US1] Add a test to `tests/unit/sdk/test_error_catalogue.py` asserting the first error governs even when it carries no code and a later one does, and that the complete list is retained unreordered. **Checkpoint**: Every catalogue code is identifiable without reading a message, on both clients. @@ -491,12 +491,12 @@ the raised type and the typed attributes, reading no message (quickstart scenari ## Phase 9: Polish & Cross-Cutting Concerns -- [ ] T076 [P] Write `docs/docs/python-sdk/topics/error_handling.mdx` covering the hierarchy, catching by +- [X] T076 [P] Write `docs/docs/python-sdk/topics/error_handling.mdx` covering the hierarchy, catching by branch versus by code, the cross-version guarantees, the two accepted broadenings, and the note that `infrahub_sdk.exceptions` is the supported import path. Link to Infrahub's published catalogue for the code list rather than restating it, and note that a catalogued message now names the failing action and resource kind where the catalogue provides them. -- [ ] T077 [P] Add a towncrier fragment for the typed errors in `changelog/`. +- [X] T077 [P] Add a towncrier fragment for the typed errors in `changelog/`. - [X] T078 [P] Add a towncrier fragment for the `NodeNotFoundError.identifier` widening in `changelog/`. **Landed in issue 1**, alongside the widening itself (T036). - [X] T079 [P] Add a towncrier fragment for the `except GraphQLError` broadening in `changelog/`. diff --git a/docs/docs/python-sdk/topics/error_handling.mdx b/docs/docs/python-sdk/topics/error_handling.mdx new file mode 100644 index 00000000..b4db5789 --- /dev/null +++ b/docs/docs/python-sdk/topics/error_handling.mdx @@ -0,0 +1,184 @@ +--- +title: Understanding error handling in the Python SDK +--- + +import Tabs from '@theme/Tabs'; +import TabItem from '@theme/TabItem'; + +# Understanding error handling in the Python SDK + +## Introduction + +Every exception the SDK raises is importable from `infrahub_sdk.exceptions`. That is the supported +import path, and the only one: the modules beneath it are internal and their layout may change. + +```python +from infrahub_sdk.exceptions import ApiError, GraphQLError, UniquenessViolationError +``` + +When Infrahub rejects a request, it describes the failure with a stable code from its +[error catalogue](https://docs.infrahub.app/reference/error-catalogue), an HTTP status, and a typed +payload. The SDK turns that description into an exception class of its own, with the payload's fields +as directly typed attributes, so branching on a specific failure never means matching words in a +message. + +## The hierarchy + +```text +Error every exception the SDK raises +└── ApiError the server rejected the request + ├── AuthenticationError the SDK observed HTTP 401 or 403 + └── GraphQLError a failure read from an `errors` array + ├── NodeNotFoundError + ├── BranchNotFoundError + ├── SchemaNotFoundError + ├── UniquenessViolationError + └── ... one class per catalogued code +``` + +The tree is plain: no class has more than one parent, and `AuthenticationError` and `GraphQLError` +are siblings. Anything the SDK raises without a server behind it - a timeout, an unreadable file, a +malformed query - stays under `Error` and outside `ApiError`. + +## Catching by branch, or by code + +| Intent | Clause | +|--------|--------| +| Anything the server rejected, on either transport | `except ApiError` | +| Any GraphQL-path failure | `except GraphQLError` | +| Any failure the SDK observed as HTTP 401 or 403 | `except AuthenticationError` | +| One specific catalogued failure | `except UniquenessViolationError`, and so on per code | +| Anything the SDK raises | `except Error` | + +Catching the specific class is the shortest route to the payload, because its attributes are typed +exactly as the catalogue declares them and a required field needs no guard: + + + + +```python +from infrahub_sdk.exceptions import ApiError, UniquenessViolationError + +try: + await node.save() +except UniquenessViolationError as exc: + print(exc.node_kind, exc.fields) +except ApiError as exc: + print("some other failure:", exc.code) +``` + + + + +```python +from infrahub_sdk.exceptions import ApiError, UniquenessViolationError + +try: + node.save() +except UniquenessViolationError as exc: + print(exc.node_kind, exc.fields) +except ApiError as exc: + print("some other failure:", exc.code) +``` + + + + +Both clients raise the same class with the same attributes for the same failure. + +### The three authentication codes + +`AUTHENTICATION_REQUIRED`, `TOKEN_EXPIRED`, and `PERMISSION_DENIED` have no class of their own. They +are the only codes that reach the SDK on two different transports, and each transport already has a +class that existing code depends on: + +| Arrival | Class raised | `exc.code` | +|---------|--------------|------------| +| A real 401 or 403, when the failure escapes before the query runs | `AuthenticationError` | the catalogue code | +| Inside an HTTP 200 `errors` array, when a resolver raised it | `GraphQLError` | the catalogue code | + +Any of the three can arrive either way, so the arrival path is a property of how the server happened +to fail rather than of the code. To handle one of them whichever way it arrived, catch `ApiError` and +test the code: + +```python +except ApiError as exc: + if exc.code == "TOKEN_EXPIRED": + ... +``` + +`AuthenticationError` descends from `ApiError`, so an `except ApiError` clause placed first makes any +later `except AuthenticationError` unreachable. + +## Reading a caught error + +These are readable on every `ApiError`, including one raised with no server response behind it, so +inspecting them never needs a guard for a missing attribute: + +| Attribute | Contract | +|-----------|----------| +| `code` | The catalogue code string, or `None`. Never an integer. `None` means no code was resolved: a server predating the catalogue, a REST failure, or an error carrying no `extensions`. | +| `http_status` | The status the failure declares, or `None`. This is metadata about the failure, not the status the transport observed: a catalogued data error arrives as HTTP 200. | +| `extensions` | The raw `extensions` mapping of the governing error, or `None`. | +| `errors` | The complete server error list, in the order the server sent it. Empty for a raise the SDK decided on its own. | +| `query`, `variables` | The GraphQL query and variables where there was one, otherwise `None`. | + +The payload's fields are not on the base class. Each catalogued class carries its own, typed as the +catalogue declares them. `NodeNotFoundError`, `BranchNotFoundError`, and `SchemaNotFoundError` are the +exception: the SDK also raises those three on its own, for a lookup that returned nothing and for the +REST 404 behind a missing file, so their attributes may be unpopulated. Test `exc.code is not None` to +tell a server-reported raise from an SDK one. + +The raw payload stays in `exc.extensions["data"]` for anything that forwards a failure verbatim. + +## Messages + +A failure the catalogue describes carries a message naming the code and the server's own words, with +no query text: + +```text +UNIQUENESS_VIOLATION: Node of kind TestPerson already has name 'John' +``` + +Where the catalogue provides them, those words name the failing action and the resource kind, so that +detail now appears in logs and CLI output in place of the query text that used to be there. The query +itself stays readable on `exc.query`. + +A failure the catalogue does **not** describe keeps the message it has always had, query text and full +error list included. Since a current server codes every error it reports, falling back to +`UNDEFINED_ERROR` where its catalogue has no entry, `exc.code is not None` is not the test for whether +the server described a failure. `code_names_the_failure(exc.code)` is, and it is importable from +`infrahub_sdk.exceptions`. + +Where a response carries several errors, the first one determines the class raised and is the only one +named beside the code. The complete list stays on `exc.errors`, in the order the server sent it. + +## Talking to any server version + +Any SDK version talks to any server version, and parsing a response never raises. + +| Situation | Behaviour | +|-----------|-----------| +| A code this SDK has a class for | That class, built from the payload the response carried | +| A code this SDK has never heard of | `GraphQLError`, or `AuthenticationError` on a 401 or 403, with `exc.code` set to the string the server sent | +| A known code whose payload gained a field | The unknown field is ignored | +| A server predating the catalogue, or an error with no `extensions` | `exc.code` is `None`, and the message is the one that version of the SDK has always produced | +| A payload that does not match what the catalogue declares | The generic class for the transport, with the code still readable | + +Every fallback is logged at debug level on the `infrahub_sdk` logger with the code involved, so an SDK +meeting a newer server is diagnosable without a debugger. + +Which generic class a fallback lands on follows the transport the SDK observed, never the status the +code declares. A code read from an `errors` array raises `GraphQLError` even when it declares 401. + +## Two clauses that now catch more + +Existing `except` clauses keep catching everything they caught before. Two of them now catch more. + +`except GraphQLError` also catches node, branch, and schema lookup misses that involved no GraphQL +request at all, because those three classes are re-rooted under it. Code that relied on them escaping +such a clause should catch the specific class ahead of it, as an ordered `except` ladder already must. + +A ladder that handles one of those three specifically now sees server-reported failures arrive there +as well as the ones the SDK decides on its own. That is the point of binding a code to a class, and `exc.code is not +None` separates the two. diff --git a/infrahub_sdk/exceptions/base.py b/infrahub_sdk/exceptions/base.py index 34c220c8..ff6e22e8 100644 --- a/infrahub_sdk/exceptions/base.py +++ b/infrahub_sdk/exceptions/base.py @@ -139,6 +139,16 @@ class ApiError(Error): errors: Sequence[dict[str, Any]] = () +def graphql_default_message(query: str | None, errors: Any) -> str: + """The message the GraphQL path produces where the server described nothing better. + + Lives beside the class rather than inside it because a class built from a payload alone carries + this text with neither the query nor the errors in it, and the factory has to recognise that and + fill in the envelope the class never saw. + """ + return f"An error occurred while executing the GraphQL Query {query}, {errors}" + + class GraphQLError(ApiError): query: str | None = None variables: dict | None = None @@ -154,7 +164,7 @@ def __init__( self.variables = variables # `is not None` rather than `or`: an empty message is a deliberate one, not a request for # the default. - default = f"An error occurred while executing the GraphQL Query {query}, {errors}" + default = graphql_default_message(query=query, errors=errors) self.message = message if message is not None else default self.errors = as_error_list(errors) super().__init__(self.message) diff --git a/infrahub_sdk/exceptions/factory.py b/infrahub_sdk/exceptions/factory.py index ec12901d..9b18520d 100644 --- a/infrahub_sdk/exceptions/factory.py +++ b/infrahub_sdk/exceptions/factory.py @@ -9,19 +9,20 @@ from __future__ import annotations import logging -from dataclasses import dataclass +from collections.abc import Mapping from typing import TYPE_CHECKING, Any +from pydantic import ValidationError as PayloadValidationError + from .base import ( AuthenticationError, - BranchNotFoundError, Error, GraphQLError, - NodeNotFoundError, - SchemaNotFoundError, as_error_list, code_names_the_failure, + graphql_default_message, ) +from .catalogue import exception_from_payload if TYPE_CHECKING: import httpx @@ -32,6 +33,11 @@ # the SDK, which imports it from this module, but it is not part of the published exception surface. __all__ = ["authentication_error_from_response", "graphql_error_from_response"] +# What a catalogued class carries when it has built itself from its payload alone: the GraphQL +# default, naming neither a query nor any errors because `from_payload` is given neither. A class +# still holding it is one with no sentence of its own, and the factory fills in the real envelope. +_MESSAGE_OF_AN_UNBUILT_ENVELOPE = graphql_default_message(query=None, errors=[]) + def _extensions_of(error: Any) -> dict[str, Any] | None: if not isinstance(error, dict): @@ -115,77 +121,37 @@ def _replace_message(exc: Error, message: str) -> None: exc.args = (message,) -@dataclass -class _NodeNotFoundData: - """Stands in for the generated payload model, which the SDK does not carry yet. - - Plain rather than frozen, because the payload protocols declare settable attributes: the shape - the generated pydantic models will have. - """ - - node_kind: str - identifier: str - - -@dataclass -class _BranchNotFoundData: - branch_name: str - - -@dataclass -class _SchemaNotFoundData: - kind: str - - -def _payload_strings(data: Any, names: tuple[str, ...]) -> dict[str, str] | None: - """The named payload fields, when every one of them is present as a string. +def _payload_of(extensions: dict[str, Any] | None) -> Mapping[str, Any]: + """The governing error's payload, or an empty one where the envelope carried none. - A payload that violates the catalogue's own contract yields `None` so the caller falls back to - the generic class, rather than a TypeError raised from inside the SDK while the caller is - already failing. + An absent payload is not the same as a malformed one: a code whose fields are all optional still + resolves to its class, while one with required fields fails the validation below and falls back. """ - if not isinstance(data, dict): - return None - values = {name: data.get(name) for name in names} - if any(not isinstance(value, str) for value in values.values()): - return None - return {name: value for name, value in values.items() if isinstance(value, str)} + data = extensions.get("data") if extensions is not None else None + return data if isinstance(data, Mapping) else {} -def _adopted_exception(code: str | None, extensions: dict[str, Any] | None) -> GraphQLError | None: - """The class that adopted `code`, built from the payload the envelope carries. +def _catalogued_exception(code: str | None, extensions: dict[str, Any] | None) -> GraphQLError | None: + """The class the catalogue binds `code` to, built from the payload the envelope carries. - Only the three codes the SDK already ships a class for are resolved here, and each class maps the - payload itself through its own `from_payload`. Every other code raises the generic class for the - transport with the code readable on `exc.code`; turning the rest into classes of their own is - what the generated bindings buy. + Resolution and validation both belong to the generated bindings: each code's payload is validated + against its own model and handed to that class's `from_payload`, so the factory never assembles + an attribute itself and never has to widen a payload type to reach a constructor. - `None` means no class was resolved, whether because the code has none or because its payload did - not carry the fields the class needs. + `None` means no class was resolved - the code has none, or its payload violates what the + catalogue declares for it - and the caller then raises the generic class for the transport it + observed, with the code still readable. """ if code is None: return None - data = extensions.get("data") if extensions is not None else None - - if code == NodeNotFoundError.CODE: - node = _payload_strings(data=data, names=("node_kind", "identifier")) - if node is not None: - payload = _NodeNotFoundData(node_kind=node["node_kind"], identifier=node["identifier"]) - return NodeNotFoundError.from_payload(payload=payload) - elif code == BranchNotFoundError.CODE: - branch = _payload_strings(data=data, names=("branch_name",)) - if branch is not None: - return BranchNotFoundError.from_payload(payload=_BranchNotFoundData(branch_name=branch["branch_name"])) - elif code == SchemaNotFoundError.CODE: - schema = _payload_strings(data=data, names=("kind",)) - if schema is not None: - return SchemaNotFoundError.from_payload(payload=_SchemaNotFoundData(kind=schema["kind"])) - else: + try: + return exception_from_payload(code=code, data=_payload_of(extensions)) + except PayloadValidationError: + # The caller is already failing, so a validation error from inside the SDK would replace the + # server's reason with one of the SDK's own. + LOGGER.debug("Payload for %s does not match what the catalogue declares: %r", code, extensions) return None - LOGGER.debug("Payload for %s does not carry the fields its class needs: %r", code, data) - return None - def _log_unresolved_code(extensions: dict[str, Any] | None, source: str) -> None: if extensions is not None and _catalogue_code(extensions) is None: @@ -215,20 +181,27 @@ def graphql_error_from_response( """Build the exception for an `errors` array returned on the GraphQL path. `errors` is raw decoded JSON, so it is read defensively. The complete list is retained - unreordered. A code the SDK has adopted a class for raises that class; every other failure raises + unreordered. A code the catalogue binds to a class raises that class; every other failure raises `GraphQLError`, and one the server's catalogue could not describe keeps the message this call site has always produced, query text included. + + This is the GraphQL branch, so its fallback is `GraphQLError` whatever status the code declares. + A code that reaches an `errors` array was read off this transport, and a declared 401 does not + make it an authentication failure the SDK observed. """ extensions = _first_extensions(errors) code = _catalogue_code(extensions) governing = _governing_message(errors) message = _named_by_code(code, governing) if code_names_the_failure(code) and governing else None - adopted = _adopted_exception(code=code, extensions=extensions) - if adopted is not None: - exc: GraphQLError = adopted - # An adopted class builds itself from its payload alone, so the envelope it came out of is - # attached here. A silent governing error leaves `message` None, and the class its own text. + catalogued = _catalogued_exception(code=code, extensions=extensions) + if catalogued is not None: + exc: GraphQLError = catalogued + # A catalogued class builds itself from its payload alone, so the envelope it came out of is + # attached here. Where the failure was not described, a class with a sentence of its own + # keeps it and a generated one takes the message this call site has always produced. + if message is None and exc.message == _MESSAGE_OF_AN_UNBUILT_ENVELOPE: + message = graphql_default_message(query=query, errors=errors) if message is not None: _replace_message(exc, message) exc.query = query @@ -238,7 +211,11 @@ def graphql_error_from_response( exc.errors = as_error_list(errors) exc.code = code - exc.http_status = _declared_http_status(extensions) + declared_status = _declared_http_status(extensions) + if declared_status is not None: + # Assigned only when the envelope declared one, so a generated class keeps the status the + # catalogue gave it rather than losing it to an envelope that omitted it. + exc.http_status = declared_status exc.extensions = extensions _log_unresolved_code(extensions=extensions, source="GraphQL") return exc @@ -256,6 +233,13 @@ def authentication_error_from_response(response: httpx.Response) -> Authenticati A described failure names its code and the governing error's message instead, on the same rule as the GraphQL path: only the error the code came from may be named beside it, and the complete list stays on `exc.errors`. + + No code is resolved to a class here. This branch is reached because the SDK observed the response + as an authentication failure, and every catalogued class descends from `GraphQLError`, so raising + one would put a failure that arrived on this transport out of reach of `except + AuthenticationError`. The three authentication codes have no class of their own for the same + reason - each of them can arrive either way - and they reach the right class only because both + branches follow the transport rather than the status the code declares. """ errors: Any = [] message = f"HTTP {response.status_code}" diff --git a/pyproject.toml b/pyproject.toml index 74978bf8..849072cf 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -137,6 +137,7 @@ filterwarnings = [ "ignore:Deprecated call to", ] markers = [ + "catalogue: Drives a catalogued failure through a real server, proving the envelope shape", "crossversion: Parses an envelope from a server whose version differs from the SDK's", "malformed: Parses an envelope that violates the shape the SDK expects", "message: Pins an exception's message text, in either the catalogued or the uncatalogued direction", diff --git a/tests/fixtures/error_catalogue/README.md b/tests/fixtures/error_catalogue/README.md index 43c051e0..68c88ed5 100644 --- a/tests/fixtures/error_catalogue/README.md +++ b/tests/fixtures/error_catalogue/README.md @@ -21,6 +21,13 @@ The `malformed_*` files are the exception: they are constructed, because a corre produce them. They stand in for a proxy, a gateway, or a future server that answers in a shape the SDK does not expect, and they exist to pin that such a shape degrades rather than raising. +`codes/` holds one envelope per catalogue code, named after the code in lower case, each carrying the +payload that code declares. They are the exhaustive set: a code without a file here is a code nothing +proves the SDK can resolve, which is why the test that reads them iterates the catalogue's own code +list rather than the directory. All of them are shaped as a GraphQL response, including the three +authentication codes, which reach that transport whenever a resolver rather than the request pipeline +raised them. + `public_names.json` is not an envelope. It is the committed snapshot of every exception class importable from `infrahub_sdk.exceptions`, which pins that restructuring the module into a package stays invisible from outside. It lists exception classes only; incidental typing imports that the diff --git a/tests/fixtures/error_catalogue/codes/attribute_constraint_violation.json b/tests/fixtures/error_catalogue/codes/attribute_constraint_violation.json new file mode 100644 index 00000000..c8eb60a4 --- /dev/null +++ b/tests/fixtures/error_catalogue/codes/attribute_constraint_violation.json @@ -0,0 +1,27 @@ +{ + "data": null, + "errors": [ + { + "message": "Value for TestPerson.name does not conform to the regex '^[A-Z]'", + "locations": [ + { + "line": 2, + "column": 3 + } + ], + "path": [ + "TestPersonCreate" + ], + "extensions": { + "code": "ATTRIBUTE_CONSTRAINT_VIOLATION", + "http_status": 422, + "data": { + "node_kind": "TestPerson", + "field_name": "name", + "constraint": "regex", + "detail": "^[A-Z]" + } + } + } + ] +} diff --git a/tests/fixtures/error_catalogue/codes/attribute_invalid_type.json b/tests/fixtures/error_catalogue/codes/attribute_invalid_type.json new file mode 100644 index 00000000..ffe04011 --- /dev/null +++ b/tests/fixtures/error_catalogue/codes/attribute_invalid_type.json @@ -0,0 +1,27 @@ +{ + "data": null, + "errors": [ + { + "message": "Value for TestPerson.height must be of type Integer, received String", + "locations": [ + { + "line": 2, + "column": 3 + } + ], + "path": [ + "TestPersonCreate" + ], + "extensions": { + "code": "ATTRIBUTE_INVALID_TYPE", + "http_status": 422, + "data": { + "node_kind": "TestPerson", + "field_name": "height", + "expected_type": "Integer", + "received_type": "String" + } + } + } + ] +} diff --git a/tests/fixtures/error_catalogue/codes/attribute_required.json b/tests/fixtures/error_catalogue/codes/attribute_required.json new file mode 100644 index 00000000..20c5e005 --- /dev/null +++ b/tests/fixtures/error_catalogue/codes/attribute_required.json @@ -0,0 +1,25 @@ +{ + "data": null, + "errors": [ + { + "message": "A value is required for TestPerson.name", + "locations": [ + { + "line": 2, + "column": 3 + } + ], + "path": [ + "TestPersonCreate" + ], + "extensions": { + "code": "ATTRIBUTE_REQUIRED", + "http_status": 422, + "data": { + "node_kind": "TestPerson", + "field_name": "name" + } + } + } + ] +} diff --git a/tests/fixtures/error_catalogue/codes/authentication_required.json b/tests/fixtures/error_catalogue/codes/authentication_required.json new file mode 100644 index 00000000..c49b7920 --- /dev/null +++ b/tests/fixtures/error_catalogue/codes/authentication_required.json @@ -0,0 +1,22 @@ +{ + "data": null, + "errors": [ + { + "message": "Authentication is required to perform this operation", + "locations": [ + { + "line": 2, + "column": 3 + } + ], + "path": [ + "TestPersonCreate" + ], + "extensions": { + "code": "AUTHENTICATION_REQUIRED", + "http_status": 401, + "data": {} + } + } + ] +} diff --git a/tests/fixtures/error_catalogue/codes/branch_already_merged.json b/tests/fixtures/error_catalogue/codes/branch_already_merged.json new file mode 100644 index 00000000..11a4ba7f --- /dev/null +++ b/tests/fixtures/error_catalogue/codes/branch_already_merged.json @@ -0,0 +1,24 @@ +{ + "data": null, + "errors": [ + { + "message": "Branch: feature-a has already been merged and is read-only.", + "locations": [ + { + "line": 2, + "column": 3 + } + ], + "path": [ + "TestPersonCreate" + ], + "extensions": { + "code": "BRANCH_ALREADY_MERGED", + "http_status": 400, + "data": { + "branch_name": "feature-a" + } + } + } + ] +} diff --git a/tests/fixtures/error_catalogue/codes/branch_needs_rebase.json b/tests/fixtures/error_catalogue/codes/branch_needs_rebase.json new file mode 100644 index 00000000..7b1268ba --- /dev/null +++ b/tests/fixtures/error_catalogue/codes/branch_needs_rebase.json @@ -0,0 +1,24 @@ +{ + "data": null, + "errors": [ + { + "message": "Branch: feature-a must be rebased before it accepts new changes.", + "locations": [ + { + "line": 2, + "column": 3 + } + ], + "path": [ + "TestPersonCreate" + ], + "extensions": { + "code": "BRANCH_NEEDS_REBASE", + "http_status": 400, + "data": { + "branch_name": "feature-a" + } + } + } + ] +} diff --git a/tests/fixtures/error_catalogue/codes/branch_not_found.json b/tests/fixtures/error_catalogue/codes/branch_not_found.json new file mode 100644 index 00000000..c88dfbab --- /dev/null +++ b/tests/fixtures/error_catalogue/codes/branch_not_found.json @@ -0,0 +1,21 @@ +{ + "data": null, + "errors": [ + { + "message": "Branch: does-not-exist not found.", + "locations": [ + { + "line": 2, + "column": 3 + } + ], + "extensions": { + "code": "BRANCH_NOT_FOUND", + "http_status": 400, + "data": { + "branch_name": "does-not-exist" + } + } + } + ] +} diff --git a/tests/fixtures/error_catalogue/codes/merge_in_progress.json b/tests/fixtures/error_catalogue/codes/merge_in_progress.json new file mode 100644 index 00000000..3494578d --- /dev/null +++ b/tests/fixtures/error_catalogue/codes/merge_in_progress.json @@ -0,0 +1,25 @@ +{ + "data": null, + "errors": [ + { + "message": "Branch main is locked while feature-a is being merged into it.", + "locations": [ + { + "line": 2, + "column": 3 + } + ], + "path": [ + "TestPersonCreate" + ], + "extensions": { + "code": "MERGE_IN_PROGRESS", + "http_status": 423, + "data": { + "branch_name": "main", + "merging_branch": "feature-a" + } + } + } + ] +} diff --git a/tests/fixtures/error_catalogue/codes/merge_recovery_required.json b/tests/fixtures/error_catalogue/codes/merge_recovery_required.json new file mode 100644 index 00000000..b7bd7e70 --- /dev/null +++ b/tests/fixtures/error_catalogue/codes/merge_recovery_required.json @@ -0,0 +1,25 @@ +{ + "data": null, + "errors": [ + { + "message": "Branch main is protected after a failed merge of feature-a. Run `infrahub recover merge`.", + "locations": [ + { + "line": 2, + "column": 3 + } + ], + "path": [ + "TestPersonCreate" + ], + "extensions": { + "code": "MERGE_RECOVERY_REQUIRED", + "http_status": 423, + "data": { + "branch_name": "main", + "merging_branch": "feature-a" + } + } + } + ] +} diff --git a/tests/fixtures/error_catalogue/codes/node_not_found.json b/tests/fixtures/error_catalogue/codes/node_not_found.json new file mode 100644 index 00000000..b606ed06 --- /dev/null +++ b/tests/fixtures/error_catalogue/codes/node_not_found.json @@ -0,0 +1,25 @@ +{ + "data": null, + "errors": [ + { + "message": "Unable to find the node john / TestPerson in the database.", + "locations": [ + { + "line": 2, + "column": 3 + } + ], + "path": [ + "TestPersonDelete" + ], + "extensions": { + "code": "NODE_NOT_FOUND", + "http_status": 404, + "data": { + "node_kind": "TestPerson", + "identifier": "john" + } + } + } + ] +} diff --git a/tests/fixtures/error_catalogue/codes/permission_denied.json b/tests/fixtures/error_catalogue/codes/permission_denied.json new file mode 100644 index 00000000..b6c1ad7c --- /dev/null +++ b/tests/fixtures/error_catalogue/codes/permission_denied.json @@ -0,0 +1,25 @@ +{ + "data": null, + "errors": [ + { + "message": "The requested operation was not authorized", + "locations": [ + { + "line": 2, + "column": 3 + } + ], + "path": [ + "TestPersonUpdate" + ], + "extensions": { + "code": "PERMISSION_DENIED", + "http_status": 403, + "data": { + "action": "update", + "resource_kind": "TestPerson" + } + } + } + ] +} diff --git a/tests/fixtures/error_catalogue/codes/schema_not_found.json b/tests/fixtures/error_catalogue/codes/schema_not_found.json new file mode 100644 index 00000000..9f265b84 --- /dev/null +++ b/tests/fixtures/error_catalogue/codes/schema_not_found.json @@ -0,0 +1,21 @@ +{ + "data": null, + "errors": [ + { + "message": "Unable to find the schema TestWidget in the database.", + "locations": [ + { + "line": 2, + "column": 3 + } + ], + "extensions": { + "code": "SCHEMA_NOT_FOUND", + "http_status": 422, + "data": { + "kind": "TestWidget" + } + } + } + ] +} diff --git a/tests/fixtures/error_catalogue/codes/token_expired.json b/tests/fixtures/error_catalogue/codes/token_expired.json new file mode 100644 index 00000000..7fcddb59 --- /dev/null +++ b/tests/fixtures/error_catalogue/codes/token_expired.json @@ -0,0 +1,21 @@ +{ + "data": null, + "errors": [ + { + "message": "Expired Signature", + "locations": [ + { + "line": 2, + "column": 3 + } + ], + "extensions": { + "code": "TOKEN_EXPIRED", + "http_status": 401, + "data": { + "expired_at": "2026-09-18T09:30:00+00:00" + } + } + } + ] +} diff --git a/tests/fixtures/error_catalogue/codes/undefined_error.json b/tests/fixtures/error_catalogue/codes/undefined_error.json new file mode 100644 index 00000000..930f532f --- /dev/null +++ b/tests/fixtures/error_catalogue/codes/undefined_error.json @@ -0,0 +1,19 @@ +{ + "data": null, + "errors": [ + { + "message": "Cannot query field 'nope' on type 'Query'.", + "locations": [ + { + "line": 1, + "column": 9 + } + ], + "extensions": { + "code": "UNDEFINED_ERROR", + "http_status": 500, + "data": {} + } + } + ] +} diff --git a/tests/fixtures/error_catalogue/codes/uniqueness_violation.json b/tests/fixtures/error_catalogue/codes/uniqueness_violation.json new file mode 100644 index 00000000..acc470d0 --- /dev/null +++ b/tests/fixtures/error_catalogue/codes/uniqueness_violation.json @@ -0,0 +1,27 @@ +{ + "data": null, + "errors": [ + { + "message": "Node of kind TestPerson already has name 'John'", + "locations": [ + { + "line": 2, + "column": 3 + } + ], + "path": [ + "TestPersonCreate" + ], + "extensions": { + "code": "UNIQUENESS_VIOLATION", + "http_status": 422, + "data": { + "node_kind": "TestPerson", + "fields": [ + "name" + ] + } + } + } + ] +} diff --git a/tests/integration/test_infrahub_client.py b/tests/integration/test_infrahub_client.py index bc39c232..f701f503 100644 --- a/tests/integration/test_infrahub_client.py +++ b/tests/integration/test_infrahub_client.py @@ -9,7 +9,13 @@ from infrahub_sdk import Config, InfrahubClient from infrahub_sdk.branch import BranchData from infrahub_sdk.constants import InfrahubClientMode -from infrahub_sdk.exceptions import BranchNotFoundError, URLNotFoundError +from infrahub_sdk.exceptions import ( + BranchNotFoundError, + GraphQLError, + NodeNotFoundError, + UniquenessViolationError, + URLNotFoundError, +) from infrahub_sdk.node import InfrahubNode from infrahub_sdk.playback import JSONPlayback from infrahub_sdk.recorder import JSONRecorder @@ -200,6 +206,41 @@ async def test_query_unexisting_branch(self, client: InfrahubClient) -> None: with pytest.raises(URLNotFoundError, match=r"/graphql/unexisting` not found."): await client.execute_graphql(query="unused", branch_name="unexisting") + @pytest.mark.catalogue + async def test_a_unique_attribute_collision_raises_its_own_class( + self, client: InfrahubClient, base_dataset: None + ) -> None: + """Against a real server, so the payload is the one Infrahub sends rather than a fixture.""" + duplicate = await client.create(kind=TESTING_PERSON, name="Liam Walker", height=180) + + # Either message shape: a described failure names its code, an undescribed one keeps the + # text the GraphQL call site has always produced. + with pytest.raises(GraphQLError, match=r"UNIQUENESS_VIOLATION|An error occurred while") as exc_info: + await duplicate.save() + + if exc_info.value.code != "UNIQUENESS_VIOLATION": + pytest.skip(f"this server reports a uniqueness violation as {exc_info.value.code}") + + assert isinstance(exc_info.value, UniquenessViolationError) + assert exc_info.value.node_kind == TESTING_PERSON + assert exc_info.value.fields == ["name"] + + @pytest.mark.catalogue + async def test_deleting_a_missing_node_raises_its_own_class( + self, client: InfrahubClient, base_dataset: None + ) -> None: + obj = await client.create(kind=TESTING_PERSON, name="Gone Walker", height=170) + await obj.save() + node_id = obj.id + await obj.delete() + + with pytest.raises(NodeNotFoundError, match="NODE_NOT_FOUND") as exc_info: + await obj.delete() + + assert exc_info.value.code == "NODE_NOT_FOUND" + assert exc_info.value.node_type == TESTING_PERSON + assert exc_info.value.identifier == node_id + async def test_create_generic_rel_with_hfid( self, client: InfrahubClient, diff --git a/tests/integration/test_infrahub_client_sync.py b/tests/integration/test_infrahub_client_sync.py index 57d7c842..4e9e9250 100644 --- a/tests/integration/test_infrahub_client_sync.py +++ b/tests/integration/test_infrahub_client_sync.py @@ -8,7 +8,13 @@ from infrahub_sdk import Config, InfrahubClientSync from infrahub_sdk.branch import BranchData from infrahub_sdk.constants import InfrahubClientMode -from infrahub_sdk.exceptions import BranchNotFoundError, URLNotFoundError +from infrahub_sdk.exceptions import ( + BranchNotFoundError, + GraphQLError, + NodeNotFoundError, + UniquenessViolationError, + URLNotFoundError, +) from infrahub_sdk.node import InfrahubNodeSync from infrahub_sdk.playback import JSONPlayback from infrahub_sdk.recorder import JSONRecorder @@ -201,6 +207,41 @@ def test_query_unexisting_branch(self, client_sync: InfrahubClientSync) -> None: with pytest.raises(URLNotFoundError, match=r"/graphql/unexisting` not found."): client_sync.execute_graphql(query="unused", branch_name="unexisting") + @pytest.mark.catalogue + def test_a_unique_attribute_collision_raises_its_own_class( + self, client_sync: InfrahubClientSync, base_dataset: None + ) -> None: + """Against a real server, so the payload is the one Infrahub sends rather than a fixture.""" + duplicate = client_sync.create(kind=TESTING_PERSON, name="Liam Walker", height=180) + + # Either message shape: a described failure names its code, an undescribed one keeps the + # text the GraphQL call site has always produced. + with pytest.raises(GraphQLError, match=r"UNIQUENESS_VIOLATION|An error occurred while") as exc_info: + duplicate.save() + + if exc_info.value.code != "UNIQUENESS_VIOLATION": + pytest.skip(f"this server reports a uniqueness violation as {exc_info.value.code}") + + assert isinstance(exc_info.value, UniquenessViolationError) + assert exc_info.value.node_kind == TESTING_PERSON + assert exc_info.value.fields == ["name"] + + @pytest.mark.catalogue + def test_deleting_a_missing_node_raises_its_own_class( + self, client_sync: InfrahubClientSync, base_dataset: None + ) -> None: + obj = client_sync.create(kind=TESTING_PERSON, name="Vanished Walker", height=170) + obj.save() + node_id = obj.id + obj.delete() + + with pytest.raises(NodeNotFoundError, match="NODE_NOT_FOUND") as exc_info: + obj.delete() + + assert exc_info.value.code == "NODE_NOT_FOUND" + assert exc_info.value.node_type == TESTING_PERSON + assert exc_info.value.identifier == node_id + def test_create_generic_rel_with_hfid( self, client_sync: InfrahubClientSync, diff --git a/tests/unit/sdk/test_client.py b/tests/unit/sdk/test_client.py index d63c8c56..21dc4518 100644 --- a/tests/unit/sdk/test_client.py +++ b/tests/unit/sdk/test_client.py @@ -3,15 +3,22 @@ import inspect import json import ssl -from dataclasses import dataclass +from dataclasses import dataclass, field +from io import BytesIO from pathlib import Path -from typing import TYPE_CHECKING +from typing import TYPE_CHECKING, Any import pytest from infrahub_sdk import Config, InfrahubClient, InfrahubClientSync -from infrahub_sdk.exceptions import NodeNotFoundError +from infrahub_sdk.exceptions import ( + ApiError, + AuthenticationError, + NodeNotFoundError, + UniquenessViolationError, +) from infrahub_sdk.node import InfrahubNode, InfrahubNodeSync +from tests.helpers.fixtures import read_fixture if TYPE_CHECKING: from collections.abc import Callable, Mapping @@ -932,3 +939,85 @@ class GraphQLURLCase: async def test_graphql_url_encodes_branch_name(clients: BothClients, client_type: str, case: GraphQLURLCase) -> None: client = clients.standard if client_type == "standard" else clients.sync assert client._graphql_url(branch_name=case.branch_name) == case.expected_url + + +@dataclass +class CatalogueParityCase: + name: str + code: str + status_code: int + expected_class: type[ApiError] + expected_attributes: dict[str, Any] = field(default_factory=dict) + upload: bool = False + + +CATALOGUE_PARITY_CASES = [ + CatalogueParityCase( + name="uniqueness-violation-inside-a-200", + code="UNIQUENESS_VIOLATION", + status_code=200, + expected_class=UniquenessViolationError, + expected_attributes={"node_kind": "TestPerson", "fields": ["name"], "http_status": 422}, + ), + CatalogueParityCase( + name="node-not-found-inside-a-200", + code="NODE_NOT_FOUND", + status_code=200, + expected_class=NodeNotFoundError, + expected_attributes={"node_type": "TestPerson", "identifier": "john", "http_status": 404}, + ), + CatalogueParityCase( + name="permission-denied-on-a-real-403", + code="PERMISSION_DENIED", + status_code=403, + expected_class=AuthenticationError, + expected_attributes={"http_status": 403}, + ), + CatalogueParityCase( + name="permission-denied-on-a-rejected-upload", + code="PERMISSION_DENIED", + status_code=403, + expected_class=AuthenticationError, + expected_attributes={"http_status": 403}, + upload=True, + ), +] + + +@pytest.mark.parametrize("client_type", client_types) +@pytest.mark.parametrize("case", [pytest.param(tc, id=tc.name) for tc in CATALOGUE_PARITY_CASES]) +async def test_both_clients_raise_the_same_catalogued_exception( + httpx_mock: HTTPXMock, clients: BothClients, client_type: str, case: CatalogueParityCase +) -> None: + """The same failure reaches the caller as the same class, whichever client sent the request. + + Each arrival path builds its exception at a different call site, so parity is a property of the + client rather than of the factory, and only driving both proves it. + """ + envelope = json.loads(read_fixture(file_name=f"{case.code.lower()}.json", fixture_subdir="error_catalogue/codes")) + httpx_mock.add_response(method="POST", status_code=case.status_code, json=envelope) + client = clients.standard if client_type == "standard" else clients.sync + + with pytest.raises(case.expected_class, match=case.code) as exc_info: + if case.upload: + if isinstance(client, InfrahubClient): + await client._execute_graphql_with_file( + query="mutation ($file: Upload!) { CoreFileUpload(data: {file: $file}) { ok }}", + file_content=BytesIO(b"x"), + file_name="f.txt", + ) + else: + client._execute_graphql_with_file( + query="mutation ($file: Upload!) { CoreFileUpload(data: {file: $file}) { ok }}", + file_content=BytesIO(b"x"), + file_name="f.txt", + ) + elif isinstance(client, InfrahubClient): + await client.execute_graphql(query="query { TestPerson { edges { node { id }}}}") + else: + client.execute_graphql(query="query { TestPerson { edges { node { id }}}}") + + assert type(exc_info.value) is case.expected_class + assert exc_info.value.code == case.code + for attribute, value in case.expected_attributes.items(): + assert getattr(exc_info.value, attribute) == value diff --git a/tests/unit/sdk/test_error_catalogue.py b/tests/unit/sdk/test_error_catalogue.py index b52101a6..644ebc68 100644 --- a/tests/unit/sdk/test_error_catalogue.py +++ b/tests/unit/sdk/test_error_catalogue.py @@ -12,11 +12,24 @@ from infrahub_sdk import Config, InfrahubClient, InfrahubClientSync from infrahub_sdk.exceptions import ( + AttributeConstraintViolationError, + AttributeInvalidTypeError, + AttributeRequiredError, AuthenticationError, + BranchAlreadyMergedError, + BranchNeedsRebaseError, + BranchNotFoundError, GraphQLError, + MergeInProgressError, + MergeRecoveryRequiredError, + NodeNotFoundError, + SchemaNotFoundError, + UndefinedError, + UniquenessViolationError, authentication_error_from_response, graphql_error_from_response, ) +from infrahub_sdk.exceptions.catalogue import CODE_TO_DATA_MODEL from tests.helpers.fixtures import read_fixture if TYPE_CHECKING: @@ -25,12 +38,25 @@ from tests.unit.sdk.conftest import BothClients FIXTURE_SUBDIR = "error_catalogue" +CODES_FIXTURE_SUBDIR = f"{FIXTURE_SUBDIR}/codes" + +# The three codes that reach the SDK on either transport, so neither transport's class can be theirs. +CODES_WITHOUT_A_CLASS = {"AUTHENTICATION_REQUIRED", "PERMISSION_DENIED", "TOKEN_EXPIRED"} def load_envelope(name: str) -> dict[str, Any]: return json.loads(read_fixture(file_name=name, fixture_subdir=FIXTURE_SUBDIR)) +def load_code_envelope(code: str) -> dict[str, Any]: + """The captured response for one catalogue code. + + Addressed by code rather than by listing the directory, so a code whose fixture is missing fails + here instead of quietly dropping out of the parametrisation that is supposed to be exhaustive. + """ + return json.loads(read_fixture(file_name=f"{code.lower()}.json", fixture_subdir=CODES_FIXTURE_SUBDIR)) + + def auth_response(envelope: dict[str, Any] | str, status_code: int = 401) -> httpx.Response: """Build the httpx response the SDK would have observed for a rejected request. @@ -80,6 +106,274 @@ def test_query_and_variables_are_retained(self) -> None: assert exc.variables == {"a": 1} +@dataclass +class CodeCase: + name: str + expected_class: type[GraphQLError] + expected_http_status: int + expected_attributes: dict[str, Any] + + +CODE_CASES = [ + CodeCase( + name="ATTRIBUTE_CONSTRAINT_VIOLATION", + expected_class=AttributeConstraintViolationError, + expected_http_status=422, + expected_attributes={ + "node_kind": "TestPerson", + "field_name": "name", + "constraint": "regex", + "detail": "^[A-Z]", + }, + ), + CodeCase( + name="ATTRIBUTE_INVALID_TYPE", + expected_class=AttributeInvalidTypeError, + expected_http_status=422, + expected_attributes={ + "node_kind": "TestPerson", + "field_name": "height", + "expected_type": "Integer", + "received_type": "String", + }, + ), + CodeCase( + name="ATTRIBUTE_REQUIRED", + expected_class=AttributeRequiredError, + expected_http_status=422, + expected_attributes={"node_kind": "TestPerson", "field_name": "name"}, + ), + CodeCase( + name="AUTHENTICATION_REQUIRED", + expected_class=GraphQLError, + expected_http_status=401, + expected_attributes={}, + ), + CodeCase( + name="BRANCH_ALREADY_MERGED", + expected_class=BranchAlreadyMergedError, + expected_http_status=400, + expected_attributes={"branch_name": "feature-a"}, + ), + CodeCase( + name="BRANCH_NEEDS_REBASE", + expected_class=BranchNeedsRebaseError, + expected_http_status=400, + expected_attributes={"branch_name": "feature-a"}, + ), + CodeCase( + name="BRANCH_NOT_FOUND", + expected_class=BranchNotFoundError, + expected_http_status=400, + expected_attributes={"identifier": "does-not-exist"}, + ), + CodeCase( + name="MERGE_IN_PROGRESS", + expected_class=MergeInProgressError, + expected_http_status=423, + expected_attributes={"branch_name": "main", "merging_branch": "feature-a"}, + ), + CodeCase( + name="MERGE_RECOVERY_REQUIRED", + expected_class=MergeRecoveryRequiredError, + expected_http_status=423, + expected_attributes={"branch_name": "main", "merging_branch": "feature-a"}, + ), + CodeCase( + name="NODE_NOT_FOUND", + expected_class=NodeNotFoundError, + expected_http_status=404, + expected_attributes={"node_type": "TestPerson", "identifier": "john"}, + ), + CodeCase( + name="PERMISSION_DENIED", + expected_class=GraphQLError, + expected_http_status=403, + expected_attributes={}, + ), + CodeCase( + name="SCHEMA_NOT_FOUND", + expected_class=SchemaNotFoundError, + expected_http_status=422, + expected_attributes={"identifier": "TestWidget"}, + ), + CodeCase( + name="TOKEN_EXPIRED", + expected_class=GraphQLError, + expected_http_status=401, + expected_attributes={}, + ), + CodeCase( + name="UNDEFINED_ERROR", + expected_class=UndefinedError, + expected_http_status=500, + expected_attributes={}, + ), + CodeCase( + name="UNIQUENESS_VIOLATION", + expected_class=UniquenessViolationError, + expected_http_status=422, + expected_attributes={"node_kind": "TestPerson", "fields": ["name"]}, + ), +] + + +class TestEveryCatalogueCode: + """One case per code, reading the raised class and its attributes and never a message.""" + + def test_the_cases_cover_every_code_the_bindings_carry(self) -> None: + """Exhaustive only if it is checked against the bindings rather than maintained by hand.""" + assert {case.name for case in CODE_CASES} == set(CODE_TO_DATA_MODEL) + + def test_the_codes_with_no_class_of_their_own_are_the_authentication_ones(self) -> None: + """The one asymmetry in the hierarchy, and the reason the fallback follows the transport.""" + assert {case.name for case in CODE_CASES if case.expected_class is GraphQLError} == CODES_WITHOUT_A_CLASS + + @pytest.mark.parametrize("case", [pytest.param(tc, id=tc.name) for tc in CODE_CASES]) + def test_the_code_raises_its_class_with_its_payload_promoted(self, case: CodeCase) -> None: + envelope = load_code_envelope(case.name) + + exc = graphql_error_from_response(errors=envelope["errors"], query="query { x }") + + assert type(exc) is case.expected_class + assert exc.code == case.name + assert exc.http_status == case.expected_http_status + for attribute, value in case.expected_attributes.items(): + assert getattr(exc, attribute) == value + + @pytest.mark.parametrize("case", [pytest.param(tc, id=tc.name) for tc in CODE_CASES]) + def test_the_code_is_identifiable_without_reading_a_message(self, case: CodeCase) -> None: + """A class of its own, or a code on the generic class: either way, no words are parsed.""" + envelope = load_code_envelope(case.name) + + exc = graphql_error_from_response(errors=envelope["errors"]) + + assert isinstance(exc, GraphQLError) + assert exc.code == case.name + + @pytest.mark.parametrize("case", [pytest.param(tc, id=tc.name) for tc in CODE_CASES]) + def test_the_raw_payload_stays_available_for_forwarding(self, case: CodeCase) -> None: + envelope = load_code_envelope(case.name) + + exc = graphql_error_from_response(errors=envelope["errors"]) + + assert exc.extensions is not None + assert exc.extensions["data"] == envelope["errors"][0]["extensions"]["data"] + + +class TestTheAdoptedClasses: + """The three classes that predate the catalogue, now reachable from a server-reported failure. + + They are the only catalogued classes that are also raised with no code behind them, so `code` is + what tells the two apart - and their attributes keep the names they have always had. + """ + + def test_a_server_reported_node_not_found_populates_the_node_attributes(self) -> None: + envelope = load_code_envelope("NODE_NOT_FOUND") + + exc = graphql_error_from_response(errors=envelope["errors"]) + + assert isinstance(exc, NodeNotFoundError) + assert exc.node_type == "TestPerson" + assert exc.identifier == "john" + assert exc.code == "NODE_NOT_FOUND" + + def test_a_server_reported_branch_not_found_populates_the_identifier(self) -> None: + envelope = load_code_envelope("BRANCH_NOT_FOUND") + + exc = graphql_error_from_response(errors=envelope["errors"]) + + assert isinstance(exc, BranchNotFoundError) + assert exc.identifier == "does-not-exist" + assert exc.code == "BRANCH_NOT_FOUND" + + def test_a_server_reported_schema_not_found_populates_the_identifier(self) -> None: + envelope = load_code_envelope("SCHEMA_NOT_FOUND") + + exc = graphql_error_from_response(errors=envelope["errors"]) + + assert isinstance(exc, SchemaNotFoundError) + assert exc.identifier == "TestWidget" + assert exc.code == "SCHEMA_NOT_FOUND" + + def test_a_client_side_raise_of_the_same_class_carries_no_code(self) -> None: + """`exc.code is not None` is the test for which of the two a caller is holding.""" + assert NodeNotFoundError(identifier="john", node_type="TestPerson").code is None + assert BranchNotFoundError(identifier="does-not-exist").code is None + assert SchemaNotFoundError(identifier="TestWidget").code is None + + +class TestTheFirstErrorGoverns: + def test_a_silent_first_error_governs_over_a_later_coded_one(self) -> None: + """Otherwise a response's class would depend on which error the SDK happens to recognise.""" + errors = [ + {"message": "the failure that came first"}, + { + "message": "and one the catalogue describes", + "extensions": { + "code": "UNIQUENESS_VIOLATION", + "http_status": 422, + "data": {"node_kind": "TestPerson", "fields": ["name"]}, + }, + }, + ] + + exc = graphql_error_from_response(errors=errors, query="mutation { TestPersonCreate }") + + assert type(exc) is GraphQLError + assert exc.code is None + assert exc.http_status is None + + def test_the_complete_list_is_retained_unreordered(self) -> None: + errors = [ + {"message": "the failure that came first"}, + { + "message": "and one the catalogue describes", + "extensions": { + "code": "UNIQUENESS_VIOLATION", + "http_status": 422, + "data": {"node_kind": "TestPerson", "fields": ["name"]}, + }, + }, + ] + + exc = graphql_error_from_response(errors=errors) + + assert [error["message"] for error in exc.errors] == [ + "the failure that came first", + "and one the catalogue describes", + ] + + +class TestTheFallbackFollowsTheObservedTransport: + """Which generic class a code falls back to is decided by how the SDK saw it arrive. + + Following the status the catalogue declares instead would send the three authentication codes to + `AuthenticationError` whenever a resolver raised them inside an HTTP 200, out of reach of the + `except GraphQLError` clause that catches them today - and send a 401 carrying a data code to a + class no caller of that path expects. + """ + + @pytest.mark.parametrize("code", sorted(CODES_WITHOUT_A_CLASS)) + def test_an_authentication_code_inside_a_graphql_response_raises_the_graphql_class(self, code: str) -> None: + envelope = load_code_envelope(code) + + exc = graphql_error_from_response(errors=envelope["errors"]) + + assert type(exc) is GraphQLError, "the declared 401 or 403 is metadata, not the transport" + assert not isinstance(exc, AuthenticationError) + assert exc.code == code + + def test_a_data_code_on_a_real_401_raises_the_authentication_class(self) -> None: + response = auth_response(envelope=load_code_envelope("NODE_NOT_FOUND")) + + exc = authentication_error_from_response(response=response) + + assert type(exc) is AuthenticationError, "a class the caller of this path cannot expect is worse than none" + assert exc.code == "NODE_NOT_FOUND" + assert exc.http_status == 404, "the declared status is still readable, it just governs nothing" + + class TestAuthenticationFactory: def test_reads_code_and_status_off_a_real_401(self) -> None: response = auth_response(envelope=load_envelope("auth_token_expired.json")) @@ -250,6 +544,7 @@ class CrossVersionCase: fixture: str expected_code: str | None expected_http_status: int | None = None + expected_class: type[GraphQLError] = GraphQLError CROSS_VERSION_CASES = [ @@ -264,6 +559,7 @@ class CrossVersionCase: fixture="graphql_extra_payload_field.json", expected_code="UNIQUENESS_VIOLATION", expected_http_status=422, + expected_class=UniquenessViolationError, ), CrossVersionCase( name="error-carrying-no-extensions", @@ -280,13 +576,17 @@ class CrossVersionCase: @pytest.mark.crossversion @pytest.mark.parametrize("case", [pytest.param(tc, id=tc.name) for tc in CROSS_VERSION_CASES]) -def test_cross_version_envelope_parses_onto_the_generic_class(case: CrossVersionCase) -> None: - """Any SDK version talks to any server version, and parsing never raises.""" +def test_a_cross_version_envelope_parses_without_raising(case: CrossVersionCase) -> None: + """Any SDK version talks to any server version, and parsing never raises. + + A code these bindings have a class for still reaches it here: gaining a field it has never heard + of changes nothing, which is the forward compatibility the payload models are for. + """ envelope = load_envelope(case.fixture) exc = graphql_error_from_response(errors=envelope["errors"], query="query { x }") - assert type(exc) is GraphQLError + assert type(exc) is case.expected_class assert exc.code == case.expected_code assert exc.http_status == case.expected_http_status