Skip to content

fix(openai_compat): clarify HTML response parse errors #1075

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
afjcjsbx merged 6 commits into from
Mar 7, 2026
+247 −8
Merged

fix(openai_compat): clarify HTML response parse errors #1075

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
4946a8b
fix(openai_compat): clarify HTML response errors
qs3c Mar 4, 2026
a305c0a
fix(openai_compat): avoid predeclared identifier in preview
qs3c Mar 4, 2026
9216cd1
fix(openai_compat): handle html error bodies and reduce allocations
qs3c Mar 5, 2026
c1a3876
fix: improve error handling for non-JSON responses by checking conten…
qs3c Mar 5, 2026
6eaa49f
fix: improve openai compat HTML response handling
qs3c Mar 7, 2026
53cba73
fix: resolve openai compat lint issues
qs3c Mar 7, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
84 changes: 82 additions & 2 deletions pkg/providers/openai_compat/provider.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"bytes"
"context"
"encoding/json"
"errors"
"fmt"
"io"
"log"
Expand Down Expand Up @@ -189,10 +190,89 @@ func (p *Provider) Chat(
}

if resp.StatusCode != http.StatusOK {
Copy link
Copy Markdown
Collaborator

@afjcjsbx afjcjsbx Mar 5, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Currently, if the server returns a 502 Bad Gateway (typical of nginx proxies) with an HTML page, the code stops there.

This means that if a proxy goes wrong by returning a 502 HTML, your new wrapResponseParseError wrapper is not invoked, because it only kicks in if the status code is 200 OK (which is rarely the case if the endpoint is broken and returns HTML).

How do you want to handle this? If the goal is to intercept HTML and show your error message even when there is an HTTP error (such as 404 Not Found, 502 Bad Gateway, 503 Service Unavailable, often accompanied by HTML), you may want to invoke your HTML check even within the resp.StatusCode != http.StatusOK block.

Copy link
Copy Markdown
Contributor Author

@qs3c qs3c Mar 5, 2026 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch, fixed in 9216cd1. For non-200 responses we now run the same HTML diagnostic path via wrapHTTPResponseError(...), so 404/502/503 HTML pages also return the actionable api_base/proxy hint instead of only raw body output. Added TestProviderChat_HTMLErrorResponseReturnsHelpfulError (502 case).

return nil, fmt.Errorf("API request failed:\n Status: %d\n Body: %s", resp.StatusCode, string(body))
return nil, wrapHTTPResponseError(resp.StatusCode, body, resp.Header.Get("Content-Type"), p.apiBase)
}

return parseResponse(body)
out, err := parseResponse(body)
if err != nil {
return nil, wrapResponseParseError(err, body, resp.Header.Get("Content-Type"), p.apiBase)
}

return out, nil
}
Copy link
Copy Markdown
Collaborator

@afjcjsbx afjcjsbx Mar 5, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

here we could simplify the code in this way:

    contentType := resp.Header.Get("Content-Type")

	// check if there is an HTTP error (caused by proxy or gateway) or if the response is HTML 
	if resp.StatusCode != http.StatusOK || strings.Contains(strings.ToLower(contentType), "text/html") {
		body, _ := io.ReadAll(resp.Body)
		return nil, wrapHTTPResponseError(resp.StatusCode, body, contentType, p.apiBase)
	}

	// directly pass the stream (resp.Body) to the JSON parser without loading everything into memory
	out, err := parseResponse(resp.Body)
	if err != nil {
		// Note: if it fails here, we do not have the full body in memory for HTML inspection, 
		// but having already checked the Content-Type above, the error is genuinely related to JSON parsing.
		return nil, fmt.Errorf("failed to parse JSON response: %w", err)
	}

	return out, nil
}

WDYT??


func wrapResponseParseError(err error, body []byte, contentType, apiBase string) error {
if message, ok := htmlResponseMessage(body, contentType, apiBase); ok {
return errors.New(message)
}
return err
}

