mirror of
https://github.com/coder/coder.git
synced 2026-09-24 15:04:27 +08:00
fix(coderd/x/chatd): classify bedrock credential errors as non-retryable (#27913)
When a Bedrock provider is misconfigured without authentication methods, AWS credential resolution fails and AIBridge writes the error as a plain-text HTTP 500. The fantasy adapter captures the body text in `ProviderError.ResponseBody`, but `Error()` returns only the SDK transport wrapper, not the body. Signal patterns in `chaterror.Classify` checked only `err.Error()` (the wrapper), missing the useful text in `structured.detail` (the body). This caused permanent configuration errors to fall through to the generic 500 rule with `retryable=true`, making the chat worker retry up to 25 times. Introduce `combinedText` (merging the wrapper with `structured.detail`) and widen signal checks that have no dedicated status code to use it: overloaded, auth, config, usage limit, and timeout patterns. The deadline signal stays on `err.Error()` to avoid treating ambiguous body text as a local context deadline. Add a "resolve aws credentials" config pattern so credential resolution failures classify as config, not generic. > Generated by Coder Agents
This commit is contained in:
@@ -178,20 +178,22 @@ func Classify(err error) ClassifiedError {
|
||||
}
|
||||
|
||||
retryableHTTP2StreamReset, hasHTTP2StreamReset := classifyHTTP2StreamReset(err)
|
||||
providerDisabledMatch := containsAny(lower, providerDisabledPatterns...)
|
||||
// combinedText merges the transport wrapper text with the structured
|
||||
// provider response body so signal patterns in either are detected.
|
||||
// AI Bridge writes some failures as plain-text bodies that never reach
|
||||
// the transport wrapper, so the body can be the only signal regardless
|
||||
// of the class's nominal status code.
|
||||
combinedText := lower + "\n" + strings.ToLower(structured.detail)
|
||||
providerDisabledMatch := containsAny(combinedText, providerDisabledPatterns...)
|
||||
deadline := errors.Is(err, context.DeadlineExceeded) || strings.Contains(lower, "context deadline exceeded")
|
||||
overloadedMatch := statusCode == 529 || containsAny(lower, overloadedPatterns...)
|
||||
// Usage limits do not have a dedicated status code, so provider
|
||||
// response bodies can be the only reliable signal. Other classes
|
||||
// already have status-code signals or transport wrapper text.
|
||||
usageLimitText := lower + "\n" + strings.ToLower(structured.detail)
|
||||
usageLimitMatch := containsAny(usageLimitText, usageLimitAnyStatusPatterns...) ||
|
||||
(statusCode != 429 && containsAny(usageLimitText, usageLimitPatterns...))
|
||||
authStrong := statusCode == 401 || containsAny(lower, authStrongPatterns...)
|
||||
configMatch := containsAny(lower, configPatterns...)
|
||||
authWeak := statusCode == 403 || containsAny(lower, authWeakPatterns...)
|
||||
overloadedMatch := statusCode == 529 || containsAny(combinedText, overloadedPatterns...)
|
||||
usageLimitMatch := containsAny(combinedText, usageLimitAnyStatusPatterns...) ||
|
||||
(statusCode != 429 && containsAny(combinedText, usageLimitPatterns...))
|
||||
authStrong := statusCode == 401 || containsAny(combinedText, authStrongPatterns...)
|
||||
authWeak := statusCode == 403 || containsAny(combinedText, authWeakPatterns...)
|
||||
configMatch := containsAny(combinedText, configPatterns...)
|
||||
rateLimitMatch := statusCode == 429 || containsAny(lower, rateLimitPatterns...)
|
||||
timeoutPatternMatch := containsAny(lower, timeoutPatterns...)
|
||||
timeoutPatternMatch := containsAny(combinedText, timeoutPatterns...)
|
||||
if hasHTTP2StreamReset && !retryableHTTP2StreamReset {
|
||||
// A typed HTTP/2 stream error gives us the reset code. Trust it
|
||||
// over broader string fallbacks so protocol bugs do not retry.
|
||||
|
||||
@@ -1669,6 +1669,106 @@ func TestClassify_MissingKeyPreClassified(t *testing.T) {
|
||||
)
|
||||
}
|
||||
|
||||
func TestClassify_BedrockCredentialResolutionDeadline(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
// AIBridge writes credential resolution failures as a plain-text 500.
|
||||
// The fantasy adapter's Error() returns only the SDK transport wrapper;
|
||||
// the useful text lives solely in the response body (structured.detail).
|
||||
// The "resolve aws credentials" pattern in configPatterns matches on
|
||||
// the body, classifying this as a non-retryable config error instead
|
||||
// of a retryable generic 500.
|
||||
classified := chaterror.Classify(testProviderError(
|
||||
`POST "https://bedrock-mantle.us-east-1.api.aws/anthropic/v1/messages": 500 Internal Server Error`,
|
||||
500,
|
||||
nil,
|
||||
testPlainDump("text/plain", "create anthropic client: resolve AWS credentials: "+
|
||||
"failed to refresh cached credentials, no EC2 IMDS role found, "+
|
||||
"operation error ec2imds: GetMetadata, canceled, context deadline exceeded"),
|
||||
))
|
||||
|
||||
require.Equal(t, codersdk.ChatErrorKindConfig, classified.Kind)
|
||||
require.False(t, classified.Retryable)
|
||||
require.Equal(t, 500, classified.StatusCode)
|
||||
require.Contains(t, classified.Detail, "context deadline exceeded")
|
||||
}
|
||||
|
||||
func TestClassify_BedrockBodyOnlySignals(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
// AIBridge returns plain-text 500 bodies for all client creation
|
||||
// failures. The fantasy adapter's Error() returns only the transport
|
||||
// wrapper, so the useful text lives solely in the response body.
|
||||
// Signal patterns must check combinedText (wrapper + body) for these
|
||||
// to classify as the correct kind instead of a retryable generic 500.
|
||||
tests := []struct {
|
||||
name string
|
||||
body string
|
||||
wantKind codersdk.ChatErrorKind
|
||||
wantRet bool
|
||||
}{
|
||||
{
|
||||
name: "OverloadedInBody",
|
||||
body: "upstream provider is overloaded, please retry",
|
||||
wantKind: codersdk.ChatErrorKindOverloaded,
|
||||
wantRet: true,
|
||||
},
|
||||
{
|
||||
name: "AuthInBody",
|
||||
body: "unauthorized: the security token included in the request is invalid",
|
||||
wantKind: codersdk.ChatErrorKindAuth,
|
||||
wantRet: false,
|
||||
},
|
||||
{
|
||||
name: "ConfigInBody",
|
||||
body: "create bedrock client: invalid model identifier for this region",
|
||||
wantKind: codersdk.ChatErrorKindConfig,
|
||||
wantRet: false,
|
||||
},
|
||||
{
|
||||
name: "TimeoutInBody",
|
||||
body: "upstream gateway timed out waiting for a response",
|
||||
wantKind: codersdk.ChatErrorKindTimeout,
|
||||
wantRet: true,
|
||||
},
|
||||
}
|
||||
for _, tt := range tests {
|
||||
t.Run(tt.name, func(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
classified := chaterror.Classify(testProviderError(
|
||||
`POST "https://bedrock-mantle.us-east-1.api.aws/anthropic/v1/messages": 500 Internal Server Error`,
|
||||
500,
|
||||
nil,
|
||||
testPlainDump("text/plain", tt.body),
|
||||
))
|
||||
require.Equal(t, tt.wantKind, classified.Kind, "kind")
|
||||
require.Equal(t, tt.wantRet, classified.Retryable, "retryable")
|
||||
require.Equal(t, 500, classified.StatusCode)
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
func TestClassify_ProviderDisabledBodyOnly(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
// AIBridge writes the provider_disabled sentinel as a plain-text 503
|
||||
// body. The fantasy adapter's Error() returns only the transport
|
||||
// wrapper, so the sentinel lives solely in the response body.
|
||||
// Without checking combinedText, the 503 status code would match the
|
||||
// timeout rule and classify as retryable instead of non-retryable.
|
||||
classified := chaterror.Classify(testProviderError(
|
||||
`POST "https://bedrock-mantle.us-east-1.api.aws/anthropic/v1/messages": 503 Service Unavailable`,
|
||||
503,
|
||||
nil,
|
||||
testPlainDump("text/plain", `provider_disabled: AI provider "anthropic" is disabled`),
|
||||
))
|
||||
|
||||
require.Equal(t, codersdk.ChatErrorKindProviderDisabled, classified.Kind)
|
||||
require.False(t, classified.Retryable)
|
||||
require.Equal(t, 503, classified.StatusCode)
|
||||
}
|
||||
|
||||
func testProviderError(
|
||||
message string,
|
||||
statusCode int,
|
||||
|
||||
@@ -84,6 +84,7 @@ var (
|
||||
"maximum context length",
|
||||
"malformed config",
|
||||
"malformed configuration",
|
||||
"resolve aws credentials",
|
||||
}
|
||||
genericRetryablePatterns = []string{"server error", "internal server error"}
|
||||
interruptedPatterns = []string{"chat interrupted", "request interrupted", "operation interrupted"}
|
||||
|
||||
Reference in New Issue
Block a user