Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
18 commits
Select commit Hold shift + click to select a range
dddbdcf
docs: add the error catalogue specification and implementation plan
ogenstad Sep 2, 2026
1ec4e05
docs: fix defects the full-diff review surfaced
ogenstad Sep 3, 2026
ed7524c
docs: correct two justifications the tenth review caught
ogenstad Sep 7, 2026
7418f48
docs: drop the multiple inheritance from the exception hierarchy
ogenstad Sep 8, 2026
7890f75
docs: make promoted attributes optional on the unified classes
ogenstad Sep 8, 2026
1707d04
docs: fix the contradictions left by removing the diamond
ogenstad Sep 9, 2026
e3f1236
docs: stop the examples restating which auth code arrives how
ogenstad Sep 9, 2026
55c5a56
docs: record both spike results, and split the catch example
ogenstad Sep 9, 2026
4d29afc
docs: reconcile the collision set and two more stale mechanisms
ogenstad Sep 9, 2026
4107885
docs: add the task breakdown and its pull request split
ogenstad Sep 9, 2026
c5ccbd0
docs: stop overstating how much of the phase order is forced
ogenstad Sep 9, 2026
cc26ca3
feat: parse the server error envelope onto the base exception classes…
ogenstad Sep 15, 2026
3735c7e
feat: unify and re-root the not-found classes under GraphQLError (#1363)
ogenstad Sep 17, 2026
2af53e7
test: align the retry suite with the catalogued error messages
ogenstad Sep 17, 2026
b02d2fc
feat: raise typed per-code exceptions from the error catalogue (#1373)
ogenstad Sep 29, 2026
beb37f1
docs: stop three changelog entries telling the same story
ogenstad Sep 29, 2026
a040b89
Merge remote-tracking branch 'origin/infrahub-develop' into pog-error…
ogenstad Sep 30, 2026
a2abeca
fix: keep a lookup miss out of the GraphQL error rendering in the CLI
ogenstad Sep 30, 2026
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
10 changes: 10 additions & 0 deletions changelog/+adopted-codes-raise-their-own-class.changed.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
A lookup miss the server reports - `NODE_NOT_FOUND`, `BRANCH_NOT_FOUND` or `SCHEMA_NOT_FOUND` - now raises `NodeNotFoundError`, `BranchNotFoundError` or `SchemaNotFoundError` built from the payload the server sent, where it previously raised a generic `GraphQLError`:

```python
try:
await client.delete(kind="NetworkDevice", id=device_id)
except NodeNotFoundError:
... # already gone
```

These three are the only catalogued classes the SDK also raises on its own, for a lookup that returned nothing and for the REST 404 behind a missing file. A ladder that handles one of them specifically will now see server-reported failures arrive there alongside the SDK's own, and `exc.code is not None` tells the two apart.
12 changes: 12 additions & 0 deletions changelog/+api-error-carries-the-request.added.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
`query` and `variables` are now readable on every `ApiError`, not only on the GraphQL branch. Reading either off an `AuthenticationError` previously raised `AttributeError`:

```python
try:
await client.execute_graphql(query=query)
except ApiError as exc:
log.error("request failed", code=exc.code, query=exc.query) # no longer raises on a 401
```

That clause is the one the SDK recommends for the three authentication codes, since each of them can arrive either on a real 401 or 403 or inside a GraphQL `errors` array, so it is exactly where the attributes had to exist. They join `code`, `http_status`, `extensions` and `errors`, which were already declared there for the same reason.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P3: ApiError is introduced in this PR, so these fields were not previously declared on it. Describe them as declared alongside query and variables on the new base.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At changelog/+api-error-carries-the-request.added.md, line 10:

<comment>`ApiError` is introduced in this PR, so these fields were not previously declared on it. Describe them as declared alongside `query` and `variables` on the new base.</comment>

<file context>
@@ -0,0 +1,12 @@
+    log.error("request failed", code=exc.code, query=exc.query)   # no longer raises on a 401
+```
+
+That clause is the one the SDK recommends for the three authentication codes, since each of them can arrive either on a real 401 or 403 or inside a GraphQL `errors` array, so it is exactly where the attributes had to exist. They join `code`, `http_status`, `extensions` and `errors`, which were already declared there for the same reason.
+
+`None` means the failed request was not recorded on the exception rather than that there was no query: an authentication failure is observed at the transport, which has neither to hand. Nothing that reads these attributes today changes - `GraphQLError` still populates both from the request it was raised for.
</file context>
Suggested change
That clause is the one the SDK recommends for the three authentication codes, since each of them can arrive either on a real 401 or 403 or inside a GraphQL `errors` array, so it is exactly where the attributes had to exist. They join `code`, `http_status`, `extensions` and `errors`, which were already declared there for the same reason.
That clause is the one the SDK recommends for the three authentication codes, since each of them can arrive either on a real 401 or 403 or inside a GraphQL `errors` array, so it is exactly where the attributes had to exist. They join `code`, `http_status`, `extensions` and `errors`, declared alongside them on the new `ApiError` base.


`None` means the failed request was not recorded on the exception rather than that there was no query: an authentication failure is observed at the transport, which has neither to hand. Nothing that reads these attributes today changes - `GraphQLError` still populates both from the request it was raised for.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P3: The GraphQL transport has query and variables in scope when it handles a 401; the auth-error factory simply receives only the response. Say the factory does not attach request details rather than claiming the transport has neither to hand.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At changelog/+api-error-carries-the-request.added.md, line 12:

<comment>The GraphQL transport has `query` and `variables` in scope when it handles a 401; the auth-error factory simply receives only the response. Say the factory does not attach request details rather than claiming the transport has neither to hand.</comment>

<file context>
@@ -0,0 +1,12 @@
+
+That clause is the one the SDK recommends for the three authentication codes, since each of them can arrive either on a real 401 or 403 or inside a GraphQL `errors` array, so it is exactly where the attributes had to exist. They join `code`, `http_status`, `extensions` and `errors`, which were already declared there for the same reason.
+
+`None` means the failed request was not recorded on the exception rather than that there was no query: an authentication failure is observed at the transport, which has neither to hand. Nothing that reads these attributes today changes - `GraphQLError` still populates both from the request it was raised for.
</file context>
Suggested change
`None` means the failed request was not recorded on the exception rather than that there was no query: an authentication failure is observed at the transport, which has neither to hand. Nothing that reads these attributes today changes - `GraphQLError` still populates both from the request it was raised for.
`None` means the failed request was not recorded on the exception rather than that there was no query: the authentication factory receives only the response, so it does not attach request details. Nothing that reads these attributes today changes - `GraphQLError` still populates both from the request it was raised for.

1 change: 1 addition & 0 deletions changelog/+api-token-no-relogin-retry.fixed.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
A client authenticating with an API token no longer replays a request after a 401 that reports an expired token. Only a client configured with a username and password can obtain a new token, so on the request paths that retry automatically the retry sent the same rejected token a second time, doubling the cost of the failure. Streaming downloads never retried and are unaffected.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P3: _get_streaming retries transient stream-initiation failures, so this unqualified claim is inaccurate. Say streaming downloads do not use the expired-token re-login retry instead.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At changelog/+api-token-no-relogin-retry.fixed.md, line 1:

<comment>`_get_streaming` retries transient stream-initiation failures, so this unqualified claim is inaccurate. Say streaming downloads do not use the expired-token re-login retry instead.</comment>

<file context>
@@ -0,0 +1 @@
+A client authenticating with an API token no longer replays a request after a 401 that reports an expired token. Only a client configured with a username and password can obtain a new token, so on the request paths that retry automatically the retry sent the same rejected token a second time, doubling the cost of the failure. Streaming downloads never retried and are unaffected.
</file context>
Suggested change
A client authenticating with an API token no longer replays a request after a 401 that reports an expired token. Only a client configured with a username and password can obtain a new token, so on the request paths that retry automatically the retry sent the same rejected token a second time, doubling the cost of the failure. Streaming downloads never retried and are unaffected.
A client authenticating with an API token no longer replays a request after a 401 that reports an expired token. Only a client configured with a username and password can obtain a new token, so on the request paths that retry automatically the retry sent the same rejected token a second time, doubling the cost of the failure. Streaming downloads do not use this expired-token re-login retry and are unaffected.

5 changes: 5 additions & 0 deletions changelog/+catalogued-error-messages.changed.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
A failure the server's error catalogue describes now carries a message naming that code and the server's own words, in place of the query text. `GraphQLError` previously rendered as `An error occurred while executing the GraphQL Query <query>, <errors>` and now reads `UNIQUENESS_VIOLATION: Node of kind TestPerson already has name 'John'`; `AuthenticationError` reads `AUTHENTICATION_REQUIRED: <reason>`. On both transports the code names the first error only, which is the one that determines the exception raised; the complete list stays on `exc.errors`, and the query and variables stay on `exc.query` and `exc.variables`.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P3: "On both transports ... the one that determines the exception raised" is not true for the authentication transport: authentication_error_from_response (infrahub_sdk/exceptions/factory.py) always raises AuthenticationError, whatever code the first error carries — the class is fixed by the HTTP status, and the code only names the message. Only the GraphQL path resolves the raised class from the first error's code. Reword so the sentence does not promise code-based dispatch on REST.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At changelog/+catalogued-error-messages.changed.md, line 1:

<comment>"On both transports ... the one that determines the exception raised" is not true for the authentication transport: `authentication_error_from_response` (infrahub_sdk/exceptions/factory.py) always raises `AuthenticationError`, whatever code the first error carries — the class is fixed by the HTTP status, and the code only names the message. Only the GraphQL path resolves the raised class from the first error's code. Reword so the sentence does not promise code-based dispatch on REST.</comment>

<file context>
@@ -0,0 +1,5 @@
+A failure the server's error catalogue describes now carries a message naming that code and the server's own words, in place of the query text. `GraphQLError` previously rendered as `An error occurred while executing the GraphQL Query <query>, <errors>` and now reads `UNIQUENESS_VIOLATION: Node of kind TestPerson already has name 'John'`; `AuthenticationError` reads `AUTHENTICATION_REQUIRED: <reason>`. On both transports the code names the first error only, which is the one that determines the exception raised; the complete list stays on `exc.errors`, and the query and variables stay on `exc.query` and `exc.variables`.
+
+A failure the catalogue does **not** describe keeps today's message exactly, query text and full error list included, as does one of the lookup-miss classes raised without a server behind it. That covers a server predating the catalogue, an error carrying no `extensions`, and, since a current server codes every error it reports, anything the server coded `UNDEFINED_ERROR`. `exc.code` is readable in every case, so it is not the test for which message form you are holding: `code_names_the_failure(exc.code)`, importable from `infrahub_sdk.exceptions`, is.
</file context>
Suggested change
A failure the server's error catalogue describes now carries a message naming that code and the server's own words, in place of the query text. `GraphQLError` previously rendered as `An error occurred while executing the GraphQL Query <query>, <errors>` and now reads `UNIQUENESS_VIOLATION: Node of kind TestPerson already has name 'John'`; `AuthenticationError` reads `AUTHENTICATION_REQUIRED: <reason>`. On both transports the code names the first error only, which is the one that determines the exception raised; the complete list stays on `exc.errors`, and the query and variables stay on `exc.query` and `exc.variables`.
On the GraphQL transport the code comes from the first error, which is the one the raised class is resolved from; on the authentication transport the exception class is fixed by the transport and that first error only names the message. In both cases the complete list stays on `exc.errors`, and the query and variables stay on `exc.query` and `exc.variables`.


A failure the catalogue does **not** describe keeps today's message exactly, query text and full error list included, as does one of the lookup-miss classes raised without a server behind it. That covers a server predating the catalogue, an error carrying no `extensions`, and, since a current server codes every error it reports, anything the server coded `UNDEFINED_ERROR`. `exc.code` is readable in every case, so it is not the test for which message form you are holding: `code_names_the_failure(exc.code)`, importable from `infrahub_sdk.exceptions`, is.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2: This fallback description applies the GraphQL query-and-error-list format to every failure, but an auth failure without extensions keeps its auth-specific message (joined messages, REST detail, or HTTP status) and has no query attached. Limit the query/full-list claim to GraphQL failures and state separately that auth fallbacks remain auth-specific.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At changelog/+catalogued-error-messages.changed.md, line 3:

<comment>This fallback description applies the GraphQL query-and-error-list format to every failure, but an auth failure without extensions keeps its auth-specific message (joined messages, REST detail, or HTTP status) and has no query attached. Limit the query/full-list claim to GraphQL failures and state separately that auth fallbacks remain auth-specific.</comment>

<file context>
@@ -0,0 +1,5 @@
+A failure the server's error catalogue describes now carries a message naming that code and the server's own words, in place of the query text. `GraphQLError` previously rendered as `An error occurred while executing the GraphQL Query <query>, <errors>` and now reads `UNIQUENESS_VIOLATION: Node of kind TestPerson already has name 'John'`; `AuthenticationError` reads `AUTHENTICATION_REQUIRED: <reason>`. On both transports the code names the first error only, which is the one that determines the exception raised; the complete list stays on `exc.errors`, and the query and variables stay on `exc.query` and `exc.variables`.
+
+A failure the catalogue does **not** describe keeps today's message exactly, query text and full error list included, as does one of the lookup-miss classes raised without a server behind it. That covers a server predating the catalogue, an error carrying no `extensions`, and, since a current server codes every error it reports, anything the server coded `UNDEFINED_ERROR`. `exc.code` is readable in every case, so it is not the test for which message form you are holding: `code_names_the_failure(exc.code)`, importable from `infrahub_sdk.exceptions`, is.
+
+In `infrahubctl`, a described failure is now reported by its code and message rather than prefixed with `Authentication failure:` or rendered as a bare error list. An undescribed one renders exactly as before, so a GraphQL validation error still shows the line and column it failed on. Code matching on any of these strings should branch on `exc.code` instead.
</file context>
Suggested change
A failure the catalogue does **not** describe keeps today's message exactly, query text and full error list included, as does one of the lookup-miss classes raised without a server behind it. That covers a server predating the catalogue, an error carrying no `extensions`, and, since a current server codes every error it reports, anything the server coded `UNDEFINED_ERROR`. `exc.code` is readable in every case, so it is not the test for which message form you are holding: `code_names_the_failure(exc.code)`, importable from `infrahub_sdk.exceptions`, is.
An undescribed GraphQL failure keeps today's message exactly, including the query text and full error list. An authentication failure keeps its auth-specific fallback message, and a lookup-miss class raised without a server behind it keeps its existing message. GraphQL fallbacks cover a server predating the catalogue, an error carrying no `extensions`, and a current server's `UNDEFINED_ERROR`. `exc.code` is readable in every case, so it is not the test for which message form you are holding: `code_names_the_failure(exc.code)`, importable from `infrahub_sdk.exceptions`, is.


In `infrahubctl`, a described failure is now reported by its code and message rather than prefixed with `Authentication failure:` or rendered as a bare error list. An undescribed one renders exactly as before, so a GraphQL validation error still shows the line and column it failed on. Code matching on any of these strings should branch on `exc.code` instead.
1 change: 1 addition & 0 deletions changelog/+ctl-error-markup-escaping.fixed.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
`infrahubctl` no longer deletes bracketed text from an error message. A message such as `The requested branch was not found on the server [main]` was printed without the branch name, because rich read the brackets as a style tag. Everything the shared error handler prints is now escaped, including the traceback the fallback branch emits. Affects branch, schema, and node lookup misses, authentication failures, HTTP transport failures, and the fallback handler.
1 change: 1 addition & 0 deletions changelog/+ctl-graphql-error-output.fixed.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
`infrahubctl` no longer exits non-zero with no output when a GraphQL failure arrives in a shape the SDK cannot read as an error envelope. `infrahubctl run`, `infrahubctl validate graphql-query`, and every command wrapped by the shared error handler now fall back to printing the exception's message, which keeps the server's payload verbatim.
5 changes: 5 additions & 0 deletions changelog/+error-catalogue-envelope.changed.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
Authentication failures now surface the server's error envelope. `AuthenticationError` and `GraphQLError` share a new `ApiError` base carrying `code`, `http_status`, `extensions`, and `errors`, so a caller can branch on the server's catalogue code instead of matching on message text.

A 401 or 403 whose body the SDK cannot read as an error envelope now raises `AuthenticationError` carrying the best reason available: the REST API's bare `detail` string where the body has one, and otherwise the plain status. Previously the same responses raised `JsonDecodeError` (or, from the object store and file handler, a raw `json.JSONDecodeError`) when the body was not JSON, and `TypeError` when the body carried an `errors` array whose entries had no `message`. A body that was JSON but carried no `errors` key raised `AuthenticationError` with its generic default message, dropping the status the server sent. Code that catches those types around `object_store`, `file_handler`, or a client request should catch `AuthenticationError` instead.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P3: The final sentence of this paragraph inaccurately describes the old behavior for a JSON body with no errors key: that only applied to the client request path. The old object_store.py and file_handler.py code read errors = response.get("errors") without a default and then iterated it, so that shape raised TypeError ('NoneType' object is not iterable), not AuthenticationError — unlike the old client code, which used response.get("errors", []) and so landed in AuthenticationError("") with the generic default message. Since the changelog elsewhere distinguishes per-site behavior (JsonDecodeError vs json.JSONDecodeError) and tells callers what to catch at each site, this sentence should attribute the no-errors-key outcome to the client request path only, or state the object-store/file-handler TypeError.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At changelog/+error-catalogue-envelope.changed.md, line 3:

<comment>The final sentence of this paragraph inaccurately describes the old behavior for a JSON body with no `errors` key: that only applied to the client request path. The old `object_store.py` and `file_handler.py` code read `errors = response.get("errors")` without a default and then iterated it, so that shape raised `TypeError` ('NoneType' object is not iterable), not `AuthenticationError` — unlike the old client code, which used `response.get("errors", [])` and so landed in `AuthenticationError("")` with the generic default message. Since the changelog elsewhere distinguishes per-site behavior (JsonDecodeError vs json.JSONDecodeError) and tells callers what to catch at each site, this sentence should attribute the no-`errors`-key outcome to the client request path only, or state the object-store/file-handler TypeError.</comment>

<file context>
@@ -0,0 +1,5 @@
+Authentication failures now surface the server's error envelope. `AuthenticationError` and `GraphQLError` share a new `ApiError` base carrying `code`, `http_status`, `extensions`, and `errors`, so a caller can branch on the server's catalogue code instead of matching on message text.
+
+A 401 or 403 whose body the SDK cannot read as an error envelope now raises `AuthenticationError` carrying the best reason available: the REST API's bare `detail` string where the body has one, and otherwise the plain status. Previously the same responses raised `JsonDecodeError` (or, from the object store and file handler, a raw `json.JSONDecodeError`) when the body was not JSON, and `TypeError` when the body carried an `errors` array whose entries had no `message`. A body that was JSON but carried no `errors` key raised `AuthenticationError` with its generic default message, dropping the status the server sent. Code that catches those types around `object_store`, `file_handler`, or a client request should catch `AuthenticationError` instead.
+
+`from infrahub_sdk.exceptions import *` now yields exactly the exception classes. It previously also carried whatever the module imported for its own annotations, such as `Mapping` and `Any`. Every exception class keeps its name and its import path.
</file context>
Suggested change
A 401 or 403 whose body the SDK cannot read as an error envelope now raises `AuthenticationError` carrying the best reason available: the REST API's bare `detail` string where the body has one, and otherwise the plain status. Previously the same responses raised `JsonDecodeError` (or, from the object store and file handler, a raw `json.JSONDecodeError`) when the body was not JSON, and `TypeError` when the body carried an `errors` array whose entries had no `message`. A body that was JSON but carried no `errors` key raised `AuthenticationError` with its generic default message, dropping the status the server sent. Code that catches those types around `object_store`, `file_handler`, or a client request should catch `AuthenticationError` instead.
A body that was JSON but carried no `errors` key raised `AuthenticationError` with its generic default message from a client request (the object store and file handler raised `TypeError` there, iterating the absent `errors` key), dropping the status the server sent.


`from infrahub_sdk.exceptions import *` now yields exactly the exception classes. It previously also carried whatever the module imported for its own annotations, such as `Mapping` and `Any`. Every exception class keeps its name and its import path.
5 changes: 5 additions & 0 deletions changelog/+except-graphql-error-broadened.changed.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
`NodeNotFoundError`, `BranchNotFoundError`, and `SchemaNotFoundError` now descend from `GraphQLError`, so that one class covers a lookup miss however it arose: reported by the server, decided by the SDK, or turned from a REST 404.

An `except GraphQLError` clause therefore also catches lookup misses that involved no GraphQL request at all. Code that relied on those escaping such a clause should catch the specific class ahead of it, as an ordered `except` ladder already must. Each class keeps its name, its constructor, and the message it produces when no server reported the failure, and `errors`, `query`, and `variables` are now readable on every one of them rather than missing on a client-side raise.

`NodeInvalidError` inherits the re-rooting but not the adopted code: it means a node of the wrong kind rather than a lookup miss, so `NodeInvalidError.CODE` is `None` where `NodeNotFoundError.CODE` is `NODE_NOT_FOUND`.
1 change: 1 addition & 0 deletions changelog/+file-handler-404-body.fixed.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
A file download answered with HTTP 404 now raises `NodeNotFoundError` whatever the response body contains. Previously a body that was not a JSON object - an HTML error page from an intermediary, an empty body, or a JSON array - escaped as a raw `json.JSONDecodeError` or `AttributeError` instead.
7 changes: 7 additions & 0 deletions changelog/+graphql-error-survives-serialisation.fixed.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
`GraphQLError` and every exception under it now survive `pickle` and `copy.deepcopy` intact. An exception that crosses a process boundary - a task queue, a parallel test runner, a process pool - previously came back wrong in one of two ways, because Python rebuilds an exception by calling its class with the message as a lone positional argument.

`GraphQLError` takes the server's error list first, so that call filed the message under `errors` and the message a caller read came back as the placeholder built from it: `An error occurred while executing the GraphQL Query None, UNIQUENESS_VIOLATION: ...` in place of the real one. The per-code classes take their payload fields as required keyword arguments, so the same call raised `TypeError: __init__() takes 1 positional argument but 2 were given` and the original failure was lost entirely.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P3: The quoted TypeError: __init__() takes 1 positional argument but 2 were given does not describe what the current per-code classes raise. They declare def __init__(self, *, node_kind: str, fields: list[str], ...) (catalogue.py, UniquenessViolationError), so reconstructing one with UniquenessViolationError(<message>) raises TypeError: ... missing 2 required keyword-only arguments: 'node_kind' and 'fields' — only one positional is ever passed, so "2 were given" cannot occur. Quote the accurate message (or drop the literal quote and keep "raised a TypeError"), since the changelog otherwise describes this behaviour precisely.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At changelog/+graphql-error-survives-serialisation.fixed.md, line 3:

<comment>The quoted `TypeError: __init__() takes 1 positional argument but 2 were given` does not describe what the current per-code classes raise. They declare `def __init__(self, *, node_kind: str, fields: list[str], ...)` (catalogue.py, UniquenessViolationError), so reconstructing one with `UniquenessViolationError(<message>)` raises `TypeError: ... missing 2 required keyword-only arguments: 'node_kind' and 'fields'` — only one positional is ever passed, so "2 were given" cannot occur. Quote the accurate message (or drop the literal quote and keep "raised a TypeError"), since the changelog otherwise describes this behaviour precisely.</comment>

<file context>
@@ -0,0 +1,7 @@
+`GraphQLError` and every exception under it now survive `pickle` and `copy.deepcopy` intact. An exception that crosses a process boundary - a task queue, a parallel test runner, a process pool - previously came back wrong in one of two ways, because Python rebuilds an exception by calling its class with the message as a lone positional argument.
+
+`GraphQLError` takes the server's error list first, so that call filed the message under `errors` and the message a caller read came back as the placeholder built from it: `An error occurred while executing the GraphQL Query None, UNIQUENESS_VIOLATION: ...` in place of the real one. The per-code classes take their payload fields as required keyword arguments, so the same call raised `TypeError: __init__() takes 1 positional argument but 2 were given` and the original failure was lost entirely.
+
+Both are fixed at the root of the GraphQL branch, so the type, the message, the payload attributes, `code`, `http_status`, `extensions`, `errors`, `query` and `variables` all come back as they went in, and an `except` clause still catches a restored exception by its own class.
</file context>
Suggested change
`GraphQLError` takes the server's error list first, so that call filed the message under `errors` and the message a caller read came back as the placeholder built from it: `An error occurred while executing the GraphQL Query None, UNIQUENESS_VIOLATION: ...` in place of the real one. The per-code classes take their payload fields as required keyword arguments, so the same call raised `TypeError: __init__() takes 1 positional argument but 2 were given` and the original failure was lost entirely.
...so the same call raised `TypeError: __init__() missing 2 required keyword-only arguments: 'node_kind' and 'fields'` and the original failure was lost entirely.


Both are fixed at the root of the GraphQL branch, so the type, the message, the payload attributes, `code`, `http_status`, `extensions`, `errors`, `query` and `variables` all come back as they went in, and an `except` clause still catches a restored exception by its own class.

`NodeNotFoundError`, `BranchNotFoundError` and `SchemaNotFoundError` were affected more quietly: their attributes and `str()` came back correct, but `exc.args` was rebuilt as the class's own default sentence rather than the message the exception was raised with. `AuthenticationError`, whose constructor takes the message first, was the only one unaffected.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P3: The default pickle round-trip does not preserve str() for BranchNotFoundError or SchemaNotFoundError: reconstruction treats the old message as identifier, generating different args, which str() returns. Narrow this claim to NodeNotFoundError or document the changed output.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At changelog/+graphql-error-survives-serialisation.fixed.md, line 7:

<comment>The default pickle round-trip does not preserve `str()` for `BranchNotFoundError` or `SchemaNotFoundError`: reconstruction treats the old message as `identifier`, generating different `args`, which `str()` returns. Narrow this claim to `NodeNotFoundError` or document the changed output.</comment>

<file context>
@@ -0,0 +1,7 @@
+
+Both are fixed at the root of the GraphQL branch, so the type, the message, the payload attributes, `code`, `http_status`, `extensions`, `errors`, `query` and `variables` all come back as they went in, and an `except` clause still catches a restored exception by its own class.
+
+`NodeNotFoundError`, `BranchNotFoundError` and `SchemaNotFoundError` were affected more quietly: their attributes and `str()` came back correct, but `exc.args` was rebuilt as the class's own default sentence rather than the message the exception was raised with. `AuthenticationError`, whose constructor takes the message first, was the only one unaffected.
</file context>
Suggested change
`NodeNotFoundError`, `BranchNotFoundError` and `SchemaNotFoundError` were affected more quietly: their attributes and `str()` came back correct, but `exc.args` was rebuilt as the class's own default sentence rather than the message the exception was raised with. `AuthenticationError`, whose constructor takes the message first, was the only one unaffected.
+`NodeNotFoundError` was affected more quietly: its attributes and `str()` came back correct, but `exc.args` was rebuilt as the class's default sentence rather than the message it was raised with. `BranchNotFoundError` and `SchemaNotFoundError` also rebuilt `args` from the message-as-identifier, changing their default `str()` output. `AuthenticationError`, whose constructor takes the message first, was the only one unaffected.

1 change: 1 addition & 0 deletions changelog/+node-not-found-identifier-widened.changed.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
`NodeNotFoundError.identifier` is now annotated `Mapping[str, list[str]] | str`. The SDK already raised it with a plain string to name a missing file, so this documents behaviour that was always there; no runtime behaviour changes and no existing caller needs updating.
1 change: 1 addition & 0 deletions changelog/+relogin-non-object-body.fixed.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Fixed an `AttributeError` escaping the client when a 401 response carried a body that was valid JSON but not an object, such as the bare array or string a proxy or gateway may return. The silent token refresh now treats any body it cannot read as an envelope as carrying no refresh signal, and the request surfaces `AuthenticationError` as it does for every other unreadable 401.
22 changes: 22 additions & 0 deletions changelog/+typed-per-code-exceptions.added.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
Codes in Infrahub's error catalogue now raise an exception class of their 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. `NODE_NOT_FOUND`, `BRANCH_NOT_FOUND` and `SCHEMA_NOT_FOUND` reach the classes the SDK already had for them, listed under Changed.

`exc.http_status` reads the status the envelope declares. Where the envelope omits one, a generated class supplies the status the catalogue gives its code; the three lookup-miss classes declare none, since they are also raised with no server behind them, and stay `None` there.

`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.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2: Pre-catalogue GraphQL envelopes can carry numeric extensions.code, which the SDK exposes as exc.code is None; this sentence incorrectly promises a readable code. Distinguish unknown string codes from legacy numeric or missing codes.

(Based on your team's feedback about GraphQL error-code shapes.)

View Feedback

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At changelog/+typed-per-code-exceptions.added.md, line 20:

<comment>Pre-catalogue GraphQL envelopes can carry numeric `extensions.code`, which the SDK exposes as `exc.code is None`; this sentence incorrectly promises a readable code. Distinguish unknown string codes from legacy numeric or missing codes.

(Based on your team's feedback about GraphQL error-code shapes.) </comment>

<file context>
@@ -0,0 +1,22 @@
+
+`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.
</file context>


`docs/python-sdk/topics/error_handling` covers the hierarchy, the attributes readable on a caught error, and the cross-version guarantees.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P3: The docs reference docs/python-sdk/topics/error_handling does not resolve to any file in this repo: the page added in this PR lives at docs/docs/python-sdk/topics/error_handling.mdx. Point the sentence at the real path (or a docs.infrahub.app link) so a reader can open it.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At changelog/+typed-per-code-exceptions.added.md, line 22:

<comment>The docs reference `docs/python-sdk/topics/error_handling` does not resolve to any file in this repo: the page added in this PR lives at `docs/docs/python-sdk/topics/error_handling.mdx`. Point the sentence at the real path (or a docs.infrahub.app link) so a reader can open it.</comment>

<file context>
@@ -0,0 +1,22 @@
+
+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.
</file context>
Suggested change
`docs/python-sdk/topics/error_handling` covers the hierarchy, the attributes readable on a caught error, and the cross-version guarantees.
`[docs/python-sdk/topics/error_handling](https://docs.infrahub.app/python-sdk/topics/error_handling)` covers the hierarchy, the attributes readable on a caught error, and the cross-version guarantees.

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.
Loading
Loading