fix(coderd/x/chatd/chaterror): extract plain-text provider error bodies (#27597)

Follows up #27538. Fixes an issue where `chaterror` would not classify a plain-text aibridge budget error 403 as ChatErrorKindUsageLimit. Root cause: Anthropic adapter drops the body from `ProviderError.Message`, and `providerErrorResponseMessage` extracted only JSON. - Parses the dumped response with `http.ReadResponse` (strips headers, de-chunks, leaves non-dump payloads like Google's raw messages whole). - When JSON extraction yields nothing, falls back to the trimmed first line of the body. - Falls back on `Content-Type: text/plain` only, and never on valid JSON as `Detail` is user-facing. - Adds an end-to-end regression test that Anthropic-shaped 403 budget error → `usage_limit`, not retryable (was `auth`). - Adds table-driven test cases: HTML skipped, whitespace skipped, first-line-only, JSON-without-message, chunked, Google raw message. --- > Generated by Coder Agents on behalf of @johnstcn.
2026-09-22 13:10:21 +08:00 · 2026-07-29 10:38:15 +01:00
parent 0b4095085e
commit 5d2a69d85a
2 changed files with 173 additions and 10 deletions
@@ -1431,12 +1431,144 @@ func TestClassify_FallsBackToProviderMessageForDetail(t *testing.T) {
 		"  image exceeds 5 MB maximum  ",
 		400,
 		nil,
-		testProviderResponseDump("not-json"),
 	))

 	require.Equal(t, "image exceeds 5 MB maximum", classified.Detail)
 }

+func TestClassify_AnthropicPlainTextBudgetBody(t *testing.T) {
+	t.Parallel()
+
+	// aibridge returns its budget error as a plain-text 403. The Anthropic
+	// adapter's Message is the SDK transport string without the body, so
+	// the budget text appears only in the dumped ResponseBody. Classify
+	// must still report a usage limit, not auth.
+	classified := chaterror.Classify(testProviderError(
+		`POST "https://api.example.com/v1/messages": 403 Forbidden`,
+		403,
+		nil,
+		[]byte("HTTP/1.1 403 Forbidden\r\n"+
+			"Content-Type: text/plain; charset=utf-8\r\n"+
+			"X-Content-Type-Options: nosniff\r\n"+
+			"\r\n"+
+			"AI budget of US$10.00 exceeded. Please contact an administrator for more details.\n"),
+	))
+
+	require.Equal(t, codersdk.ChatErrorKindUsageLimit, classified.Kind)
+	require.False(t, classified.Retryable)
+	require.Equal(t,
+		"AI budget of US$10.00 exceeded. Please contact an administrator for more details.",
+		classified.Detail)
+}
+
+func TestClassify_ProviderResponseDumps(t *testing.T) {
+	t.Parallel()
+
+	tests := []struct {
+		name          string
+		message       string
+		status        int
+		dump          []byte
+		wantDetail    string
+		wantKind      codersdk.ChatErrorKind
+		wantRetryable bool
+	}{
+		{
+			// Proxies and load balancers return HTML error pages with a
+			// text/html Content-Type. The text/plain gate keeps them out
+			// of the user-facing detail, so detail falls back to the
+			// provider message.
+			name:          "SkipsHTMLBody",
+			message:       "upstream failed",
+			status:        502,
+			dump:          testPlainDump("text/html", "<html><body>502 Bad Gateway</body></html>"),
+			wantDetail:    "upstream failed",
+			wantKind:      codersdk.ChatErrorKindTimeout,
+			wantRetryable: true,
+		},
+		{
+			name:          "SkipsWhitespaceOnlyBody",
+			message:       "upstream failed",
+			status:        400,
+			dump:          testPlainDump("text/plain", "  \n\t\n"),
+			wantDetail:    "upstream failed",
+			wantKind:      codersdk.ChatErrorKindGeneric,
+			wantRetryable: false,
+		},
+		{
+			name:          "PlainTextBodyFirstLineOnly",
+			status:        400,
+			dump:          testPlainDump("text/plain", "first line of the error\nsecond line\nthird line\n"),
+			wantDetail:    "first line of the error",
+			wantKind:      codersdk.ChatErrorKindGeneric,
+			wantRetryable: false,
+		},
+		{
+			// Valid JSON without an extractable message must not leak raw
+			// JSON into the user-facing detail, even when served as
+			// text/plain.
+			name:          "JSONBodyWithoutMessage",
+			message:       "upstream failed",
+			status:        400,
+			dump:          testPlainDump("text/plain", `{"type":"error"}`),
+			wantDetail:    "upstream failed",
+			wantKind:      codersdk.ChatErrorKindGeneric,
+			wantRetryable: false,
+		},
+		{
+			// A plain-text body feeds the same pattern matching as a JSON
+			// message, so "quota" beats the 503 timeout signal.
+			name:          "PlainTextQuotaBodyOn503",
+			status:        503,
+			dump:          testPlainDump("text/plain", "quota exceeded for this key\n"),
+			wantDetail:    "quota exceeded for this key",
+			wantKind:      codersdk.ChatErrorKindUsageLimit,
+			wantRetryable: false,
+		},
+		{
+			// A dump of a chunked response keeps the chunk framing.
+			// Parsing the dump as an HTTP response removes it, so the
+			// detail is the joined body, not a hex chunk-size line.
+			name:   "DechunksPlainTextBody",
+			status: 403,
+			dump: []byte("HTTP/1.1 403 Forbidden\r\n" +
+				"Content-Type: text/plain; charset=utf-8\r\n" +
+				"Transfer-Encoding: chunked\r\n" +
+				"\r\n" +
+				"16\r\nAI budget of US$10.00 \r\n" +
+				"9\r\nexceeded.\r\n" +
+				"0\r\n\r\n"),
+			wantDetail:    "AI budget of US$10.00 exceeded.",
+			wantKind:      codersdk.ChatErrorKindUsageLimit,
+			wantRetryable: false,
+		},
+		{
+			// Fantasy's Google adapter stores a raw message (not an HTTP
+			// dump) in ResponseBody. A blank line inside it is not a
+			// header/body separator: detail must fall back to the full
+			// trimmed Message, not the second paragraph.
+			name:          "GoogleRawMessageWithBlankLine",
+			message:       "google: model overloaded",
+			status:        500,
+			dump:          []byte("model overloaded\n\nplease try again later"),
+			wantDetail:    "google: model overloaded",
+			wantKind:      codersdk.ChatErrorKindOverloaded,
+			wantRetryable: true,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			t.Parallel()
+
+			classified := chaterror.Classify(testProviderError(tt.message, tt.status, nil, tt.dump))
+			require.Equal(t, tt.wantDetail, classified.Detail)
+			require.Equal(t, tt.wantKind, classified.Kind)
+			require.Equal(t, tt.wantRetryable, classified.Retryable)
+		})
+	}
+}
+
 func TestClassify_UnwrapsTransportWrapperInMessageFallback(t *testing.T) {
 	t.Parallel()

@@ -1507,6 +1639,12 @@ func testProviderError(
 	}
 }

