feat: record and expose terminal upstream interception errors#26961
feat: record and expose terminal upstream interception errors#26961dannykopping wants to merge 2 commits into
Conversation
Docs preview📖 View docs preview for |
|
Warning This pull request is not mergeable via GitHub because a downstack PR is open. Once all requirements are satisfied, merge this PR as a stack on Graphite.
This stack of pull requests is managed by Graphite. Learn more about stacking. |
318142c to
a66498e
Compare
aa06e78 to
1b0a895
Compare
|
/coder-agents-review |
|
Chat: Review posted | View chat Review history
deep-review v0.9.0 | Round 1 | Last posted: Round 1, 13 findings (2 P2, 5 P3, 4 Nit, 2 Note), COMMENT. Review Finding inventoryFindings
Contested and acknowledged(none) Round logRound 1Panel. 0 P0, 2 P2, 5 P3, 4 Nit, 2 Note. Reviewed against a66498e..1b0a895. About deep-reviewCRF = Coder Review Finding (P0-P4, Nit, Note)
|
1b0a895 to
66f667d
Compare
a66498e to
8b5b3e9
Compare
There was a problem hiding this comment.
The error categorization architecture is well-designed. The three-layer dispatch (domain in categorizeInterceptionError, provider-specific via CategorizeError, shared ErrorTypeFromStatus) is the right decomposition, and the narrow errorCategorizer interface keeps provider imports out of the bridge package. Test density at 64% covers every layer from unit through integration. The migration is safe (nullable columns, no rewrite), the proto optional fields are correct, and the enum/constant alignment is clean.
2 P2, 5 P3, 4 Nit.
The two P2s are related: the streaming interceptors lose the original key-pool error cause (returning the client-facing 502 ResponseError instead of wrapping the keyPoolErr), and the Copilot provider's CategorizeError only recognizes OpenAI-shaped errors, missing its own /v1/messages route's Anthropic-shaped errors. Both produce incorrect categorization for real failure modes.
"Same root cause, different recorded type. The test expectations document this intentionally, but the asymmetry is observable by API consumers filtering on error_type." (Knov)
Notes for the body (not inline): the truncation test at interception_error_internal_test.go:92 only exercises ASCII, so the multi-byte UTF-8 boundary behavior of strings.ToValidUTF8 is untested (the code is correct, the test just doesn't prove the interesting case). ErrorTypeFromStatus maps 408/413/422 to unknown; worth noting if the enum expands.
🤖 This review was automatically generated with Coder Agents.
Categorises the terminal error of a failed interception and persists it on the interception record, then surfaces it on the AI Gateway API. - Categorise into an enum (`bad_request`, `unauthorized`, `rate_limited`, `overloaded`, `server_error`, `unknown`), unwrapping the ResponseError envelope, the upstream Anthropic/OpenAI SDK errors, and key-pool exhaustion so blocking and streaming paths agree. - Thread the type and raw message through the recorder dRPC into the `aibridge_interceptions` row (optional proto fields; NULL on success). - Expose the error on the AI Gateway thread API from the root interception. *This PR was produced by opencode (agent) using the `anthropic/claude-opus-4-8` model, under human direction and review.*
8b5b3e9 to
af87435
Compare
66f667d to
761c8c8
Compare
- record streaming key-pool exhaustion as unauthorized (not server_error) by preserving the *keypool.Error on the messages/chatcompletions streaming paths; the client still receives the masked envelope - categorize Copilot's Anthropic-style /v1/messages errors - store an unrecognized error type as unknown instead of NULL, and cap the error message at the dRPC write boundary - nest the API error message under error_type to avoid a half-populated error on the response - add a timeout error type (HTTP 408 and context deadline; 413/422 map to bad_request) - integration test covers streaming; drop dead field
761c8c8 to
d8e72b3
Compare
Categorises the terminal error of a failed interception and persists it
on the interception record, then surfaces it on the AI Gateway API.
bad_request,unauthorized,rate_limited,overloaded,server_error,unknown), unwrappingthe ResponseError envelope, the upstream Anthropic/OpenAI SDK errors,
and key-pool exhaustion so blocking and streaming paths agree.
aibridge_interceptionsrow (optional proto fields; NULL on success).interception.
This PR was produced by opencode (agent) using the
anthropic/claude-opus-4-8model, under human direction and review.