From e5b7adfb49907cee0be8e31c30ada7f283c59da9 Mon Sep 17 00:00:00 2001 From: Eric Rakestraw Date: Sun, 23 Aug 2026 18:58:15 +0000 Subject: [PATCH] Return structured errors for provider status failures --- docs/roadmap/implementation.md | 2 + internal/llm/openai_compatible_client.go | 7 +- internal/llm/openai_compatible_client_test.go | 9 ++ .../llm/provider_http_error_transport_test.go | 145 ++++++++++++++++++ 4 files changed, 161 insertions(+), 2 deletions(-) create mode 100644 internal/llm/provider_http_error_transport_test.go diff --git a/docs/roadmap/implementation.md b/docs/roadmap/implementation.md index 0f82041..ff31253 100644 --- a/docs/roadmap/implementation.md +++ b/docs/roadmap/implementation.md @@ -274,6 +274,8 @@ Stage 3 is complete when the built-in client emits the structured internal error for every non-2xx response and all prior internal identities remain green. +**Status:** Complete. + ## Stage 4: Add the Public Error and Root Mapping Contract ### Objective diff --git a/internal/llm/openai_compatible_client.go b/internal/llm/openai_compatible_client.go index 3071487..b5da684 100644 --- a/internal/llm/openai_compatible_client.go +++ b/internal/llm/openai_compatible_client.go @@ -155,8 +155,11 @@ func (c *OpenAICompatibleClient) Generate(ctx context.Context, req domain.Genera defer httpResp.Body.Close() if httpResp.StatusCode < 200 || httpResp.StatusCode >= 300 { - _, _ = io.Copy(io.Discard, io.LimitReader(httpResp.Body, 4096)) - return nil, fmt.Errorf("%w: status=%d", ErrUnexpectedStatus, httpResp.StatusCode) + return nil, providerHTTPErrorFromBody( + httpResp.StatusCode, + httpResp.ContentLength, + httpResp.Body, + ) } if httpResp.ContentLength > maxOpenAIChatResponseBytes { return nil, openAIChatResponseTooLargeError() diff --git a/internal/llm/openai_compatible_client_test.go b/internal/llm/openai_compatible_client_test.go index b3e8574..486fa37 100644 --- a/internal/llm/openai_compatible_client_test.go +++ b/internal/llm/openai_compatible_client_test.go @@ -1114,6 +1114,15 @@ func checkCommonResponseFailures(t *testing.T) { if !errors.Is(err, tc.wantErr) { t.Fatalf("error = %v, want %v", err, tc.wantErr) } + if tc.statusCode < http.StatusOK || tc.statusCode >= http.StatusMultipleChoices { + var providerHTTPError *ProviderHTTPError + if !errors.As(err, &providerHTTPError) { + t.Fatalf("error = %T, want *ProviderHTTPError", err) + } + if got := providerHTTPError.StatusCode(); got != tc.statusCode { + t.Fatalf("provider status = %d, want %d", got, tc.statusCode) + } + } if tc.wantText != "" && !strings.Contains(err.Error(), tc.wantText) { t.Fatalf("error %q does not contain %q", err, tc.wantText) } diff --git a/internal/llm/provider_http_error_transport_test.go b/internal/llm/provider_http_error_transport_test.go new file mode 100644 index 0000000..a55b8ea --- /dev/null +++ b/internal/llm/provider_http_error_transport_test.go @@ -0,0 +1,145 @@ +package llm + +import ( + "context" + "errors" + "io" + "net/http" + "strings" + "testing" +) + +func TestOpenAICompatibleClientStructuredNonSuccessResponse(t *testing.T) { + body := `{"error":{"message":" provider\nmessage\u200b","type":"invalid\ttype","code":1.5e+4}}` + responseBody := &countingReadCloser{reader: strings.NewReader(body)} + client := newNonSuccessResponseClient(t, http.StatusBadRequest, int64(len(body)), responseBody) + + response, err := client.Generate(context.Background(), ordinaryGenerateRequest()) + if response != nil { + t.Fatalf("response = %#v, want nil", response) + } + if !errors.Is(err, ErrUnexpectedStatus) { + t.Fatalf("errors.Is(%v, ErrUnexpectedStatus) = false", err) + } + var providerHTTPError *ProviderHTTPError + if !errors.As(err, &providerHTTPError) { + t.Fatalf("error = %T, want *ProviderHTTPError", err) + } + if providerHTTPError.StatusCode() != http.StatusBadRequest || providerHTTPError.ProviderCode() != "1.5e+4" || providerHTTPError.ProviderType() != "invalid type" || providerHTTPError.ProviderMessage() != "provider message" { + t.Fatalf("provider error = %#v", providerHTTPError) + } + if !responseBody.closed { + t.Fatal("non-success response body was not closed") + } +} + +func TestOpenAICompatibleClientNonSuccessBodyOwnership(t *testing.T) { + const marker = "provider-body-marker" + normalBody := `{"error":{"message":"` + marker + `"}}` + overLimitBody := normalBody + strings.Repeat(" ", int(maxProviderErrorResponseBytes)+1-len(normalBody)) + tests := []struct { + name string + contentLength int64 + reader io.Reader + wantRead int64 + wantMessage string + }{ + { + name: "normal", + contentLength: int64(len(normalBody)), + reader: strings.NewReader(normalBody), + wantRead: int64(len(normalBody)), + wantMessage: marker, + }, + { + name: "declared oversize", + contentLength: maxProviderErrorResponseBytes + 1, + reader: strings.NewReader(normalBody), + wantRead: 0, + }, + { + name: "streamed oversize", + contentLength: -1, + reader: &guardedReader{ + reader: strings.NewReader(overLimitBody), + remaining: maxProviderErrorResponseBytes + 1, + }, + wantRead: maxProviderErrorResponseBytes + 1, + }, + { + name: "underreported oversize", + contentLength: maxProviderErrorResponseBytes, + reader: &guardedReader{ + reader: strings.NewReader(overLimitBody), + remaining: maxProviderErrorResponseBytes + 1, + }, + wantRead: maxProviderErrorResponseBytes + 1, + }, + { + name: "malformed", + contentLength: 1, + reader: strings.NewReader("{"), + wantRead: 1, + }, + { + name: "read failure", + contentLength: -1, + reader: failingReader{err: errors.New("response read failed")}, + wantRead: 0, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + body := &countingReadCloser{reader: tc.reader} + client := newNonSuccessResponseClient(t, http.StatusBadGateway, tc.contentLength, body) + + response, err := client.Generate(context.Background(), ordinaryGenerateRequest()) + if response != nil { + t.Fatalf("response = %#v, want nil", response) + } + var providerHTTPError *ProviderHTTPError + if !errors.As(err, &providerHTTPError) { + t.Fatalf("error = %T, want *ProviderHTTPError", err) + } + if !body.closed { + t.Fatal("response body was not closed") + } + if body.bytesRead != tc.wantRead { + t.Fatalf("body bytes read = %d, want %d", body.bytesRead, tc.wantRead) + } + if body.bytesRead > maxProviderErrorResponseBytes+1 { + t.Fatalf("body bytes read = %d, exceeds overflow probe", body.bytesRead) + } + if guarded, ok := tc.reader.(*guardedReader); ok && guarded.violated { + t.Fatal("body reader was asked to read beyond the overflow probe") + } + if got := providerHTTPError.ProviderMessage(); got != tc.wantMessage { + t.Fatalf("provider message = %q, want %q", got, tc.wantMessage) + } + if tc.wantMessage == "" && (providerHTTPError.ProviderCode() != "" || providerHTTPError.ProviderType() != "" || strings.Contains(providerHTTPError.Error(), marker)) { + t.Fatalf("discarded details were retained: %#v", providerHTTPError) + } + }) + } +} + +func newNonSuccessResponseClient(t *testing.T, statusCode int, contentLength int64, body io.ReadCloser) *OpenAICompatibleClient { + t.Helper() + + client, err := NewOpenAICompatibleClient(OpenAICompatibleConfig{ + BaseURL: "https://provider.example/v1", + Model: "m", + HTTPClient: &http.Client{Transport: roundTripFunc(func(*http.Request) (*http.Response, error) { + return &http.Response{ + StatusCode: statusCode, + ContentLength: contentLength, + Body: body, + }, nil + })}, + }) + if err != nil { + t.Fatalf("construct client: %v", err) + } + return client +}