Bound the exception taxonomy and prove its redaction #98

Merged
coilysiren merged 1 commit from exception-taxonomy into main 2026-08-12 11:29:00 +00:00
Member

Closes #74.

What was actually wrong

The catch-all baseline emitted an exception event for every handled failure, and record_error already set StatusCode.ERROR (proven in #71). But the error code was whatever string the call site passed. Every current caller happens to pass a literal — I checked all 14 sites — so the set was closed by convention with nothing enforcing it.

That is a latent double failure. A future dynamic value (an upstream message, a model name, a path) would widen SigNoz grouping cardinality and carry caller data into a span attribute in the same move. _error() in app/main.py forwards its err_type straight through, so the client-facing error path was one careless call away from it.

The taxonomy

ERROR_TAXONOMY in app/obs.py: 13 codes across 8 stages, each mapping to a stage tag and a fixed human-readable summary.

  • error.type — the stable machine key, and the SigNoz grouping key
  • error.stagerequest, dispatch, upstream, stream, queue, capture, trajectory, unknown
  • exception message and status description — the fixed human summary

classify_error collapses anything outside the table to unclassified_error and discards the requested value rather than passing it through. Recording the offending string would reintroduce exactly the unbounded attribute the taxonomy exists to prevent.

How the closed set stays closed

tests/test_exception_taxonomy.py does an AST walk over app/, collects every string literal passed to record_error(...), and fails if any is absent from the table. A new code has to be declared, not invented at a call site.

Mutation-verified: renaming queue_worker_failed to an undeclared code fails the test and names it in the assertion message. Reverted after the check.

Redaction is parametrized over hostile inputs — a credential (sk-live-…), a URL with a token, an email address, an internal host, and the empty string — asserting none reaches a span attribute, an exception field, or the status description.

Behaviour change worth reviewing

The exception message and span status description are now the human summary rather than the raw code — that is the issue's "improve human-readable wording while keeping messages closed-set and body-free". Existing assertions in tests/test_obs.py and tests/test_error_spans.py moved from status.description to error.type, which is the stable machine key and what SigNoz actually groups on.

If any SigNoz view or alert matches on the exception message text, it will need updating — the grouping key error.type is unchanged, so anything keyed on that is unaffected.

Verification

  • pytest — 283 passed, 9 of them new
  • ruff / black / mypy — clean
  • pre-commit run --all-files — every hook passes
  • docs/exception-taxonomy.md documents the cardinality bound, the fields, the full code table, and alerting guidance

Human checkpoint — not claimed

This issue is interactive. Its last acceptance criterion is a live verification recording one safe representative error in SigNoz after rollout. That needs a deployed build and a person looking at the tenant, so it is not marked done. Everything else in the acceptance list is covered.

Closes #74. ## What was actually wrong The catch-all baseline emitted an exception event for every handled failure, and `record_error` already set `StatusCode.ERROR` (proven in #71). But the **error code was whatever string the call site passed**. Every current caller happens to pass a literal — I checked all 14 sites — so the set was closed *by convention* with nothing enforcing it. That is a latent double failure. A future dynamic value (an upstream message, a model name, a path) would widen SigNoz grouping cardinality **and** carry caller data into a span attribute in the same move. `_error()` in `app/main.py` forwards its `err_type` straight through, so the client-facing error path was one careless call away from it. ## The taxonomy `ERROR_TAXONOMY` in `app/obs.py`: **13 codes across 8 stages**, each mapping to a stage tag and a fixed human-readable summary. - `error.type` — the stable machine key, and the SigNoz grouping key - `error.stage` — `request`, `dispatch`, `upstream`, `stream`, `queue`, `capture`, `trajectory`, `unknown` - exception message and status description — the fixed human summary `classify_error` collapses anything outside the table to `unclassified_error` and **discards the requested value** rather than passing it through. Recording the offending string would reintroduce exactly the unbounded attribute the taxonomy exists to prevent. ## How the closed set stays closed `tests/test_exception_taxonomy.py` does an AST walk over `app/`, collects every string literal passed to `record_error(...)`, and fails if any is absent from the table. A new code has to be declared, not invented at a call site. **Mutation-verified**: renaming `queue_worker_failed` to an undeclared code fails the test and names it in the assertion message. Reverted after the check. Redaction is parametrized over hostile inputs — a credential (`sk-live-…`), a URL with a token, an email address, an internal host, and the empty string — asserting none reaches a span attribute, an exception field, or the status description. ## Behaviour change worth reviewing The exception message and span status description are now the human summary rather than the raw code — that is the issue's *"improve human-readable wording while keeping messages closed-set and body-free"*. Existing assertions in `tests/test_obs.py` and `tests/test_error_spans.py` moved from `status.description` to `error.type`, which is the stable machine key and what SigNoz actually groups on. **If any SigNoz view or alert matches on the exception message text, it will need updating** — the grouping key `error.type` is unchanged, so anything keyed on that is unaffected. ## Verification - `pytest` — 283 passed, 9 of them new - `ruff` / `black` / `mypy` — clean - `pre-commit run --all-files` — every hook passes - `docs/exception-taxonomy.md` documents the cardinality bound, the fields, the full code table, and alerting guidance ## Human checkpoint — not claimed This issue is `interactive`. Its last acceptance criterion is a live verification recording one safe representative error in SigNoz after rollout. That needs a deployed build and a person looking at the tenant, so **it is not marked done**. Everything else in the acceptance list is covered.
Bound the exception taxonomy and prove its redaction
All checks were successful
ci / smoke (pull_request) Successful in 7s
ci / quality (pull_request) Successful in 21s
0a622c8005
The catch-all baseline emitted an exception for every handled failure, but the
error code was whatever string the call site passed. Every current caller
happens to pass a literal, so the set was closed by convention and nothing
enforced it - a future dynamic value would have widened SigNoz grouping
cardinality and carried caller data into a span attribute at the same time.

Adds ERROR_TAXONOMY: 13 codes across 8 stages, each mapping to a stage tag and
a fixed human-readable summary. classify_error collapses anything outside the
table to unclassified_error and discards the requested value rather than
passing it through, since an unbounded string on a span is the exact failure
being prevented.

Recorded spans now carry error.type as the stable grouping key and error.stage
as the bounded operational tag. The exception message and status description
become the human summary instead of the raw code, which is the wording
improvement the issue asked for while staying closed-set and body-free.

tests/test_exception_taxonomy.py walks every record_error literal in app/ with
an AST pass and fails when one is missing from the table, so a new code must be
declared rather than invented at a call site. Verified by mutation: an
undeclared code fails the test by name. Hostile inputs - a credential, a URL
with a token, an email, a host - are parametrized to prove none reaches a span
attribute, an exception field, or the status description.

Existing assertions moved from status.description to error.type, which is the
stable machine key and what SigNoz groups on.

Live SigNoz verification of one representative error after rollout is the
remaining human checkpoint and is not claimed here.

closes #74

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Co-authored-by: Kai Siren <coilysiren@gmail.com>
Co-authored-by: Claude <noreply@anthropic.com>
Sign in to join this conversation.
No reviewers
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference
coilyco-flight-deck/agent-proxy!98
No description provided.