Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
60 changes: 60 additions & 0 deletions dev/specs/ifc-3034-error-catalogue/checklists/requirements.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,60 @@
# Specification Quality Checklist: Error Catalogue in the Python SDK

**Purpose**: Validate specification completeness and quality before proceeding to planning
**Created**: 2026-08-21
**Feature**: [spec.md](../spec.md)

## Content Quality

- [x] No implementation details (languages, frameworks, APIs)
- [x] Focused on user value and business needs
- [x] Written for non-technical stakeholders
- [x] All mandatory sections completed

## Requirement Completeness

- [x] No [NEEDS CLARIFICATION] markers remain
- [x] Requirements are testable and unambiguous
- [x] Success criteria are measurable
- [x] Success criteria are technology-agnostic (no implementation details)
- [x] All acceptance scenarios are defined
- [x] Edge cases are identified
- [x] Scope is clearly bounded
- [x] Dependencies and assumptions identified

## Feature Readiness

- [x] All functional requirements have clear acceptance criteria
- [x] User scenarios cover primary flows
- [x] Feature meets measurable outcomes defined in Success Criteria
- [x] No implementation details leak into specification

## Notes

Two checklist items were resolved by scoping rather than by rewriting, and the reasoning is recorded
here so the plan phase does not relitigate it:

- **"No implementation details" / "written for non-technical stakeholders"** — for a library, the
exception hierarchy *is* the user-facing product, so class names, catalogue codes, and the
transport split are domain vocabulary rather than implementation leakage. The spec names those and
deliberately withholds module layout, file names, generator implementation, and test mechanics.
Recorded as an explicit assumption in the spec rather than left implicit.
- **"Success criteria are technology-agnostic"** — SC-001 through SC-008 are stated as outcomes a
consumer or reviewer can verify (a failure is handleable without reading a message; no string
matching remains; a stale artefact fails validation) rather than as internal mechanics. They do
reference exceptions and catalogue codes, which is unavoidable and correct for this feature.

Two items were originally deferred to the plan and have since been pulled back into the spec, both
prompted by automated review of the pull request:

- **The `identifier` contract on the unified `NodeNotFoundError`.** Deferring the whole question was
wrong: *which* attributes a consumer can read is observable API surface and belongs here, even
though the mechanism does not. FR-016 now pins the contract — every construction shape in use today
keeps working, the server-reported kind and identifier are reachable, one documented accessor works
for both cases, and any type widening is called out in release notes. Surveying the code for this
also turned up that the attribute is *already* heterogeneous: the file handler passes a plain string
where the declared type is a mapping.
- **Multi-error precedence.** FR-013 originally required only that a rule exist, which is untestable
until the rule does. It now specifies that the first error in the response governs, with the
complete list retained, and records why first-*recognised* was rejected: it would make the raised
type depend on binding freshness rather than on the response.
123 changes: 123 additions & 0 deletions dev/specs/ifc-3034-error-catalogue/contracts/exception-hierarchy.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,123 @@
# Contract: The exception hierarchy

The SDK's public interface here is the set of names a consumer can import from
`infrahub_sdk.exceptions`, catch, and read attributes off. This is what the change promises.

`infrahub_sdk.exceptions` is the supported import path for every exception the SDK raises, generated or
hand-written. A consumer never needs to know which module inside it defines a given class, and the
modules beneath it are internal. Every name importable from `infrahub_sdk.exceptions` before this
change is still importable from it afterwards, pinned by a test against a committed snapshot rather
than asserted.

## Catching

| Intent | Clause |
|--------|--------|
| Anything the server rejected, on either transport | `except ApiError` |
| Any GraphQL-path failure, including catalogued permission and token failures | `except GraphQLError` |
| Any authentication or permission failure, either transport | `except AuthenticationError` |
| One specific catalogued failure | `except UniquenessViolationError` (and so on per code) |
| Anything the SDK raises | `except Error` |

The catalogued 401/403 classes deliberately satisfy both `except GraphQLError` and
Comment thread
cubic-dev-ai[bot] marked this conversation as resolved.
`except AuthenticationError`, because they reach the SDK on the GraphQL transport two different ways:

- **Inside a 200 response's `errors` array**, when the failure was raised from within a resolver.
`except GraphQLError` catches such a response today; the dual base is what keeps that true while also
making `except AuthenticationError` catch it.
- **As a real 401 or 403**, when the failure escapes before query execution.
`except AuthenticationError` catches this today. `except GraphQLError` does not, because the SDK
raises before reading the body — under this change it will, since the authentication path now resolves
the catalogue code and raises the specific class. That is a broadening, listed below.

Every clause that worked before the change still catches what it caught before (FR-018). Three
broadenings are deliberate:

- `except GraphQLError` now also catches node, branch, and schema lookup misses that involved no
GraphQL request at all — both the client-side ones and the REST 404 the file handler turns into a
`NodeNotFoundError` — because those classes are re-rooted under it.
- `except GraphQLError` now also catches a real 401 or 403 whose code **this SDK's bindings recognise**,
which previously raised a plain `AuthenticationError`. This follows from the dual base: the class is
chosen by the code, and the same class serves both arrival paths. It does not extend to a 401 or 403
carrying a code the bindings do not know — that falls back to the generic `AuthenticationError` for
the transport, which is not a `GraphQLError`.
- Code that catches the generic error to inspect its message will now sometimes receive a subclass
whose message names the code instead of embedding the query.

## Reading a caught error

Available on every `ApiError`:

| Attribute | Contract |
|-----------|----------|
| `code` | The catalogue code string, or `None`. Never an integer. `None` means the SDK resolved no catalogue code — a pre-catalogue server, a REST failure, an error with no `extensions`, or an integer `code` on the wire. An unrecognised string code from a newer server is still readable here. |
| `http_status` | The code's catalogue-declared status, or `None`. This is metadata about the failure, not the HTTP status of the response — a catalogued data error arrives as HTTP 200. Where the error carried an `extensions` mapping, the status the server actually returned is available as `exc.extensions["http_status"]` — guard on `exc.extensions` first, since it is `None` when the error carried none. The two can legitimately differ: the server replaces a declared 500 with the real HTTP status when it has a more accurate one. |
| the payload's fields | Not on the base. Each catalogued class carries its payload's fields as directly typed attributes — `UniquenessViolationError.node_kind` is a `str`, `.fields` a `list[str]` — typed exactly as the catalogue declares them, so a required field is never optional and needs no guard. The raw payload dict remains in `extensions["data"]` for anything forwarding it verbatim. |
| `extensions` | The raw `extensions` mapping of the governing error, or `None`. |
| `errors` | The complete server error list, unreordered — empty for a client-side raise. |
| `query`, `variables` | The GraphQL query and variables where there was one, otherwise `None`. |

`errors`, `query`, and `variables` are readable on every `ApiError`, not only on those built from a
server response. A purely client-side `NodeNotFoundError` has an empty `errors` and `None` for the rest,
so code that catches `GraphQLError` and inspects them never has to guard for a missing attribute.

`UNDEFINED_ERROR` is a code like any other: it means the server explicitly reported a gap in its own
catalogue, and it is not the same as an error carrying no `extensions`.

## Cross-version behaviour

Any SDK version talks to any server version. Parsing never raises.

| Situation | Behaviour |
|-----------|-----------|
| A code the SDK has never heard of | The generic class for the branch is raised — `GraphQLError` for data failures, `AuthenticationError` for 401/403 — with `code` set to the string the server sent. |
| A known code whose payload gained a field | The unknown field is ignored; behaviour is unchanged. |
| A server predating the catalogue, or an error with no `extensions` | Today's behaviour exactly; `code` is `None`. |
| An integer `code` on `/graphql` from a pre-catalogue server | Not surfaced as a catalogue code; `code` is `None`. |
| A payload that violates the catalogue's own contract | The generic class for the branch, with the code still readable. The specific class's attributes are typed as the catalogue declares them, so there is nothing to populate a required one with. |

Every fallback above is logged at debug level with the code involved, so an SDK meeting a newer server
is diagnosable in the field rather than only in tests.