+func testPlainDump(contentType, body string) []byte {
+	return []byte("HTTP/1.1 400 Bad Request\r\n" +
+		"Content-Type: " + contentType + "\r\n" +
+		"\r\n" + body)
+}
+
 func testProviderResponseDump(body string) []byte {
 	return []byte(`HTTP/1.1 400 Bad Request
 Content-Type: application/json
@@ -1,9 +1,12 @@
 package chaterror

 import (
+	"bufio"
 	"bytes"
 	"encoding/json"
 	"errors"
+	"io"
+	"mime"
 	"net/http"
 	"regexp"
 	"strconv"
@@ -53,13 +56,24 @@ func providerErrorDetail(providerErr *fantasy.ProviderError) string {
 // and headers. It understands both the top-level `{"message":...}` shape
 // used by many providers and the nested `{"error":{"message":...}}`
 // envelope. When the extracted message is itself an SDK-formatted transport
-// error wrapper, the clean inner provider message is returned.
+// error wrapper, the clean inner provider message is returned. For
+// non-JSON text/plain bodies (e.g. aibridge's budget errors) it returns
+// the trimmed first line of the body; the result surfaces in the
+// user-facing detail, so other content types (proxy HTML, opaque bodies)
+// and valid JSON without an extractable message yield nothing.
 func providerErrorResponseMessage(responseDump []byte) string {
 	if len(responseDump) == 0 || len(responseDump) > 64*1024 {
 		return ""
 	}
-	body := providerErrorResponseBody(responseDump)
-	return unwrapTransportErrorMessage(jsonErrorMessage(body))
+	body, textPlain := readResponseDump(responseDump)
+	if msg := unwrapTransportErrorMessage(jsonErrorMessage(body)); msg != "" {
+		return msg
+	}
+	if !textPlain || json.Valid(body) {
+		return ""
+	}
+	line, _, _ := strings.Cut(strings.TrimSpace(string(body)), "\n")
+	return strings.TrimSpace(line)
 }

 // unwrapTransportErrorMessage extracts the clean provider message from an
@@ -115,14 +129,25 @@ func jsonErrorMessage(body []byte) string {
 	return strings.TrimSpace(env.Message)
 }

-func providerErrorResponseBody(responseDump []byte) []byte {
-	if _, body, ok := bytes.Cut(responseDump, []byte("\r\n\r\n")); ok {
-		return body
+// readResponseDump parses a dumped HTTP response into its body, removing
+// the status line, headers, and any chunk framing, and reports whether the
+// response declares Content-Type text/plain (ignoring media-type
+// parameters such as charset). Payloads that do not parse as an HTTP
+// response, such as the raw message fantasy's Google adapter stores in
+// ResponseBody, are returned whole.
+func readResponseDump(responseDump []byte) (body []byte, textPlain bool) {
+	resp, err := http.ReadResponse(bufio.NewReader(bytes.NewReader(responseDump)), nil)
+	if err != nil {
+		return responseDump, false
 	}
-	if _, body, ok := bytes.Cut(responseDump, []byte("\n\n")); ok {
-		return body
+	defer resp.Body.Close()
+	// The caller already bounds dumps at 64KB.
+	body, err = io.ReadAll(resp.Body)
+	if err != nil {
+		return responseDump, false
 	}
-	return responseDump
+	mediaType, _, err := mime.ParseMediaType(resp.Header.Get("Content-Type"))
+	return body, err == nil && mediaType == "text/plain"
 }

 func retryAfterFromHeaders(headers map[string]string) time.Duration {