fix(provider): improve error handling and add retry tests for HTML responses

This commit is contained in:
tong3jie 2026-03-11 17:57:13 +08:00
parent aa5ee02507
commit f164e0d3a2
2 changed files with 78 additions and 8 deletions

View file

@ -14,6 +14,7 @@ import (
"time" "time"
"github.com/hashicorp/go-retryablehttp" "github.com/hashicorp/go-retryablehttp"
"github.com/sipeed/picoclaw/pkg/providers/protocoltypes" "github.com/sipeed/picoclaw/pkg/providers/protocoltypes"
) )
@ -87,7 +88,6 @@ func WithRetry(maxRetries int, minWait, maxWait time.Duration) Option {
} }
func NewProvider(apiKey, apiBase, proxy string, opts ...Option) *Provider { func NewProvider(apiKey, apiBase, proxy string, opts ...Option) *Provider {
retryClient := retryablehttp.NewClient() retryClient := retryablehttp.NewClient()
retryClient.RetryMax = 3 retryClient.RetryMax = 3
retryClient.RetryWaitMin = 1 * time.Second retryClient.RetryWaitMin = 1 * time.Second
@ -97,7 +97,6 @@ func NewProvider(apiKey, apiBase, proxy string, opts ...Option) *Provider {
transport := &http.Transport{} transport := &http.Transport{}
if proxy != "" { if proxy != "" {
if parsed, err := url.Parse(proxy); err == nil { if parsed, err := url.Parse(proxy); err == nil {
transport.Proxy = http.ProxyURL(parsed) transport.Proxy = http.ProxyURL(parsed)
} else { } else {
@ -114,6 +113,8 @@ func NewProvider(apiKey, apiBase, proxy string, opts ...Option) *Provider {
httpClient: retryClient.StandardClient(), httpClient: retryClient.StandardClient(),
} }
p.httpClient.Timeout = defaultRequestTimeout
for _, opt := range opts { for _, opt := range opts {
if opt != nil { if opt != nil {
opt(p) opt(p)
@ -268,7 +269,9 @@ func (p *Provider) resolveMaxTokenField(model string) string {
} }
lowerModel := strings.ToLower(model) lowerModel := strings.ToLower(model)
if strings.Contains(lowerModel, "o1") || strings.Contains(lowerModel, "glm-4") || strings.Contains(lowerModel, "gpt-5") { if strings.Contains(lowerModel, "o1") ||
strings.Contains(lowerModel, "glm-4") ||
strings.Contains(lowerModel, "gpt-5") {
return "max_completion_tokens" return "max_completion_tokens"
} }
return "max_tokens" return "max_tokens"

View file

@ -12,6 +12,8 @@ import (
"testing" "testing"
"time" "time"
"github.com/hashicorp/go-retryablehttp"
"github.com/sipeed/picoclaw/pkg/providers/protocoltypes" "github.com/sipeed/picoclaw/pkg/providers/protocoltypes"
) )
@ -252,7 +254,7 @@ func TestProviderChat_HTMLResponsesReturnHelpfulError(t *testing.T) {
{ {
name: "html error response", name: "html error response",
contentType: "text/html; charset=utf-8", contentType: "text/html; charset=utf-8",
statusCode: http.StatusBadGateway, statusCode: http.StatusOK,
body: "<!DOCTYPE html><html><body>bad gateway</body></html>", body: "<!DOCTYPE html><html><body>bad gateway</body></html>",
}, },
{ {
@ -272,7 +274,7 @@ func TestProviderChat_HTMLResponsesReturnHelpfulError(t *testing.T) {
})) }))
defer server.Close() defer server.Close()
p := NewProvider("key", server.URL, "") p := NewProvider("key", server.URL, "", WithRetry(0, time.Second*1, time.Second*3))
_, err := p.Chat(t.Context(), []Message{{Role: "user", Content: "hi"}}, nil, "gpt-4o", nil) _, err := p.Chat(t.Context(), []Message{{Role: "user", Content: "hi"}}, nil, "gpt-4o", nil)
if err == nil { if err == nil {
t.Fatal("expected error, got nil") t.Fatal("expected error, got nil")
@ -290,6 +292,65 @@ func TestProviderChat_HTMLResponsesReturnHelpfulError(t *testing.T) {
} }
} }
func TestProviderChatErrorPost(t *testing.T) {
tests := []struct {
name string
statusCode int
body string
reTry [3]int
}{
{
name: "retry on 502 with html response",
statusCode: http.StatusBadGateway,
body: "<!DOCTYPE html><html><body>gateway login</body></html>",
reTry: [3]int{0, 2, 3},
},
{
name: "retry times is 0",
statusCode: http.StatusBadGateway,
body: "<!DOCTYPE html><html><body>bad gateway</body></html>",
reTry: [3]int{1, 0, 0},
},
{
name: "default retry 3 times with 502 and html response",
statusCode: http.StatusBadGateway,
body: " \r\n\t<!DOCTYPE html><html><body>gateway login</body></html>",
},
}
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.WriteHeader(tt.statusCode)
_, _ = w.Write([]byte(tt.body))
}))
defer server.Close()
p := NewProvider(
"key",
server.URL,
"",
WithRetry(tt.reTry[0], time.Second*time.Duration(tt.reTry[1]), time.Second*time.Duration(tt.reTry[2])),
)
_, 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(), "failed to send request") {
t.Fatalf("expected status code in error, got %v", err)
}
times := 1
if tt.reTry[0] != 0 {
times = tt.reTry[0] + 1
}
if !strings.Contains(err.Error(), fmt.Sprintf("giving up after %d attempt(s)", times)) {
t.Fatalf("expected retry count in error, got %v", err)
}
})
}
}
func TestProviderChat_SuccessResponseUsesStreamingDecoder(t *testing.T) { func TestProviderChat_SuccessResponseUsesStreamingDecoder(t *testing.T) {
content := strings.Repeat("a", 1024) content := strings.Repeat("a", 1024)
body := `{"choices":[{"message":{"content":"` + content + `"},"finish_reason":"stop"}]}` body := `{"choices":[{"message":{"content":"` + content + `"},"finish_reason":"stop"}]}`
@ -323,7 +384,7 @@ func TestProviderChat_LargeHTMLResponsePreviewIsTruncated(t *testing.T) {
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
w.Header().Set("Content-Type", "text/html; charset=utf-8") w.Header().Set("Content-Type", "text/html; charset=utf-8")
w.WriteHeader(http.StatusBadGateway) w.WriteHeader(http.StatusOK)
_, _ = w.Write(body) _, _ = w.Write(body)
})) }))
defer server.Close() defer server.Close()
@ -454,12 +515,18 @@ func TestProvider_ProxyConfigured(t *testing.T) {
proxyURL := "http://127.0.0.1:8080" proxyURL := "http://127.0.0.1:8080"
p := NewProvider("key", "https://example.com", proxyURL) p := NewProvider("key", "https://example.com", proxyURL)
transport, ok := p.httpClient.Transport.(*http.Transport) transports, ok := p.httpClient.Transport.(*retryablehttp.RoundTripper)
if !ok || transports == nil {
t.Fatalf("expected retryablehttp transport, got %T", p.httpClient.Transport)
}
transport, ok := transports.Client.HTTPClient.Transport.(*http.Transport)
if !ok || transport == nil { if !ok || transport == nil {
t.Fatalf("expected http transport with proxy, got %T", p.httpClient.Transport) t.Fatalf("expected http transport with proxy, got %T", p.httpClient.Transport)
} }
req := &http.Request{URL: &url.URL{Scheme: "https", Host: "api.example.com"}} req := &http.Request{URL: &url.URL{Scheme: "https", Host: "api.example.com"}}
gotProxy, err := transport.Proxy(req) gotProxy, err := transport.Proxy(req)
if err != nil { if err != nil {
t.Fatalf("proxy function returned error: %v", err) t.Fatalf("proxy function returned error: %v", err)