Regenerating bindings buys typed handling of newly catalogued codes. It never changes which exception
a byte-identical response produces for a code the SDK already knows, because the first error in the
response governs unconditionally — not the first *recognised* one.

## Multiple errors in one response

The first error in the response determines the class raised. The complete list is retained on the
exception, unreordered, and nothing is discarded. If the first error carries no code and a later one
does, the generic class for the branch is raised.

## Messages

A catalogued failure's message names the code and the server's message and contains no query text.
An uncatalogued failure's message is byte-identical to today's, query text included. The query is
available as an attribute in both cases.

Where the catalogue provides them, the server's message names the failing action and resource kind, so
that detail now appears in logs and CLI output in place of the query text that used to be there.

## Parity

The async and sync clients raise the same type with the same attributes for the same failure, for
every catalogued code.

## Stability

`infrahub_sdk.exceptions` is treated as public and is the one import path a consumer needs. That is a
stronger promise than the constitution's tiering strictly requires — only `Config`, `InfrahubClient`,
and `InfrahubClientSync` are exported at top level — and it is made deliberately, because
`infrahubctl`, the Ansible collection, and external consumers already import from it directly.

Concretely:

- No name is removed or renamed, and no constructor loses a signature it has today.
- Every name importable from `infrahub_sdk.exceptions` before this change remains importable from it,
which a test pins against a committed snapshot. Restructuring the module into a package must not be
observable from the outside.
- Modules beneath `infrahub_sdk.exceptions` are internal. Importing `…exceptions.catalogue` or
`…exceptions.payloads` directly is not supported, and their layout may change.
- One annotation widens: `NodeNotFoundError.identifier` becomes `Mapping[str, list[str]] | str`. It is
called out in a changelog fragment because external consumers read these attributes even though
nothing in this repository does.
122 changes: 122 additions & 0 deletions dev/specs/ifc-3034-error-catalogue/contracts/generator-contract.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,122 @@
# Contract: Generating the SDK's error bindings

One artefact crosses from the Infrahub repository into the SDK. This is the contract between the two
sides. The SDK holds no copy of the catalogue schema (FR-010).

## The artefact

- **Source**: `schema/error-catalogue.json` in the Infrahub repository.
- **Output**: `infrahub_sdk/exceptions/catalogue.py` in the SDK submodule.
- **Owner**: Infrahub. The SDK never regenerates it, exactly as it never regenerates `protocols.py`
or its schema models.
- **Committed**: yes. Generation happens in a pull request, not at install time, so catalogue drift is
visible in the diff.

The file is generated **in full**. There is never a hand-edited region inside a generated file, nor a
generated region inside a hand-written one.

## What the generated module contains

It opens with a header marking it generated and not to be edited, naming the source artefact, recording
the catalogue's `infrahub_catalogue_version`, and giving the regeneration command (FR-009) — the same
marking style as the repository's other generated files. It declares `__all__`, which is what lets the
package façade re-export it without a hand-maintained list.

The body holds three things:

- One pydantic payload model per catalogue code, including codes with an empty payload and including
adopted codes. These validate the envelope and supply the promoted attributes' types.
- One exception class per catalogue code the SDK has not adopted, each promoting its payload's fields
to directly typed attributes and exposing a `from_payload` classmethod.
- `CODE_TO_EXCEPTION`, mapping every catalogue code to its class — generated classes for most,
imported adopted classes for the rest.

It imports only `infrahub_sdk.exceptions.base`, which imports nothing from inside the package. That
keeps the package's import graph one-way with no cycle: `base` → `catalogue` → `factory` → the façade.

Codes are emitted in sorted order so that reordering the catalogue's JSON does not churn the diff.

## Derivation rules

No hand-maintained per-code table exists on either side. Everything is derived from the catalogue
entry:

