mirror of
https://github.com/coder/coder.git
synced 2026-09-22 05:05:20 +08:00
feat: surface upstream provider error details in chat callout (#24546)
Anthropic HTTP 400 responses (e.g. "image exceeds 5 MB maximum") were collapsed in the chat UI to the generic headline "Anthropic returned an unexpected error (HTTP 400)." with no actionable detail — the upstream message survived to the processor log but was dropped before reaching the client. Add a new optional `Detail` field on `codersdk.ChatStreamError` that carries the upstream provider message alongside the existing normalized headline. The backend extracts `error.message` from `fantasy.ProviderError.ResponseBody` (the JSON envelope shared by Anthropic and OpenAI), falls back to the trimmed provider message when the body is absent or unparseable, and caps the result at 500 runes. The frontend threads `Detail` through `useChatStore`, `liveStatusModel`, and `ChatStatusCallout`, rendering it as a muted secondary line inside the existing `AlertDescription`. Before: <img width="1552" height="185" alt="image" src="https://github.com/user-attachments/assets/524b588e-3cee-4fad-bc15-6bf3aec0899d" /> After: <img width="814" height="173" alt="image" src="https://github.com/user-attachments/assets/eae82a89-3ac1-4a33-8d18-ef9f77263d89" /> ## Persistence `Detail` is **not** persisted — it disappears on refresh. Persisting it would require a DB change (today `chats.last_error` is a single nullable `TEXT` column), and the shape of persisted chat errors is worth a more deliberate rethink — e.g. promoting `last_error` to `JSONB` so we can also retain structured fields like `kind`, `statusCode`, `provider`, and `retryable` instead of only the normalized headline string. That's a bigger design discussion than this PR should carry. In the meantime, seeing the upstream error reason *immediately on failure* is already a large UX improvement over the status quo, and this PR gets us there without prejudicing the eventual persistence design. Tracking persistence in CODAGT-239. Closes CODAGT-235
This commit is contained in:
@@ -11,6 +11,7 @@ import (
|
||||
// underlying provider or runtime error.
|
||||
type ClassifiedError struct {
|
||||
Message string
|
||||
Detail string
|
||||
Kind string
|
||||
Provider string
|
||||
Retryable bool
|
||||
@@ -78,7 +79,7 @@ func Classify(err error) ClassifiedError {
|
||||
|
||||
structured := extractProviderErrorDetails(err)
|
||||
message := strings.TrimSpace(err.Error())
|
||||
if message == "" && structured.statusCode == 0 && structured.retryAfter <= 0 {
|
||||
if message == "" && structured.detail == "" && structured.statusCode == 0 && structured.retryAfter <= 0 {
|
||||
return ClassifiedError{}
|
||||
}
|
||||
|
||||
@@ -93,6 +94,7 @@ func Classify(err error) ClassifiedError {
|
||||
if canceled || interrupted {
|
||||
return normalizeClassification(ClassifiedError{
|
||||
Message: "The request was canceled before it completed.",
|
||||
Detail: structured.detail,
|
||||
Kind: KindGeneric,
|
||||
Provider: provider,
|
||||
StatusCode: statusCode,
|
||||
@@ -163,6 +165,7 @@ func Classify(err error) ClassifiedError {
|
||||
continue
|
||||
}
|
||||
return normalizeClassification(ClassifiedError{
|
||||
Detail: structured.detail,
|
||||
Kind: rule.kind,
|
||||
Provider: provider,
|
||||
Retryable: rule.retryable,
|
||||
@@ -172,6 +175,7 @@ func Classify(err error) ClassifiedError {
|
||||
}
|
||||
|
||||
return normalizeClassification(ClassifiedError{
|
||||
Detail: structured.detail,
|
||||
Kind: KindGeneric,
|
||||
Provider: provider,
|
||||
StatusCode: statusCode,
|
||||
@@ -181,13 +185,15 @@ func Classify(err error) ClassifiedError {
|
||||
|
||||
func normalizeClassification(classified ClassifiedError) ClassifiedError {
|
||||
classified.Message = strings.TrimSpace(classified.Message)
|
||||
classified.Detail = normalizeClassificationDetail(classified.Detail)
|
||||
classified.Kind = strings.TrimSpace(classified.Kind)
|
||||
classified.Provider = normalizeProvider(classified.Provider)
|
||||
if classified.RetryAfter < 0 {
|
||||
classified.RetryAfter = 0
|
||||
}
|
||||
if classified.Kind == "" && classified.Message == "" {
|
||||
if classified.StatusCode == 0 && classified.RetryAfter <= 0 {
|
||||
if classified.Detail == "" && classified.StatusCode == 0 &&
|
||||
classified.RetryAfter <= 0 {
|
||||
return ClassifiedError{}
|
||||
}
|
||||
classified.Kind = KindGeneric
|
||||
@@ -200,3 +206,17 @@ func normalizeClassification(classified ClassifiedError) ClassifiedError {
|
||||
}
|
||||
return classified
|
||||
}
|
||||
|
||||
const maxClassificationDetailRunes = 500
|
||||
|
||||
func normalizeClassificationDetail(detail string) string {
|
||||
detail = strings.TrimSpace(detail)
|
||||
if detail == "" {
|
||||
return ""
|
||||
}
|
||||
runes := []rune(detail)
|
||||
if len(runes) <= maxClassificationDetailRunes {
|
||||
return detail
|
||||
}
|
||||
return string(runes[:maxClassificationDetailRunes-1]) + "…"
|
||||
}
|
||||
|
||||
@@ -3,6 +3,7 @@ package chaterror_test
|
||||
import (
|
||||
"context"
|
||||
"net/http"
|
||||
"strings"
|
||||
"testing"
|
||||
"time"
|
||||
|
||||
@@ -574,7 +575,7 @@ func TestWithProviderPreservesRetryAfter(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
classified := chaterror.Classify(testProviderError(
|
||||
"upstream failed",
|
||||
"",
|
||||
429,
|
||||
map[string]string{"Retry-After": "30"},
|
||||
))
|
||||
@@ -591,10 +592,75 @@ func TestWithProviderPreservesRetryAfter(t *testing.T) {
|
||||
}, enriched)
|
||||
}
|
||||
|
||||
func testProviderError(message string, statusCode int, headers map[string]string) error {
|
||||
func TestClassify_UsesStructuredProviderDetailFromResponseDump(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
classified := chaterror.Classify(testProviderError(
|
||||
"",
|
||||
400,
|
||||
nil,
|
||||
testProviderResponseDump(`{"error":{"type":"invalid_request_error","message":"Image exceeds 5 MB maximum."}}`),
|
||||
))
|
||||
|
||||
require.Equal(t, chaterror.ClassifiedError{
|
||||
Message: "The AI provider returned an unexpected error (HTTP 400).",
|
||||
Detail: "Image exceeds 5 MB maximum.",
|
||||
Kind: chaterror.KindGeneric,
|
||||
Provider: "",
|
||||
Retryable: false,
|
||||
StatusCode: 400,
|
||||
}, classified)
|
||||
}
|
||||
|
||||
func TestClassify_FallsBackToProviderMessageForDetail(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
classified := chaterror.Classify(testProviderError(
|
||||
" image exceeds 5 MB maximum ",
|
||||
400,
|
||||
nil,
|
||||
testProviderResponseDump("not-json"),
|
||||
))
|
||||
|
||||
require.Equal(t, "image exceeds 5 MB maximum", classified.Detail)
|
||||
}
|
||||
|
||||
func TestClassify_TruncatesProviderDetail(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
detail := strings.Repeat("x", 510)
|
||||
classified := chaterror.Classify(testProviderError(
|
||||
"",
|
||||
400,
|
||||
nil,
|
||||
testProviderResponseDump(`{"error":{"message":"`+detail+`"}}`),
|
||||
))
|
||||
|
||||
require.Len(t, []rune(classified.Detail), 500)
|
||||
require.True(t, strings.HasSuffix(classified.Detail, "…"))
|
||||
}
|
||||
|
||||
func testProviderError(
|
||||
message string,
|
||||
statusCode int,
|
||||
headers map[string]string,
|
||||
responseBody ...[]byte,
|
||||
) error {
|
||||
var body []byte
|
||||
if len(responseBody) > 0 {
|
||||
body = responseBody[0]
|
||||
}
|
||||
return &fantasy.ProviderError{
|
||||
Message: message,
|
||||
StatusCode: statusCode,
|
||||
ResponseHeaders: headers,
|
||||
ResponseBody: body,
|
||||
}
|
||||
}
|
||||
|
||||
func testProviderResponseDump(body string) []byte {
|
||||
return []byte(`HTTP/1.1 400 Bad Request
|
||||
Content-Type: application/json
|
||||
|
||||
` + body)
|
||||
}
|
||||
|
||||
@@ -12,6 +12,7 @@ func StreamErrorPayload(classified ClassifiedError) *codersdk.ChatStreamError {
|
||||
}
|
||||
return &codersdk.ChatStreamError{
|
||||
Message: classified.Message,
|
||||
Detail: classified.Detail,
|
||||
Kind: classified.Kind,
|
||||
Provider: classified.Provider,
|
||||
Retryable: classified.Retryable,
|
||||
|
||||
@@ -28,6 +28,19 @@ func TestStreamErrorPayloadUsesNormalizedClassification(t *testing.T) {
|
||||
}, payload)
|
||||
}
|
||||
|
||||
func TestStreamErrorPayloadIncludesProviderDetail(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
payload := chaterror.StreamErrorPayload(chaterror.Classify(testProviderError(
|
||||
"",
|
||||
400,
|
||||
nil,
|
||||
testProviderResponseDump(`{"error":{"message":"Image exceeds 5 MB maximum."}}`),
|
||||
)))
|
||||
|
||||
require.Equal(t, "Image exceeds 5 MB maximum.", payload.Detail)
|
||||
}
|
||||
|
||||
func TestStreamErrorPayloadNilForEmptyClassification(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
|
||||
@@ -1,6 +1,8 @@
|
||||
package chaterror
|
||||
|
||||
import (
|
||||
"bytes"
|
||||
"encoding/json"
|
||||
"errors"
|
||||
"net/http"
|
||||
"strconv"
|
||||
@@ -11,6 +13,7 @@ import (
|
||||
)
|
||||
|
||||
type providerErrorDetails struct {
|
||||
detail string
|
||||
statusCode int
|
||||
retryAfter time.Duration
|
||||
}
|
||||
@@ -22,11 +25,48 @@ func extractProviderErrorDetails(err error) providerErrorDetails {
|
||||
}
|
||||
|
||||
return providerErrorDetails{
|
||||
detail: providerErrorDetail(providerErr),
|
||||
statusCode: providerErr.StatusCode,
|
||||
retryAfter: retryAfterFromHeaders(providerErr.ResponseHeaders),
|
||||
}
|
||||
}
|
||||
|
||||
func providerErrorDetail(providerErr *fantasy.ProviderError) string {
|
||||
if detail := providerErrorResponseMessage(providerErr.ResponseBody); detail != "" {
|
||||
return detail
|
||||
}
|
||||
return strings.TrimSpace(providerErr.Message)
|
||||
}
|
||||
|
||||
// providerErrorResponseMessage extracts error.message from the common
|
||||
// provider error JSON envelope after stripping any dumped HTTP status
|
||||
// line and headers.
|
||||
func providerErrorResponseMessage(responseDump []byte) string {
|
||||
if len(responseDump) == 0 || len(responseDump) > 64*1024 {
|
||||
return ""
|
||||
}
|
||||
body := providerErrorResponseBody(responseDump)
|
||||
var envelope struct {
|
||||
Error struct {
|
||||
Message string `json:"message"`
|
||||
} `json:"error"`
|
||||
}
|
||||
if err := json.Unmarshal(body, &envelope); err != nil {
|
||||
return ""
|
||||
}
|
||||
return strings.TrimSpace(envelope.Error.Message)
|
||||
}
|
||||
|
||||
func providerErrorResponseBody(responseDump []byte) []byte {
|
||||
if _, body, ok := bytes.Cut(responseDump, []byte("\r\n\r\n")); ok {
|
||||
return body
|
||||
}
|
||||
if _, body, ok := bytes.Cut(responseDump, []byte("\n\n")); ok {
|
||||
return body
|
||||
}
|
||||
return responseDump
|
||||
}
|
||||
|
||||
func retryAfterFromHeaders(headers map[string]string) time.Duration {
|
||||
if len(headers) == 0 {
|
||||
return 0
|
||||
|
||||
@@ -1200,6 +1200,9 @@ type ChatStreamStatus struct {
|
||||
type ChatStreamError struct {
|
||||
// Message is the normalized, user-facing error message.
|
||||
Message string `json:"message"`
|
||||
// Detail is optional provider-specific context shown alongside the
|
||||
// normalized error message when available.
|
||||
Detail string `json:"detail,omitempty"`
|
||||
// Kind classifies the error for consistent client rendering.
|
||||
Kind string `json:"kind,omitempty"`
|
||||
// Provider identifies the upstream model provider when known.
|
||||
|
||||
Generated
+5
@@ -2179,6 +2179,11 @@ export interface ChatStreamError {
|
||||
* Message is the normalized, user-facing error message.
|
||||
*/
|
||||
readonly message: string;
|
||||
/**
|
||||
* Detail is optional provider-specific context shown alongside the
|
||||
* normalized error message when available.
|
||||
*/
|
||||
readonly detail?: string;
|
||||
/**
|
||||
* Kind classifies the error for consistent client rendering.
|
||||
*/
|
||||
|
||||
@@ -216,7 +216,7 @@ export const WithError: Story = {
|
||||
<StoryAgentChatPageView
|
||||
persistedError={{
|
||||
kind: "overloaded",
|
||||
message: "Anthropic is currently overloaded.",
|
||||
message: "Anthropic is temporarily overloaded (HTTP 529).",
|
||||
provider: "anthropic",
|
||||
retryable: true,
|
||||
statusCode: 529,
|
||||
@@ -229,11 +229,10 @@ export const WithError: Story = {
|
||||
canvas.getByRole("heading", { name: /service overloaded/i }),
|
||||
).toBeVisible();
|
||||
expect(
|
||||
canvas.getByText(/anthropic is currently overloaded\./i),
|
||||
canvas.getByText(/anthropic is temporarily overloaded \(http 529\)/i),
|
||||
).toBeVisible();
|
||||
expect(canvas.queryByText(/please try again/i)).not.toBeInTheDocument();
|
||||
expect(canvas.queryByText(/^retryable$/i)).not.toBeInTheDocument();
|
||||
expect(canvas.getByText(/http 529/i)).toBeVisible();
|
||||
},
|
||||
};
|
||||
|
||||
|
||||
@@ -133,9 +133,6 @@ const StatusAlert: FC<{ status: RetryOrFailedStatus }> = ({ status }) => {
|
||||
if (status.phase === "retrying") {
|
||||
metadataItems.push(<span key="attempt">Attempt {status.attempt}</span>);
|
||||
}
|
||||
if (status.phase === "failed" && status.statusCode !== undefined) {
|
||||
metadataItems.push(<span key="code">HTTP {status.statusCode}</span>);
|
||||
}
|
||||
|
||||
return (
|
||||
<Alert
|
||||
@@ -150,11 +147,18 @@ const StatusAlert: FC<{ status: RetryOrFailedStatus }> = ({ status }) => {
|
||||
>
|
||||
<AlertTitle>{status.title}</AlertTitle>
|
||||
<AlertDescription>
|
||||
{status.message}{" "}
|
||||
{statusURL && (
|
||||
<Link href={statusURL} target="_blank" rel="noreferrer">
|
||||
Status
|
||||
</Link>
|
||||
<span>
|
||||
{status.message}{" "}
|
||||
{statusURL && (
|
||||
<Link href={statusURL} target="_blank" rel="noreferrer">
|
||||
Status
|
||||
</Link>
|
||||
)}
|
||||
</span>
|
||||
{status.phase === "failed" && status.detail && (
|
||||
<span className="mt-1 block text-content-secondary">
|
||||
{status.detail}
|
||||
</span>
|
||||
)}
|
||||
</AlertDescription>
|
||||
</Alert>
|
||||
|
||||
@@ -81,7 +81,7 @@ export const TerminalOverloadedError: Story = {
|
||||
liveStatus: buildLiveStatus({
|
||||
persistedError: {
|
||||
kind: "overloaded",
|
||||
message: "Anthropic is currently overloaded.",
|
||||
message: "Anthropic is temporarily overloaded (HTTP 529).",
|
||||
provider: "anthropic",
|
||||
retryable: true,
|
||||
statusCode: 529,
|
||||
@@ -94,11 +94,10 @@ export const TerminalOverloadedError: Story = {
|
||||
canvas.getByRole("heading", { name: /service overloaded/i }),
|
||||
).toBeVisible();
|
||||
expect(
|
||||
canvas.getByText(/anthropic is currently overloaded./i),
|
||||
canvas.getByText(/anthropic is temporarily overloaded \(http 529\)/i),
|
||||
).toBeVisible();
|
||||
expect(canvas.queryByText(/please try again/i)).not.toBeInTheDocument();
|
||||
expect(canvas.queryByText(/^retryable$/i)).not.toBeInTheDocument();
|
||||
expect(canvas.getByText(/http 529/i)).toBeVisible();
|
||||
expect(canvas.getByRole("link", { name: /status/i })).toBeVisible();
|
||||
expect(canvas.queryByText(/provider anthropic/i)).not.toBeInTheDocument();
|
||||
},
|
||||
@@ -248,6 +247,34 @@ export const GenericErrorDoesNotShowUsageAction: Story = {
|
||||
},
|
||||
};
|
||||
|
||||
/** Provider detail renders as a muted secondary line under the main error. */
|
||||
export const GenericErrorShowsProviderDetail: Story = {
|
||||
args: {
|
||||
...defaultArgs,
|
||||
liveStatus: buildLiveStatus({
|
||||
streamError: {
|
||||
kind: "generic",
|
||||
message: "Anthropic returned an unexpected error (HTTP 400).",
|
||||
detail:
|
||||
"messages.0.content.1.image.source.base64: image exceeds 5 MB maximum.",
|
||||
provider: "anthropic",
|
||||
statusCode: 400,
|
||||
retryable: false,
|
||||
},
|
||||
}),
|
||||
},
|
||||
play: async ({ canvasElement }) => {
|
||||
const canvas = within(canvasElement);
|
||||
expect(
|
||||
canvas.getByRole("heading", { name: /request failed/i }),
|
||||
).toBeVisible();
|
||||
expect(
|
||||
canvas.getByText(/anthropic returned an unexpected error \(http 400\)/i),
|
||||
).toBeVisible();
|
||||
expect(canvas.getByText(/image exceeds 5 mb maximum/i)).toBeVisible();
|
||||
},
|
||||
};
|
||||
|
||||
/** Reconnecting keeps already-streamed content visible without a terminal footer. */
|
||||
export const ReconnectingKeepsPartialOutputVisible: Story = {
|
||||
args: {
|
||||
|
||||
@@ -237,6 +237,7 @@ describe("setStreamError / clearStreamError", () => {
|
||||
store.setStreamError({
|
||||
kind: "generic",
|
||||
message: "oops",
|
||||
detail: "Image exceeds 5 MB maximum.",
|
||||
});
|
||||
|
||||
let notified = false;
|
||||
@@ -246,6 +247,7 @@ describe("setStreamError / clearStreamError", () => {
|
||||
store.setStreamError({
|
||||
kind: "generic",
|
||||
message: "oops",
|
||||
detail: "Image exceeds 5 MB maximum.",
|
||||
});
|
||||
|
||||
expect(notified).toBe(false);
|
||||
|
||||
@@ -1710,6 +1710,7 @@ describe("useChatStore", () => {
|
||||
chat_id: chatID,
|
||||
error: {
|
||||
message: "Rate limit exceeded",
|
||||
detail: "Image exceeds 5 MB maximum.",
|
||||
kind: "rate_limit",
|
||||
provider: "anthropic",
|
||||
retryable: true,
|
||||
@@ -1724,6 +1725,7 @@ describe("useChatStore", () => {
|
||||
expect(result.current.streamError).toEqual({
|
||||
kind: "rate_limit",
|
||||
message: "Rate limit exceeded",
|
||||
detail: "Image exceeds 5 MB maximum.",
|
||||
provider: "anthropic",
|
||||
retryable: true,
|
||||
statusCode: 429,
|
||||
@@ -1732,6 +1734,7 @@ describe("useChatStore", () => {
|
||||
expect(setChatErrorReason).toHaveBeenCalledWith(chatID, {
|
||||
kind: "rate_limit",
|
||||
message: "Rate limit exceeded",
|
||||
detail: "Image exceeds 5 MB maximum.",
|
||||
provider: "anthropic",
|
||||
retryable: true,
|
||||
statusCode: 429,
|
||||
|
||||
@@ -137,6 +137,19 @@ describe("deriveLiveStatus", () => {
|
||||
});
|
||||
});
|
||||
|
||||
it("passes provider detail through failed status", () => {
|
||||
expect(
|
||||
derive({
|
||||
streamError: makeStreamError({
|
||||
detail: "Image exceeds 5 MB maximum.",
|
||||
}),
|
||||
}),
|
||||
).toMatchObject({
|
||||
phase: "failed",
|
||||
detail: "Image exceeds 5 MB maximum.",
|
||||
});
|
||||
});
|
||||
|
||||
it("tracks accumulated output while reconnecting", () => {
|
||||
expect(
|
||||
derive({
|
||||
|
||||
@@ -36,6 +36,7 @@ export type LiveStatusModel =
|
||||
title: string;
|
||||
kind: string;
|
||||
message: string;
|
||||
detail?: string;
|
||||
provider?: string;
|
||||
statusCode?: number;
|
||||
} & LiveStatusBase);
|
||||
@@ -87,6 +88,7 @@ const toFailedLiveStatus = (
|
||||
title: getErrorTitle(error.kind, "error"),
|
||||
kind: error.kind,
|
||||
message: error.message,
|
||||
...(error.detail ? { detail: error.detail } : {}),
|
||||
provider: error.provider,
|
||||
statusCode: error.statusCode,
|
||||
});
|
||||
|
||||
@@ -24,13 +24,17 @@ import type { RetryState } from "./types";
|
||||
|
||||
const normalizeChatDetailError = (
|
||||
error: TypesGen.ChatStreamError | undefined,
|
||||
): ChatDetailError => ({
|
||||
message: error?.message.trim() || "Chat processing failed.",
|
||||
kind: error?.kind?.trim() || "generic",
|
||||
provider: error?.provider?.trim() || undefined,
|
||||
retryable: error?.retryable,
|
||||
statusCode: error?.status_code,
|
||||
});
|
||||
): ChatDetailError => {
|
||||
const detail = error?.detail?.trim();
|
||||
return {
|
||||
message: error?.message.trim() || "Chat processing failed.",
|
||||
kind: error?.kind?.trim() || "generic",
|
||||
provider: error?.provider?.trim() || undefined,
|
||||
retryable: error?.retryable,
|
||||
statusCode: error?.status_code,
|
||||
...(detail ? { detail } : {}),
|
||||
};
|
||||
};
|
||||
|
||||
const normalizeRetryState = (retry: TypesGen.ChatStreamRetry): RetryState => ({
|
||||
attempt: Math.max(1, retry.attempt),
|
||||
|
||||
@@ -97,6 +97,9 @@ describe("chatDetailErrorsEqual", () => {
|
||||
expect(chatDetailErrorsEqual(error, { ...error, statusCode: 500 })).toBe(
|
||||
false,
|
||||
);
|
||||
expect(
|
||||
chatDetailErrorsEqual(error, { ...error, detail: "Bad image." }),
|
||||
).toBe(false);
|
||||
});
|
||||
});
|
||||
|
||||
|
||||
@@ -32,6 +32,7 @@ export type ChatProviderFailureKind =
|
||||
*/
|
||||
export type ChatDetailError = {
|
||||
message: string;
|
||||
detail?: string;
|
||||
kind: ChatProviderFailureKind | (string & {});
|
||||
provider?: string;
|
||||
retryable?: boolean;
|
||||
@@ -54,6 +55,7 @@ export const chatDetailErrorsEqual = (
|
||||
return (
|
||||
left.kind === right.kind &&
|
||||
left.message === right.message &&
|
||||
left.detail === right.detail &&
|
||||
left.provider === right.provider &&
|
||||
left.retryable === right.retryable &&
|
||||
left.statusCode === right.statusCode
|
||||
|
||||
Reference in New Issue
Block a user