diff --git a/pkg/agent/agent_outbound.go b/pkg/agent/agent_outbound.go index f5de6fa41..b19ea75ae 100644 --- a/pkg/agent/agent_outbound.go +++ b/pkg/agent/agent_outbound.go @@ -6,7 +6,6 @@ import ( "context" "encoding/json" "errors" - "fmt" "strings" "time" @@ -21,7 +20,7 @@ func (al *AgentLoop) maybePublishError(ctx context.Context, channel, chatID, ses if errors.Is(err, context.Canceled) { return false } - al.PublishResponseIfNeeded(ctx, channel, chatID, sessionKey, fmt.Sprintf("Error processing message: %v", err)) + al.PublishResponseIfNeeded(ctx, channel, chatID, sessionKey, formatProcessingError(err)) return true } diff --git a/pkg/agent/error_format.go b/pkg/agent/error_format.go new file mode 100644 index 000000000..0d0106060 --- /dev/null +++ b/pkg/agent/error_format.go @@ -0,0 +1,36 @@ +package agent + +import ( + "fmt" + + "github.com/sipeed/picoclaw/pkg/providers" +) + +func formatProcessingError(err error) string { + if err == nil { + return "" + } + + if kind, ok := providers.ClassifyAuthError(err); ok { + return fmt.Sprintf( + "Error processing message: %s\n\nOriginal error:\n%s", + authErrorFriendlyMessage(kind), + err.Error(), + ) + } + + return fmt.Sprintf("Error processing message: %v", err) +} + +func authErrorFriendlyMessage(kind providers.AuthErrorKind) string { + switch kind { + case providers.AuthErrorInvalidAPIKey: + return "Authentication failed: the API key appears to be invalid. Check the API key configured for this model or provider." + case providers.AuthErrorMissingAPIKey: + return "Authentication failed: no API key is configured for this model or provider. Add an API key in the model settings or config." + case providers.AuthErrorExpiredToken: + return "Authentication failed: the saved login or token appears to be expired. Re-authenticate the provider." + default: + return "Authentication failed: check the API key, token, OAuth login, or provider permissions for this model." + } +} diff --git a/pkg/agent/error_format_test.go b/pkg/agent/error_format_test.go new file mode 100644 index 000000000..531f9ebb0 --- /dev/null +++ b/pkg/agent/error_format_test.go @@ -0,0 +1,52 @@ +package agent + +import ( + "errors" + "strings" + "testing" + + "github.com/sipeed/picoclaw/pkg/providers/common" +) + +func TestFormatProcessingError_InvalidAPIKey(t *testing.T) { + err := errors.New( + `LLM call failed after retries: API request failed: Status: 401 Body: {"error":{"message":"Incorrect API key provided"}}`, + ) + + got := formatProcessingError(err) + if !strings.Contains(got, "API key appears to be invalid") { + t.Fatalf("formatted error missing friendly API key hint: %q", got) + } + if !strings.Contains(got, "Original error:") { + t.Fatalf("formatted error missing original error label: %q", got) + } + if !strings.Contains(got, err.Error()) { + t.Fatalf("formatted error missing original error: %q", got) + } +} + +func TestFormatProcessingError_GenericAuthHTTPError(t *testing.T) { + err := &common.HTTPError{ + StatusCode: 401, + BodyPreview: `{"error":"unauthorized"}`, + ContentType: "application/json", + APIBase: "https://api.example.com", + } + + got := formatProcessingError(err) + if !strings.Contains(got, "check the API key, token, OAuth login, or provider permissions") { + t.Fatalf("formatted error missing generic auth hint: %q", got) + } + if !strings.Contains(got, "Original error:") { + t.Fatalf("formatted error missing original error: %q", got) + } +} + +func TestFormatProcessingError_NonAuth(t *testing.T) { + err := errors.New("connection reset by peer") + got := formatProcessingError(err) + want := "Error processing message: connection reset by peer" + if got != want { + t.Fatalf("formatted error = %q, want %q", got, want) + } +} diff --git a/pkg/providers/auth_error.go b/pkg/providers/auth_error.go new file mode 100644 index 000000000..92a79da60 --- /dev/null +++ b/pkg/providers/auth_error.go @@ -0,0 +1,142 @@ +package providers + +import ( + "errors" + "regexp" + "strings" + + "github.com/sipeed/picoclaw/pkg/providers/common" +) + +type AuthErrorKind string + +const ( + AuthErrorInvalidAPIKey AuthErrorKind = "invalid_api_key" + AuthErrorMissingAPIKey AuthErrorKind = "missing_api_key" + AuthErrorExpiredToken AuthErrorKind = "expired_token" + AuthErrorGeneric AuthErrorKind = "auth" +) + +var ( + invalidAPIKeyPattern = regexp.MustCompile( + `(?i)\b(?:invalid|incorrect|malformed|wrong)[-_\s]+(?:api[-_\s]*)?key\b|\b(?:api[-_\s]*)?key[-_\s]+(?:is[-_\s]+)?(?:invalid|incorrect|malformed|wrong)\b|\binvalid[-_]?api[-_]?key\b`, + ) + missingAPIKeyPattern = regexp.MustCompile( + `(?i)\b(?:missing|no|not[-_\s]+configured)[-_\s]+(?:api[-_\s]*)?key\b|\bno[-_\s]+credentials[-_\s]+found\b|\bapi[-_\s]+key[-_\s]+not[-_\s]+configured\b`, + ) + expiredTokenPattern = regexp.MustCompile( + `(?i)\b(?:oauth[-_\s]+token[-_\s]+refresh[-_\s]+failed|re-authenticate|token[-_\s]+(?:has[-_\s]+)?expired|expired[-_\s]+token)\b`, + ) + genericAuthPattern = regexp.MustCompile( + `(?i)\b(?:unauthorized|forbidden|authentication[-_\s]+(?:failed|required)|access[-_\s]+denied)\b`, + ) +) + +func ClassifyAuthError(err error) (AuthErrorKind, bool) { + if err == nil { + return "", false + } + + var exhausted *FallbackExhaustedError + if errors.As(err, &exhausted) && exhausted != nil { + return classifyFallbackExhaustedAuthError(exhausted) + } + + msg := authErrorText(err) + if missingAPIKeyPattern.MatchString(msg) { + return AuthErrorMissingAPIKey, true + } + if expiredTokenPattern.MatchString(msg) { + return AuthErrorExpiredToken, true + } + if invalidAPIKeyPattern.MatchString(msg) { + return AuthErrorInvalidAPIKey, true + } + + if hasStructuredAuthError(err) || genericAuthPattern.MatchString(msg) { + return AuthErrorGeneric, true + } + return "", false +} + +func authErrorText(err error) string { + var parts []string + if err != nil { + parts = append(parts, err.Error()) + } + + var httpErr *common.HTTPError + if errors.As(err, &httpErr) && httpErr != nil { + parts = append(parts, httpErr.BodyPreview) + } + + return strings.Join(parts, "\n") +} + +func hasStructuredAuthError(err error) bool { + var failErr *FailoverError + if errors.As(err, &failErr) && failErr != nil && failErr.Reason == FailoverAuth { + return true + } + + var httpErr *common.HTTPError + if errors.As(err, &httpErr) && httpErr != nil { + return httpErr.StatusCode == 401 || httpErr.StatusCode == 403 + } + + return false +} + +func classifyFallbackExhaustedAuthError(err *FallbackExhaustedError) (AuthErrorKind, bool) { + if err == nil { + return "", false + } + + var authMessages []string + nonSkipped := 0 + for _, attempt := range err.Attempts { + if attempt.Skipped { + continue + } + nonSkipped++ + if !attemptIsAuthFailure(attempt) { + return "", false + } + if attempt.Error != nil { + authMessages = append(authMessages, authErrorText(attempt.Error)) + } + } + if nonSkipped == 0 { + return "", false + } + + msg := strings.Join(authMessages, "\n") + if missingAPIKeyPattern.MatchString(msg) { + return AuthErrorMissingAPIKey, true + } + if expiredTokenPattern.MatchString(msg) { + return AuthErrorExpiredToken, true + } + if invalidAPIKeyPattern.MatchString(msg) { + return AuthErrorInvalidAPIKey, true + } + return AuthErrorGeneric, true +} + +func attemptIsAuthFailure(attempt FallbackAttempt) bool { + if attempt.Reason == FailoverAuth { + return true + } + if attempt.Error == nil { + return false + } + var failErr *FailoverError + if errors.As(attempt.Error, &failErr) && failErr != nil && failErr.Reason == FailoverAuth { + return true + } + var httpErr *common.HTTPError + if errors.As(attempt.Error, &httpErr) && httpErr != nil { + return httpErr.StatusCode == 401 || httpErr.StatusCode == 403 + } + return false +} diff --git a/pkg/providers/auth_error_test.go b/pkg/providers/auth_error_test.go new file mode 100644 index 000000000..09f53cd13 --- /dev/null +++ b/pkg/providers/auth_error_test.go @@ -0,0 +1,133 @@ +package providers + +import ( + "errors" + "fmt" + "testing" + + "github.com/sipeed/picoclaw/pkg/providers/common" +) + +func TestClassifyError_HTTPErrorStatus(t *testing.T) { + err := fmt.Errorf("provider request: %w", &common.HTTPError{ + StatusCode: httpStatusUnauthorized, + BodyPreview: `{"error":"unauthorized"}`, + ContentType: "application/json", + APIBase: "https://api.example.com", + }) + + result := ClassifyError(err, "openai", "gpt-4") + if result == nil { + t.Fatal("expected classified error") + } + if result.Reason != FailoverAuth { + t.Fatalf("reason = %q, want %q", result.Reason, FailoverAuth) + } + if result.Status != httpStatusUnauthorized { + t.Fatalf("status = %d, want %d", result.Status, httpStatusUnauthorized) + } +} + +func TestClassifyAuthError(t *testing.T) { + tests := []struct { + name string + err error + want AuthErrorKind + }{ + { + name: "invalid api key", + err: errors.New( + `API request failed: Status: 401 Body: {"error":{"message":"Incorrect API key provided"}}`, + ), + want: AuthErrorInvalidAPIKey, + }, + { + name: "missing api key", + err: errors.New("API key not configured"), + want: AuthErrorMissingAPIKey, + }, + { + name: "expired token", + err: errors.New("oauth token refresh failed: token has expired"), + want: AuthErrorExpiredToken, + }, + { + name: "structured generic auth", + err: &common.HTTPError{ + StatusCode: httpStatusUnauthorized, + BodyPreview: `{"error":"unauthorized"}`, + ContentType: "application/json", + APIBase: "https://api.example.com", + }, + want: AuthErrorGeneric, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got, ok := ClassifyAuthError(tt.err) + if !ok { + t.Fatal("expected auth classification") + } + if got != tt.want { + t.Fatalf("kind = %q, want %q", got, tt.want) + } + }) + } +} + +func TestClassifyAuthError_FallbackExhaustedAllAuth(t *testing.T) { + err := &FallbackExhaustedError{ + Attempts: []FallbackAttempt{ + { + Reason: FailoverAuth, + Error: &FailoverError{ + Reason: FailoverAuth, + Wrapped: errors.New("invalid api key"), + }, + }, + { + Reason: FailoverAuth, + Error: &FailoverError{ + Reason: FailoverAuth, + Wrapped: errors.New("unauthorized"), + }, + }, + }, + } + + got, ok := ClassifyAuthError(err) + if !ok { + t.Fatal("expected auth classification") + } + if got != AuthErrorInvalidAPIKey { + t.Fatalf("kind = %q, want %q", got, AuthErrorInvalidAPIKey) + } +} + +func TestClassifyAuthError_FallbackExhaustedMixedFailures(t *testing.T) { + err := &FallbackExhaustedError{ + Attempts: []FallbackAttempt{ + { + Reason: FailoverAuth, + Error: &FailoverError{ + Reason: FailoverAuth, + Wrapped: errors.New("invalid api key"), + }, + }, + { + Reason: FailoverRateLimit, + Error: &FailoverError{ + Reason: FailoverRateLimit, + Wrapped: errors.New("rate limit exceeded"), + }, + }, + }, + } + + if got, ok := ClassifyAuthError(err); ok { + t.Fatalf("kind = %q, want no auth classification for mixed failures", got) + } +} + +const httpStatusUnauthorized = 401 diff --git a/pkg/providers/common/common.go b/pkg/providers/common/common.go index 5e03bc0c2..10bdddd35 100644 --- a/pkg/providers/common/common.go +++ b/pkg/providers/common/common.go @@ -371,6 +371,36 @@ func DecodeToolCallArguments(raw json.RawMessage, name string) map[string]any { // --- HTTP response helpers --- +// HTTPError captures structured details for non-OK provider HTTP responses. +// Error preserves the previous user/log-facing text format for compatibility. +type HTTPError struct { + StatusCode int + BodyPreview string + ContentType string + APIBase string + IsHTML bool +} + +func (e *HTTPError) Error() string { + if e == nil { + return "API request failed" + } + if e.IsHTML { + return fmt.Sprintf( + "API request failed: %s returned HTML instead of JSON (content-type: %s); check api_base or proxy configuration.\n Status: %d\n Body: %s", + e.APIBase, + e.ContentType, + e.StatusCode, + e.BodyPreview, + ) + } + return fmt.Sprintf( + "API request failed:\n Status: %d\n Body: %s", + e.StatusCode, + e.BodyPreview, + ) +} + // HandleErrorResponse reads a non-200 response body and returns an appropriate error. func HandleErrorResponse(resp *http.Response, apiBase string) error { contentType := resp.Header.Get("Content-Type") @@ -381,11 +411,12 @@ func HandleErrorResponse(resp *http.Response, apiBase string) error { if LooksLikeHTML(body, contentType) { return WrapHTMLResponseError(resp.StatusCode, body, contentType, apiBase) } - return fmt.Errorf( - "API request failed:\n Status: %d\n Body: %s", - resp.StatusCode, - ResponsePreview(body, 128), - ) + return &HTTPError{ + StatusCode: resp.StatusCode, + BodyPreview: ResponsePreview(body, 128), + ContentType: contentType, + APIBase: apiBase, + } } // ReadAndParseResponse peeks at the response body to detect HTML errors, @@ -422,14 +453,13 @@ func LooksLikeHTML(body []byte, contentType string) bool { // WrapHTMLResponseError creates a descriptive error for HTML responses. 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 &HTTPError{ + StatusCode: statusCode, + BodyPreview: ResponsePreview(body, 128), + ContentType: contentType, + APIBase: apiBase, + IsHTML: true, + } } // ResponsePreview returns a truncated preview of response body for error messages. diff --git a/pkg/providers/common/http_error_test.go b/pkg/providers/common/http_error_test.go new file mode 100644 index 000000000..96be6662c --- /dev/null +++ b/pkg/providers/common/http_error_test.go @@ -0,0 +1,59 @@ +package common + +import ( + "errors" + "net/http" + "net/http/httptest" + "strings" + "testing" +) + +func TestHandleErrorResponse_ReturnsHTTPError(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.StatusUnauthorized) + _, _ = w.Write([]byte(`{"error":"unauthorized"}`)) + })) + defer server.Close() + + resp, err := http.Get(server.URL) + if err != nil { + t.Fatalf("http.Get() error = %v", err) + } + defer resp.Body.Close() + + err = HandleErrorResponse(resp, server.URL) + var httpErr *HTTPError + if !errors.As(err, &httpErr) { + t.Fatalf("HandleErrorResponse() error = %T, want *HTTPError", err) + } + if httpErr.StatusCode != http.StatusUnauthorized { + t.Fatalf("StatusCode = %d, want %d", httpErr.StatusCode, http.StatusUnauthorized) + } + if httpErr.BodyPreview != `{"error":"unauthorized"}` { + t.Fatalf("BodyPreview = %q", httpErr.BodyPreview) + } + if !strings.Contains(err.Error(), "Status: 401") { + t.Fatalf("Error() should preserve status text, got %q", err.Error()) + } +} + +func TestWrapHTMLResponseError_ReturnsHTTPError(t *testing.T) { + err := WrapHTMLResponseError( + http.StatusBadGateway, + []byte("bad"), + "text/html", + "https://api.example.com", + ) + + var httpErr *HTTPError + if !errors.As(err, &httpErr) { + t.Fatalf("WrapHTMLResponseError() error = %T, want *HTTPError", err) + } + if !httpErr.IsHTML { + t.Fatal("expected IsHTML") + } + if !strings.Contains(err.Error(), "HTML instead of JSON") { + t.Fatalf("Error() should preserve HTML message, got %q", err.Error()) + } +} diff --git a/pkg/providers/error_classifier.go b/pkg/providers/error_classifier.go index caddd5895..31e7b1cb6 100644 --- a/pkg/providers/error_classifier.go +++ b/pkg/providers/error_classifier.go @@ -8,6 +8,8 @@ import ( "regexp" "strings" "syscall" + + "github.com/sipeed/picoclaw/pkg/providers/common" ) // Common patterns in Go HTTP error messages @@ -88,6 +90,8 @@ var ( } authPatterns = []errorPattern{ + rxp(`\b(?:invalid|incorrect|malformed|wrong)[-_\s]+(?:api[-_\s]*)?key\b`), + rxp(`\b(?:api[-_\s]*)?key[-_\s]+(?:is[-_\s]+)?(?:invalid|incorrect|malformed|wrong)\b`), rxp(`invalid[_ ]?api[_ ]?key`), substr("incorrect api key"), substr("invalid token"), @@ -188,6 +192,18 @@ func ClassifyError(err error, provider, model string) *FailoverError { } // Try HTTP status code extraction first. + var httpErr *common.HTTPError + if errors.As(err, &httpErr) && httpErr != nil { + if reason := classifyByStatus(httpErr.StatusCode); reason != "" { + return &FailoverError{ + Reason: reason, + Provider: provider, + Model: model, + Status: httpErr.StatusCode, + Wrapped: err, + } + } + } if status := extractHTTPStatus(msg); status > 0 { if reason := classifyByStatus(status); reason != "" { return &FailoverError{