diff --git a/changelog/+typed-per-code-exceptions.added.md b/changelog/+typed-per-code-exceptions.added.md new file mode 100644 index 000000000..22994adb7 --- /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 2ed8af506..1a36d01d9 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 000000000..b4db57897 --- /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/__init__.py b/infrahub_sdk/exceptions/__init__.py index 94b565290..a958fbbd3 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/infrahub_sdk/exceptions/base.py b/infrahub_sdk/exceptions/base.py index 34c220c81..ff6e22e8a 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/catalogue.py b/infrahub_sdk/exceptions/catalogue.py new file mode 100644 index 000000000..d2b8f0a73 --- /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) diff --git a/infrahub_sdk/exceptions/factory.py b/infrahub_sdk/exceptions/factory.py index ec12901df..9b18520d3 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 74978bf82..849072cfe 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 43c051e08..68c88ed5c 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 000000000..c8eb60a4a --- /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 000000000..ffe04011e --- /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 000000000..20c5e005c --- /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 000000000..c49b79209 --- /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 000000000..11a4ba7fa --- /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 000000000..7b1268ba2 --- /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 000000000..c88dfbab8 --- /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 000000000..3494578d6 --- /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 000000000..b7bd7e704 --- /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 000000000..b606ed061 --- /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 000000000..b6c1ad7c7 --- /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 000000000..9f265b84f --- /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 000000000..7fcddb596 --- /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 000000000..930f532fb --- /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 000000000..acc470d08 --- /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/fixtures/error_catalogue/public_names.json b/tests/fixtures/error_catalogue/public_names.json index 79a809034..172c49f5b 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/integration/test_infrahub_client.py b/tests/integration/test_infrahub_client.py index bc39c2320..f701f503e 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 57d7c842e..4e9e9250e 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 d63c8c564..21dc4518d 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 b52101a6f..644ebc68d 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 diff --git a/tests/unit/sdk/test_exceptions_layering.py b/tests/unit/sdk/test_exceptions_layering.py index e6f71bb4d..e76897553 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 5d100e191..c94a4190e 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: