fix(providers): surface friendly auth error messages (#3198)

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
 }
 

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."
+	}
+}

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)
+	}
+}

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
+}

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

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.

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("<html>bad</html>"),
+		"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())
+	}
+}

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{