fix: surface chat error diagnostics (#26367)

Closes CODAGT-223

## What's already on `main` (via #25803)

#25803 fixed how `detail` is *rendered* when present:
`ChatStatusCallout` shows `status.detail` in a monospace `<code>` block
for `kind === "generic"`, `AgentChatPage` reads
`error.response?.data?.detail` inline, and the auth message was
tightened.

It did not fix `detail` being absent in the first place.

## The gap

`chaterror.Classify` only populates `Detail` from
`*fantasy.ProviderError` (OpenAI-shaped JSON envelope). Every other
realistic failure shape produces blank `Detail`:
`context.DeadlineExceeded`, `Post "…": connection refused`, `stream
error: stream ID …; INTERNAL_ERROR`, `Post "https://api.openai.com/…":
400 invalid model: gpt-9000`, `fantasy.Error` from the stream decoder,
`xerrors.New("status 401 from upstream")`, HTTP/2 peer resets. Users
still see the dead-end alert: "Request failed / The chat request failed
unexpectedly." with no third line.

## The fix

A new `chaterror.FormatDiagnosticDetail` entry point shares
diagnostic-detail logic with `classify.go`: non-auth rule-table branches
now fall back to a bounded raw error string when structured detail is
absent, while auth-classified failures keep only structured provider
detail. Curated branches (canceled, interrupted, Responses-API,
stream-incomplete, chain-broken) are left alone. The `exp_chats.go` POST
catch-all uses the exported helper, so the backend consistently emits a
bounded diagnostic string instead of leaving `Detail` blank. Fallback
diagnostics redact URLs preserved in typed transport errors by stripping
userinfo, query strings, and fragments before display, which keeps
provider error text useful while reducing credential exposure from
standard request URL wrappers.

## Security

This change surfaces upstream error text in the chat UI, where it is
also persisted in `chats.last_error`, so it crosses a trust boundary.
Codex brought this up as an issue through reviews. Mindful of cases like
#20968, where a sensitive field leaked into agent logs, the design
deliberately narrows what can reach a user:

- Auth-classified failures keep only structured provider detail and
never fall back to the raw error string.
- Fallback diagnostics redact any URL preserved in a typed `*url.Error`
by removing userinfo, query strings, and fragments, so credentials in
standard transport URL wrappers do not leak.
- Request-side credentials are not exposed: providers authenticate via
headers, and `fantasy.ProviderError.Error()` does not print the URL or
request dump. Dumped response headers are stripped before parsing, and
detail is length-capped.

The remaining channels are structured provider detail (`error.message`
from the provider's response body), which is surfaced verbatim because
it is the useful diagnostic this PR exists to deliver, and
already-flattened fallback text where typed transport context has been
lost. A well-behaved provider returns a description of the failure here,
not a secret; OpenAI, for example, masks the middle of the submitted key
and returns only a short fragment alongside a docs link. For a real
secret to appear, the upstream API, or a proxy an admin points
`base_url` at, would have to echo a plaintext credential into its own
error body or flattened error prose. I judge that any secret leakage as
a result of this PR would require a misbehaving API or middleware, and
that the usefulness of real diagnostics outweighs that bounded risk.
This commit is contained in:
Ethan
2026-06-16 00:21:13 +10:00
committed by GitHub
parent ce21a565dd
commit 1b9745c311
7 changed files with 181 additions and 3 deletions
+2 -1
View File
@@ -49,6 +49,7 @@ import (
"github.com/coder/coder/v2/coderd/workspaceapps"
"github.com/coder/coder/v2/coderd/wsbuilder"
"github.com/coder/coder/v2/coderd/x/chatd"
"github.com/coder/coder/v2/coderd/x/chatd/chaterror"
"github.com/coder/coder/v2/coderd/x/chatd/chatprovider"
"github.com/coder/coder/v2/coderd/x/chatd/chatstate"
"github.com/coder/coder/v2/coderd/x/chatd/chattool"
@@ -3123,7 +3124,7 @@ func (api *API) postChatMessages(rw http.ResponseWriter, r *http.Request) {
}
httpapi.Write(ctx, rw, http.StatusInternalServerError, codersdk.Response{
Message: "Failed to create chat message.",
Detail: sendErr.Error(),
Detail: chaterror.FormatDiagnosticDetail(sendErr),
})
return
}
+6 -2
View File
@@ -292,8 +292,12 @@ func Classify(err error) ClassifiedError {
if !rule.match {
continue
}
detail := structured.detail
if rule.kind != codersdk.ChatErrorKindAuth {
detail = resolveDiagnosticDetail(structured.detail, err)
}
return normalizeClassification(ClassifiedError{
Detail: structured.detail,
Detail: detail,
Kind: rule.kind,
Provider: provider,
Retryable: rule.retryable,
@@ -314,7 +318,7 @@ func Classify(err error) ClassifiedError {
}
return normalizeClassification(ClassifiedError{
Detail: structured.detail,
Detail: resolveDiagnosticDetail(structured.detail, err),
Kind: codersdk.ChatErrorKindGeneric,
Provider: provider,
StatusCode: statusCode,
+46
View File
@@ -32,6 +32,7 @@ func TestClassify(t *testing.T) {
err: xerrors.New("status 529 from upstream"),
want: chaterror.ClassifiedError{
Message: "The AI provider is temporarily overloaded.",
Detail: "status 529 from upstream",
Kind: codersdk.ChatErrorKindOverloaded,
Provider: "",
Retryable: true,
@@ -43,6 +44,7 @@ func TestClassify(t *testing.T) {
err: xerrors.New("anthropic overloaded_error"),
want: chaterror.ClassifiedError{
Message: "Anthropic is temporarily overloaded.",
Detail: "anthropic overloaded_error",
Kind: codersdk.ChatErrorKindOverloaded,
Provider: "anthropic",
Retryable: true,
@@ -93,6 +95,7 @@ func TestClassify(t *testing.T) {
err: xerrors.New("invalid model"),
want: chaterror.ClassifiedError{
Message: "The AI provider rejected the model configuration. Check the selected model and provider settings.",
Detail: "invalid model",
Kind: codersdk.ChatErrorKindConfig,
Provider: "",
Retryable: false,
@@ -137,6 +140,7 @@ func TestClassify(t *testing.T) {
err: xerrors.New("forbidden: context length exceeded"),
want: chaterror.ClassifiedError{
Message: "The AI provider rejected the model configuration. Check the selected model and provider settings.",
Detail: "forbidden: context length exceeded",
Kind: codersdk.ChatErrorKindConfig,
Provider: "",
Retryable: false,
@@ -148,6 +152,7 @@ func TestClassify(t *testing.T) {
err: xerrors.New("status 429 from upstream"),
want: chaterror.ClassifiedError{
Message: "The AI provider is rate limiting requests.",
Detail: "status 429 from upstream",
Kind: codersdk.ChatErrorKindRateLimit,
Provider: "",
Retryable: true,
@@ -159,6 +164,7 @@ func TestClassify(t *testing.T) {
err: xerrors.New("status 429: invalid model"),
want: chaterror.ClassifiedError{
Message: "The AI provider rejected the model configuration. Check the selected model and provider settings.",
Detail: "status 429: invalid model",
Kind: codersdk.ChatErrorKindConfig,
Provider: "",
Retryable: false,
@@ -170,6 +176,7 @@ func TestClassify(t *testing.T) {
err: xerrors.New("status 429: invalid model quota"),
want: chaterror.ClassifiedError{
Message: "The AI provider rejected the model configuration. Check the selected model and provider settings.",
Detail: "status 429: invalid model quota",
Kind: codersdk.ChatErrorKindConfig,
Provider: "",
Retryable: false,
@@ -181,6 +188,7 @@ func TestClassify(t *testing.T) {
err: xerrors.New("service unavailable"),
want: chaterror.ClassifiedError{
Message: "The AI provider is temporarily unavailable.",
Detail: "service unavailable",
Kind: codersdk.ChatErrorKindTimeout,
Provider: "",
Retryable: true,
@@ -192,6 +200,7 @@ func TestClassify(t *testing.T) {
err: xerrors.New("status 503: invalid model"),
want: chaterror.ClassifiedError{
Message: "The AI provider rejected the model configuration. Check the selected model and provider settings.",
Detail: "status 503: invalid model",
Kind: codersdk.ChatErrorKindConfig,
Provider: "",
Retryable: false,
@@ -203,6 +212,7 @@ func TestClassify(t *testing.T) {
err: xerrors.New("service unavailable: model not found"),
want: chaterror.ClassifiedError{
Message: "The AI provider rejected the model configuration. Check the selected model and provider settings.",
Detail: "service unavailable: model not found",
Kind: codersdk.ChatErrorKindConfig,
Provider: "",
Retryable: false,
@@ -214,6 +224,7 @@ func TestClassify(t *testing.T) {
err: xerrors.New("connection refused: unsupported model"),
want: chaterror.ClassifiedError{
Message: "The AI provider rejected the model configuration. Check the selected model and provider settings.",
Detail: "connection refused: unsupported model",
Kind: codersdk.ChatErrorKindConfig,
Provider: "",
Retryable: false,
@@ -225,6 +236,7 @@ func TestClassify(t *testing.T) {
err: context.DeadlineExceeded,
want: chaterror.ClassifiedError{
Message: "The request timed out before it completed.",
Detail: "context deadline exceeded",
Kind: codersdk.ChatErrorKindTimeout,
Provider: "",
Retryable: false,
@@ -236,6 +248,7 @@ func TestClassify(t *testing.T) {
err: errors.Join(chaterror.ErrProviderTransportReset, context.Canceled),
want: chaterror.ClassifiedError{
Message: "The AI provider is temporarily unavailable.",
Detail: "provider transport reset context canceled",
Kind: codersdk.ChatErrorKindTimeout,
Provider: "",
Retryable: true,
@@ -258,6 +271,7 @@ func TestClassify(t *testing.T) {
err: xerrors.Errorf("received status 500 from upstream: %w", context.Canceled),
want: chaterror.ClassifiedError{
Message: "The AI provider returned an unexpected error.",
Detail: "received status 500 from upstream: context canceled",
Kind: codersdk.ChatErrorKindGeneric,
Provider: "",
Retryable: true,
@@ -324,6 +338,7 @@ func TestClassify(t *testing.T) {
err: xerrors.New(fmt.Sprintf("%s: AI provider %q is disabled", codersdk.ChatErrorKindProviderDisabled, "anthropic")),
want: chaterror.ClassifiedError{
Message: "The Anthropic provider has been disabled. Contact your Coder administrator.",
Detail: fmt.Sprintf("%s: AI provider %q is disabled", codersdk.ChatErrorKindProviderDisabled, "anthropic"),
Kind: codersdk.ChatErrorKindProviderDisabled,
Provider: "anthropic",
Retryable: false,
@@ -663,6 +678,7 @@ func TestClassify_HTTP2TransportErrors(t *testing.T) {
require.Equal(t, codersdk.ChatErrorKindTimeout, classified.Kind, "Kind")
require.True(t, classified.Retryable, "Retryable")
require.Equal(t, tt.provider, classified.Provider, "Provider")
require.Equal(t, tt.err, classified.Detail, "Detail")
require.Equal(t, tt.wantMessage, classified.Message, "Message")
})
}
@@ -689,6 +705,7 @@ func TestClassify_HTTP2StreamErrorValues(t *testing.T) {
err: peerReset(http2.ErrCodeInternal),
want: chaterror.ClassifiedError{
Message: "The AI provider is temporarily unavailable.",
Detail: "stream error: stream ID 455; INTERNAL_ERROR; received from peer",
Kind: codersdk.ChatErrorKindTimeout,
Retryable: true,
},
@@ -698,6 +715,7 @@ func TestClassify_HTTP2StreamErrorValues(t *testing.T) {
err: peerReset(http2.ErrCodeRefusedStream),
want: chaterror.ClassifiedError{
Message: "The AI provider is temporarily unavailable.",
Detail: "stream error: stream ID 455; REFUSED_STREAM; received from peer",
Kind: codersdk.ChatErrorKindTimeout,
Retryable: true,
},
@@ -711,6 +729,7 @@ func TestClassify_HTTP2StreamErrorValues(t *testing.T) {
},
want: chaterror.ClassifiedError{
Message: "The AI provider is temporarily unavailable.",
Detail: "stream error: stream ID 455; CANCEL; received from peer",
Kind: codersdk.ChatErrorKindTimeout,
Retryable: true,
},
@@ -720,6 +739,7 @@ func TestClassify_HTTP2StreamErrorValues(t *testing.T) {
err: peerReset(http2.ErrCodeEnhanceYourCalm),
want: chaterror.ClassifiedError{
Message: "The AI provider is temporarily unavailable.",
Detail: "stream error: stream ID 455; ENHANCE_YOUR_CALM; received from peer",
Kind: codersdk.ChatErrorKindTimeout,
Retryable: true,
},
@@ -729,6 +749,7 @@ func TestClassify_HTTP2StreamErrorValues(t *testing.T) {
err: peerReset(http2.ErrCodeNo),
want: chaterror.ClassifiedError{
Message: "The AI provider is temporarily unavailable.",
Detail: "stream error: stream ID 455; NO_ERROR; received from peer",
Kind: codersdk.ChatErrorKindTimeout,
Retryable: true,
},
@@ -807,6 +828,7 @@ func TestClassify_HTTP2StreamIDDoesNotBecomeStatusCode(t *testing.T) {
},
want: chaterror.ClassifiedError{
Message: "The AI provider is temporarily unavailable.",
Detail: "stream error: stream ID 401; INTERNAL_ERROR; received from peer",
Kind: codersdk.ChatErrorKindTimeout,
Retryable: true,
},
@@ -820,6 +842,7 @@ func TestClassify_HTTP2StreamIDDoesNotBecomeStatusCode(t *testing.T) {
},
want: chaterror.ClassifiedError{
Message: "The chat request failed unexpectedly.",
Detail: "stream error: stream ID 503; PROTOCOL_ERROR; received from peer",
Kind: codersdk.ChatErrorKindGeneric,
},
},
@@ -828,6 +851,7 @@ func TestClassify_HTTP2StreamIDDoesNotBecomeStatusCode(t *testing.T) {
err: xerrors.New("stream error: stream ID 401; INTERNAL_ERROR; received from peer"),
want: chaterror.ClassifiedError{
Message: "The AI provider is temporarily unavailable.",
Detail: "stream error: stream ID 401; INTERNAL_ERROR; received from peer",
Kind: codersdk.ChatErrorKindTimeout,
Retryable: true,
},
@@ -837,6 +861,7 @@ func TestClassify_HTTP2StreamIDDoesNotBecomeStatusCode(t *testing.T) {
err: xerrors.New("stream error: stream ID 503; PROTOCOL_ERROR; received from peer"),
want: chaterror.ClassifiedError{
Message: "The chat request failed unexpectedly.",
Detail: "stream error: stream ID 503; PROTOCOL_ERROR; received from peer",
Kind: codersdk.ChatErrorKindGeneric,
},
},
@@ -1106,6 +1131,7 @@ func TestWithProviderUsesExplicitHint(t *testing.T) {
enriched := classified.WithProvider("azure openai")
require.Equal(t, chaterror.ClassifiedError{
Message: "Azure OpenAI is rate limiting requests.",
Detail: "openai received status 429 from upstream",
Kind: codersdk.ChatErrorKindRateLimit,
Provider: "azure",
Retryable: true,
@@ -1122,6 +1148,7 @@ func TestWithProviderAddsProviderWhenUnknown(t *testing.T) {
enriched := classified.WithProvider("openai")
require.Equal(t, chaterror.ClassifiedError{
Message: "OpenAI is rate limiting requests.",
Detail: "received status 429 from upstream",
Kind: codersdk.ChatErrorKindRateLimit,
Provider: "openai",
Retryable: true,
@@ -1241,6 +1268,25 @@ func TestClassify_UsesStructuredProviderDetailFromResponseDump(t *testing.T) {
}, classified)
}
func TestClassify_AuthKeepsStructuredProviderDetail(t *testing.T) {
t.Parallel()
classified := chaterror.Classify(testProviderError(
"invalid api key test-key",
401,
nil,
testProviderResponseDump(`{"error":{"message":"Incorrect API key provided."}}`),
))
require.Equal(t, chaterror.ClassifiedError{
Message: "Authentication with the AI provider failed. Check the API key and permissions.",
Detail: "Incorrect API key provided.",
Kind: codersdk.ChatErrorKindAuth,
Retryable: false,
StatusCode: 401,
}, classified)
}
func TestClassify_FallsBackToProviderMessageForDetail(t *testing.T) {
t.Parallel()
+58
View File
@@ -0,0 +1,58 @@
package chaterror
import (
"errors"
"net/url"
"strings"
)
// FormatDiagnosticDetail returns a bounded, single-line diagnostic string from
// err, suitable for surfacing to a user.
func FormatDiagnosticDetail(err error) string {
return resolveDiagnosticDetail("", err)
}
// resolveDiagnosticDetail picks the detail string to surface: structured
// provider detail always wins, otherwise the raw error string is used as a
// fallback after redacting any URL preserved in a typed *url.Error and bounding
// its length.
func resolveDiagnosticDetail(structured string, err error) string {
if strings.TrimSpace(structured) != "" {
return structured
}
if err == nil {
return ""
}
detail := strings.TrimSpace(err.Error())
if detail == "" {
return ""
}
detail = redactTypedTransportURL(detail, err)
return normalizeClassificationDetail(strings.Join(strings.Fields(detail), " "))
}
func redactTypedTransportURL(message string, err error) string {
var urlErr *url.Error
if !errors.As(err, &urlErr) || urlErr == nil || urlErr.URL == "" {
return message
}
redactedURL, changed := redactDiagnosticURL(urlErr.URL)
if !changed {
return message
}
return strings.ReplaceAll(message, urlErr.URL, redactedURL)
}
func redactDiagnosticURL(rawURL string) (string, bool) {
parsed, err := url.Parse(rawURL)
if err != nil {
return "[REDACTED_URL]", true
}
redacted := *parsed
redacted.User = nil
redacted.RawQuery = ""
redacted.ForceQuery = false
redacted.Fragment = ""
redactedURL := redacted.String()
return redactedURL, redactedURL != rawURL
}
@@ -0,0 +1,67 @@
package chaterror_test
import (
"net/url"
"strings"
"testing"
"github.com/stretchr/testify/require"
"golang.org/x/xerrors"
"github.com/coder/coder/v2/coderd/x/chatd/chaterror"
)
func TestFormatDiagnosticDetail(t *testing.T) {
t.Parallel()
tests := []struct {
name string
err error
want string
}{
{
name: "Nil",
err: nil,
},
{
name: "CollapsesWhitespace",
err: xerrors.New("stream response:\n\tconnection reset by peer"),
want: "stream response: connection reset by peer",
},
{
name: "RedactsURLUserinfoQueryAndFragment",
err: &url.Error{
Op: "Post",
URL: "https://test-user:test-password@gateway.internal/v1/chat?test_token=test-value#fragment",
Err: xerrors.New("unexpected EOF"),
},
want: `Post "https://gateway.internal/v1/chat": unexpected EOF`,
},
{
name: "RedactsWrappedURLError",
err: xerrors.Errorf("stream failed: %w", &url.Error{
Op: "Get",
URL: "https://test-key@gateway.internal/v1/chat?test_token=test-value",
Err: xerrors.New("connection refused"),
}),
want: `stream failed: Get "https://gateway.internal/v1/chat": connection refused`,
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
t.Parallel()
got := chaterror.FormatDiagnosticDetail(tt.err)
require.Equal(t, tt.want, got)
})
}
}
func TestFormatDiagnosticDetail_TruncatesLongDiagnostic(t *testing.T) {
t.Parallel()
got := chaterror.FormatDiagnosticDetail(xerrors.New(strings.Repeat("x", 510)))
require.Len(t, []rune(got), 500)
require.True(t, strings.HasSuffix(got, "…"))
}
+1
View File
@@ -22,6 +22,7 @@ func TestTerminalErrorPayloadUsesNormalizedClassification(t *testing.T) {
require.Equal(t, &codersdk.ChatError{
Message: "Azure OpenAI is rate limiting requests.",
Detail: "azure openai received status 429 from upstream",
Kind: codersdk.ChatErrorKindRateLimit,
Provider: "azure",
Retryable: true,
@@ -173,6 +173,7 @@ func TestRetry_ContextCanceledFromAttemptWithHealthyParentRetries(t *testing.T)
require.ErrorIs(t, retryErr, context.Canceled)
require.Equal(t, chaterror.ClassifiedError{
Message: "The AI provider is temporarily unavailable.",
Detail: "provider transport reset context canceled",
Kind: codersdk.ChatErrorKindTimeout,
Retryable: true,
StatusCode: 0,