Bound the exception taxonomy and prove its redaction #98
No reviewers
Labels
No labels
autonomy
async-consult
autonomy
epic
autonomy
headless
autonomy
live-collab
coherence-core
priority
P0
priority
P1
priority
P2
priority
P3
priority
P4
qa-fixture
role/ai
role/creator
role/design
role/director
role/engineer
role/exec
role/human
role/ops
role/qa
No milestone
No project
No assignees
1 participant
Notifications
Due date
No due date set.
Dependencies
No dependencies set
Reference
coilyco-flight-deck/agent-proxy!98
Loading…
Reference in a new issue
No description provided.
Delete branch "exception-taxonomy"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Closes #74.
What was actually wrong
The catch-all baseline emitted an exception event for every handled failure, and
record_erroralready setStatusCode.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()inapp/main.pyforwards itserr_typestraight through, so the client-facing error path was one careless call away from it.The taxonomy
ERROR_TAXONOMYinapp/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 keyerror.stage—request,dispatch,upstream,stream,queue,capture,trajectory,unknownclassify_errorcollapses anything outside the table tounclassified_errorand 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.pydoes an AST walk overapp/, collects every string literal passed torecord_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_failedto 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.pyandtests/test_error_spans.pymoved fromstatus.descriptiontoerror.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.typeis unchanged, so anything keyed on that is unaffected.Verification
pytest— 283 passed, 9 of them newruff/black/mypy— cleanpre-commit run --all-files— every hook passesdocs/exception-taxonomy.mddocuments the cardinality bound, the fields, the full code table, and alerting guidanceHuman 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.