Note: this issue was filed by an over-eager agent with too many MCP tools.
Context
PR coder/coder#27913 fixed a bug where a Bedrock credential resolution failure was incorrectly retried as a generic 500. The fix widened several signal checks in chaterror.Classify to read combinedText (the merged transport wrapper + structured response body) instead of lower (just the wrapper).
This exposed a structural issue: the classifier has two parallel text sources (lower from err.Error() and structured.detail from ProviderError.ResponseBody) and signal checks inconsistently pick which to match against. PR coder/coder#27913 widened the body-only signals, but the underlying design is fragile.
Problems
-
Two-source inconsistency: Some signals check lower, some check combinedText, and it is unclear which uses which without reading each line. New signal checks can silently regress to lower-only.
-
Pattern sprawl: Too many string patterns across too many signal lists (overloadedPatterns, authStrongPatterns, configPatterns, timeoutPatterns, etc.) without documentation of which provider incident motivated each pattern.
-
Signal/display coupling: structured.detail (used for signal matching) is the first line of the response body only (providerErrorResponseMessage truncates at the first newline). If the useful signal text appears on line 2+, it is missed. The signal source and the user-facing detail display are the same string; they should be decoupled.
Proposed approach
Data-flow normalization, not a rule engine:
- One
gatherEvidence step at the top of Classify that produces a single struct: lowercased combined text (transport error + full structured body, not just first line), status code, and any structured provider fields.
- Every signal check reads only from the evidence struct. No signal touches raw inputs.
- Keep the prioritized rule ordering as plain Go code. Do not build a pattern-matching DSL or data-driven rule table.
- Decouple signal matching text from user-facing detail display text.
Non-goals
- No new error taxonomy.
- No change to
Classify's public signature.
- No pattern DSL or data-driven rule engine.
Test strategy
Convert signal tests to a matrix that programmatically asserts each source-agnostic signal classifies correctly when the pattern arrives via body-only and via transport-only. This makes "forgot to check the other source" impossible to reintroduce silently.
Related
Context
PR coder/coder#27913 fixed a bug where a Bedrock credential resolution failure was incorrectly retried as a generic 500. The fix widened several signal checks in
chaterror.Classifyto readcombinedText(the merged transport wrapper + structured response body) instead oflower(just the wrapper).This exposed a structural issue: the classifier has two parallel text sources (
lowerfromerr.Error()andstructured.detailfromProviderError.ResponseBody) and signal checks inconsistently pick which to match against. PR coder/coder#27913 widened the body-only signals, but the underlying design is fragile.Problems
Two-source inconsistency: Some signals check
lower, some checkcombinedText, and it is unclear which uses which without reading each line. New signal checks can silently regress tolower-only.Pattern sprawl: Too many string patterns across too many signal lists (
overloadedPatterns,authStrongPatterns,configPatterns,timeoutPatterns, etc.) without documentation of which provider incident motivated each pattern.Signal/display coupling:
structured.detail(used for signal matching) is the first line of the response body only (providerErrorResponseMessagetruncates at the first newline). If the useful signal text appears on line 2+, it is missed. The signal source and the user-facing detail display are the same string; they should be decoupled.Proposed approach
Data-flow normalization, not a rule engine:
gatherEvidencestep at the top ofClassifythat produces a single struct: lowercased combined text (transport error + full structured body, not just first line), status code, and any structured provider fields.Non-goals
Classify's public signature.Test strategy
Convert signal tests to a matrix that programmatically asserts each source-agnostic signal classifies correctly when the pattern arrives via body-only and via transport-only. This makes "forgot to check the other source" impossible to reintroduce silently.
Related
coderd/x/chatd/chaterror/classify.go(the classifier)coderd/x/chatd/chaterror/signals.go(the pattern definitions)coderd/x/chatd/chaterror/provider_error.go(response body extraction)