| Output | Derived from |
|--------|--------------|
| Exception class name | The code's parts capitalised and joined, with `Error` appended only if it does not already end in `Error`. `UNDEFINED_ERROR` → `UndefinedError`. |
| Payload model name | `data_schema.title`, verbatim. |
| Base classes | `GraphQLError` always; `http_status in {401, 403}` additionally adds `AuthenticationError`, emitted as `(GraphQLError, AuthenticationError)`. |
| `http_status` class attribute | The catalogue's declared `http_status`. |
| Docstring | The catalogue's `description` and `stability`. |
| Promoted attribute names | The payload field names, verbatim. |
| Field and attribute types | The JSON Schema mapping in [data-model.md](../data-model.md); a required field is non-optional, a nullable one carries its declared default. |

## Adoption

Some catalogue codes are represented by a class the SDK already ships. Those classes declare the code
they represent with a `CODE` class attribute. The generator parses
`infrahub_sdk/exceptions/base.py` with `ast`, collects every class whose body assigns a `CODE`
string, and for those codes emits an import and a map entry instead of a class definition. The payload
model is still generated.

An adopted class supplies its own `from_payload`, since its attribute names are its existing ones
rather than the catalogue's.

Adopting a further code later is a one-line change in the SDK's hand-written module with no generator
edit. Discovery is by parsing rather than importing, so the generator stays a pure text transform and
generation never depends on the SDK checkout being importable. The same walk collects every class name
defined in `base.py`, which is what the collision check below needs. Parsing also sidesteps a trap an
attribute walk would hit: `NodeInvalidError` inherits `CODE` from `NodeNotFoundError`, so two classes
would appear to claim the same code.

## Failing loudly

Generation aborts, rather than emitting a guess, when:

- a catalogue entry has no integer `http_status`;
- a catalogue entry has no non-empty `data_schema.title`;
- a `data_schema` uses a construct outside the supported vocabulary, in which case the offending
fragment appears in the error;
- `codes` is empty or the root is not an object;
- a derived class name collides with a class already defined in `base.py` that has not declared that
code as adopted.

The first four are the assertions the frontend generator already makes, for the same reason. The last
is specific to Python's import semantics: the SDK's façade re-exports `base` and then `catalogue`, so an
undeclared collision would let the generated class silently take the name and change what an existing
`except` clause catches. The SDK already defines `ValidationError`, `RateLimitError`,
`InvalidResponseError`, `FileNotValidError`, and `ResourceNotDefinedError` — every one of them the name
a plausible future code would derive — so this is a live hazard rather than a theoretical one. Failing
generation forces the choice (adopt the code, or rename) into the pull request that adds the code.

## Validation

`uv run invoke frontend.regenerate-error-bindings` regenerates the catalogue JSON, the frontend
TypeScript bindings, the docs page, and now the SDK bindings, so all catalogue-derived artefacts move
together.

`uv run invoke backend.validate-generated` verifies the SDK artefact with
`git -C python_sdk diff --exit-code infrahub_sdk/exceptions/catalogue.py`. The diff must run inside the
submodule, because from the superproject `git diff` only sees the submodule pointer — the same reason
the existing schema-model and protocol checks are written that way.

In CI, `backend-validate-generated` hosts this because it is the job that runs
`backend.validate-generated`. Its trigger condition gains `error_catalogue == 'true'` so a
catalogue-only change cannot slip past, and `error_catalogue_files` in `.github/file-filters.yml`
gains the submodule artefact path and the template, so editing the committed bindings or the generator
also triggers the check.

Submodule availability is not a factor in that placement. Infrahub declares
`infrahub-sdk = { path = "python_sdk", editable = true }`, so `uv sync` fails without the submodule and
every Python job there already requires `submodules: true`. Any job that needs the submodule declares
it; the check goes where it belongs and the checkout follows.

A catalogue change that skips regeneration therefore fails the pull request that made it (FR-026).
There is no release-time gate on either side; pull-request-time validation is the mechanism, matching
how the existing generated artefacts are treated (FR-027).

## What regeneration does and does not buy

Regenerating adds typed handling for newly catalogued codes. It never changes which exception a
byte-identical response produces for a code the SDK already knows, and correctness never depends on
having regenerated — an SDK with stale bindings falls back rather than failing.
Loading