func wrapHTTPResponseError(statusCode int, body []byte, contentType, apiBase string) error {
if message, ok := htmlResponseMessage(body, contentType, apiBase); ok {
return fmt.Errorf("API request failed:\n Status: %d\n Detail: %s", statusCode, message)
}
return fmt.Errorf("API request failed:\n Status: %d\n Body: %s", statusCode, string(body))
}

func htmlResponseMessage(body []byte, contentType, apiBase string) (string, bool) {
trimmedContentType := strings.TrimSpace(contentType)
if !looksLikeHTML(body, trimmedContentType) {
return "", false
}

contentTypeHint := ""
if trimmedContentType != "" {
contentTypeHint = fmt.Sprintf(" (content-type: %s)", trimmedContentType)
}

return fmt.Sprintf(
"expected JSON response from %s/chat/completions, but received HTML%s; check api_base or proxy configuration. Response preview: %s",
apiBase,
contentTypeHint,
responsePreview(body, 160),
), true
}

func looksLikeHTML(body []byte, contentType string) bool {
contentType = strings.ToLower(strings.TrimSpace(contentType))
if strings.Contains(contentType, "text/html") || strings.Contains(contentType, "application/xhtml+xml") {
return true
}

trimmed := strings.ToLower(string(leadingTrimmedPrefix(body, 128)))
return strings.HasPrefix(trimmed, "<!doctype html") ||
Copy link
Copy Markdown
Collaborator

@afjcjsbx afjcjsbx Mar 5, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If the body contains a very large HTML page or a huge string, converting the whole byte array to string, doing TrimSpace and then ToLower on the whole body allocates a lot of memory unnecessarily, since you only care about the very first few characters. You can optimize by reading only the first N bytes ignoring the initial whitespace.

Copy link
Copy Markdown
Contributor Author

@qs3c qs3c Mar 5, 2026 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Addressed in 9216cd1. looksLikeHTML no longer converts the whole body; it now inspects only a trimmed prefix (leadingTrimmedPrefix, 128 bytes) before lowercase/prefix checks. I also switched responsePreview to bytes.TrimSpace plus bounded string conversion to avoid large allocations.

strings.HasPrefix(trimmed, "<html") ||
strings.HasPrefix(trimmed, "<head") ||
strings.HasPrefix(trimmed, "<body")
}

func leadingTrimmedPrefix(body []byte, maxLen int) []byte {
i := 0
for i < len(body) {
switch body[i] {
case ' ', '\t', '\n', '\r', '\f', '\v':
i++
default:
end := i + maxLen
if end > len(body) {
end = len(body)
}
return body[i:end]
}
}
return nil
}

func responsePreview(body []byte, maxLen int) string {
trimmed := bytes.TrimSpace(body)
if len(trimmed) == 0 {
return "<empty>"
}
if len(trimmed) <= maxLen {
return string(trimmed)
}
return string(trimmed[:maxLen]) + "..."
}

func parseResponse(body []byte) (*LLMResponse, error) {
Expand Down
53 changes: 53 additions & 0 deletions pkg/providers/openai_compat/provider_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package openai_compat

import (
"bytes"
"encoding/json"
"net/http"
"net/http/httptest"
Expand Down Expand Up @@ -212,6 +213,58 @@ func TestProviderChat_HTTPError(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("<!DOCTYPE html><html><body>gateway login</body></html>"))
}))
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(), "received HTML") {
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_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("<!DOCTYPE html><html><body>bad gateway</body></html>"))
}))
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(), "received HTML") {
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 TestLooksLikeHTML_SniffsPrefixWithLargeBody(t *testing.T) {
body := append([]byte(" \r\n\t<!DOCTYPE html><html><body>x</body></html>"), bytes.Repeat([]byte("A"), 1024*1024)...)
if !looksLikeHTML(body, "") {
t.Fatal("expected looksLikeHTML to detect html prefix")
}
}

func TestProviderChat_StripsMoonshotPrefixAndNormalizesKimiTemperature(t *testing.T) {
var requestBody map[string]any

Expand Down
Loading