diff --git a/pkg/providers/openai_compat/provider.go b/pkg/providers/openai_compat/provider.go index 6b8f0181d..83966180a 100644 --- a/pkg/providers/openai_compat/provider.go +++ b/pkg/providers/openai_compat/provider.go @@ -188,14 +188,18 @@ func (p *Provider) Chat( // Non-200: read a prefix to tell HTML error page apart from JSON error body. if resp.StatusCode != http.StatusOK { - body, err := io.ReadAll(io.LimitReader(resp.Body, 256)) - if err != nil { - return nil, fmt.Errorf("failed to read response: %w", err) + body, readErr := io.ReadAll(io.LimitReader(resp.Body, 256)) + if readErr != nil { + return nil, fmt.Errorf("failed to read response: %w", readErr) } if looksLikeHTML(body, contentType) { return nil, wrapHTMLResponseError(resp.StatusCode, body, contentType, p.apiBase) } - return nil, fmt.Errorf("API request failed:\n Status: %d\n Body: %s", resp.StatusCode, responsePreview(body, 128)) + return nil, fmt.Errorf( + "API request failed:\n Status: %d\n Body: %s", + resp.StatusCode, + responsePreview(body, 128), + ) } // Peek without consuming so the full stream reaches the JSON decoder. @@ -218,7 +222,13 @@ func (p *Provider) Chat( func wrapHTMLResponseError(statusCode int, body []byte, contentType, apiBase string) error { respPreview := responsePreview(body, 128) - return fmt.Errorf("API request failed: %s returned HTML instead of JSON (content-type: %s); check api_base or proxy configuration.\n Status: %d\n Body: %s", apiBase, contentType, statusCode, respPreview) + return fmt.Errorf( + "API request failed: %s returned HTML instead of JSON (content-type: %s); check api_base or proxy configuration.\n Status: %d\n Body: %s", + apiBase, + contentType, + statusCode, + respPreview, + ) } func looksLikeHTML(body []byte, contentType string) bool { diff --git a/pkg/providers/openai_compat/provider_test.go b/pkg/providers/openai_compat/provider_test.go index c729289d4..5c4dcd1b0 100644 --- a/pkg/providers/openai_compat/provider_test.go +++ b/pkg/providers/openai_compat/provider_test.go @@ -3,6 +3,7 @@ package openai_compat import ( "bytes" "encoding/json" + "fmt" "io" "net/http" "net/http/httptest" @@ -235,69 +236,57 @@ func TestProviderChat_JSONHTTPErrorDoesNotReportHTML(t *testing.T) { } } -func TestProviderChat_HTMLSuccessResponseReturnsHelpfulError(t *testing.T) { - server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - w.Header().Set("Content-Type", "text/html; charset=utf-8") - w.WriteHeader(http.StatusOK) - _, _ = w.Write([]byte("gateway login")) - })) - defer server.Close() +func TestProviderChat_HTMLResponsesReturnHelpfulError(t *testing.T) { + tests := []struct { + name string + contentType string + statusCode int + body string + }{ + { + name: "html success response", + contentType: "text/html; charset=utf-8", + statusCode: http.StatusOK, + body: "gateway login", + }, + { + name: "html error response", + contentType: "text/html; charset=utf-8", + statusCode: http.StatusBadGateway, + body: "bad gateway", + }, + { + name: "mislabeled html success response", + contentType: "application/json", + statusCode: http.StatusOK, + body: " \r\n\tgateway login", + }, + } - p := NewProvider("key", server.URL, "") - _, err := p.Chat(t.Context(), []Message{{Role: "user", Content: "hi"}}, nil, "gpt-4o", nil) - if err == nil { - t.Fatal("expected error, got nil") - } - if !strings.Contains(err.Error(), "returned HTML instead of JSON") { - t.Fatalf("expected helpful HTML error, got %v", err) - } - if !strings.Contains(err.Error(), "check api_base or proxy configuration") { - t.Fatalf("expected configuration hint, got %v", err) - } -} + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", tt.contentType) + w.WriteHeader(tt.statusCode) + _, _ = w.Write([]byte(tt.body)) + })) + defer server.Close() -func TestProviderChat_HTMLErrorResponseReturnsHelpfulError(t *testing.T) { - server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - w.Header().Set("Content-Type", "text/html; charset=utf-8") - w.WriteHeader(http.StatusBadGateway) - _, _ = w.Write([]byte("bad gateway")) - })) - defer server.Close() - - p := NewProvider("key", server.URL, "") - _, err := p.Chat(t.Context(), []Message{{Role: "user", Content: "hi"}}, nil, "gpt-4o", nil) - if err == nil { - t.Fatal("expected error, got nil") - } - if !strings.Contains(err.Error(), "Status: 502") { - t.Fatalf("expected status code in error, got %v", err) - } - if !strings.Contains(err.Error(), "returned HTML instead of JSON") { - t.Fatalf("expected helpful HTML error, got %v", err) - } - if !strings.Contains(err.Error(), "check api_base or proxy configuration") { - t.Fatalf("expected configuration hint, got %v", err) - } -} - -func TestProviderChat_MislabeledHTMLSuccessResponseReturnsHelpfulError(t *testing.T) { - server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - w.Header().Set("Content-Type", "application/json") - w.WriteHeader(http.StatusOK) - _, _ = w.Write([]byte(" \r\n\tgateway login")) - })) - defer server.Close() - - p := NewProvider("key", server.URL, "") - _, err := p.Chat(t.Context(), []Message{{Role: "user", Content: "hi"}}, nil, "gpt-4o", nil) - if err == nil { - t.Fatal("expected error, got nil") - } - if !strings.Contains(err.Error(), "returned HTML instead of JSON") { - t.Fatalf("expected helpful HTML error, got %v", err) - } - if !strings.Contains(err.Error(), "check api_base or proxy configuration") { - t.Fatalf("expected configuration hint, got %v", err) + p := NewProvider("key", server.URL, "") + _, err := p.Chat(t.Context(), []Message{{Role: "user", Content: "hi"}}, nil, "gpt-4o", nil) + if err == nil { + t.Fatal("expected error, got nil") + } + if !strings.Contains(err.Error(), fmt.Sprintf("Status: %d", tt.statusCode)) { + t.Fatalf("expected status code in error, got %v", err) + } + if !strings.Contains(err.Error(), "returned HTML instead of JSON") { + t.Fatalf("expected helpful HTML error, got %v", err) + } + if !strings.Contains(err.Error(), "check api_base or proxy configuration") { + t.Fatalf("expected configuration hint, got %v", err) + } + }) } }