diff --git a/agent/internal/catalog/emotes.go b/agent/internal/catalog/emotes.go index 218651464..b5045da1d 100644 --- a/agent/internal/catalog/emotes.go +++ b/agent/internal/catalog/emotes.go @@ -175,7 +175,7 @@ func (s *Server) twitchEmotes(ctx context.Context, token, channel string) ([]Emo } login := text(first(user["data"])["login"]) if login == "" { - return nil, unavailable() + return nil, upstreamError("twitch users", errors.New("provider response missing user login")) } // One Helix call lists everything the user can use, including the // channel's follower emotes. Tokens from before user:read:emotes was @@ -212,7 +212,7 @@ func (s *Server) twitchEmotes(ctx context.Context, token, channel string) ([]Emo func (s *Server) userEmotes(ctx context.Context, token, userID, channel string) ([]twitchEmote, error) { if userID == "" { - return nil, unavailable() + return nil, upstreamError("twitch user emotes", errors.New("provider response missing user ID")) } result := []twitchEmote{} after := "" @@ -234,7 +234,7 @@ func (s *Server) userEmotes(ctx context.Context, token, userID, channel string) return result, nil } } - return nil, unavailable() + return nil, upstreamError("twitch user emotes", errors.New("provider response exceeded 50 pages")) } // setEmotes resolves the emote sets IRC reports, 25 sets per Helix request, @@ -249,7 +249,7 @@ func (s *Server) setEmotes(ctx context.Context, token, login string) ([]twitchEm return []twitchEmote{}, nil } if len(sets) > 5000 { - return nil, unavailable() + return nil, upstreamError("twitch emote sets", errors.New("provider response exceeded 5000 emote sets")) } batches := make([][]twitchEmote, (len(sets)+24)/25) var group errgroup.Group diff --git a/agent/internal/catalog/error_context_test.go b/agent/internal/catalog/error_context_test.go new file mode 100644 index 000000000..fc0571b19 --- /dev/null +++ b/agent/internal/catalog/error_context_test.go @@ -0,0 +1,95 @@ +package catalog + +import ( + "context" + "errors" + "net/http" + "net/url" + "strings" + "testing" + + "github.com/muxable/rtchat/agent/internal/callable" + "github.com/muxable/rtchat/agent/internal/clients" + "go.uber.org/zap" + "go.uber.org/zap/zaptest/observer" +) + +func TestEmoteProviderFailureKeepsLogContextAndGenericResponse(t *testing.T) { + for _, tc := range []struct { + name, want string + status int + body string + transport bool + }{ + {name: "rate limit", status: 429, body: `{"message":"secret-provider-body"}`, want: "provider returned HTTP 429"}, + {name: "upstream failure", status: 502, body: "secret-provider-body", want: "provider returned HTTP 502"}, + {name: "invalid JSON", status: 200, body: "secret-provider-body", want: "invalid provider response"}, + {name: "missing user", status: 200, body: `{"data":[]}`, want: "provider response missing user login"}, + {name: "transport failure", transport: true, want: "provider transport failure"}, + } { + t.Run(tc.name, func(t *testing.T) { + s, _, tokens := fixture() + tokens.user["uid:twitch"] = "secret-access-token" + s.HTTP = &http.Client{Transport: transport(func(r *http.Request) (*http.Response, error) { + if r.URL.Host == "api.twitch.tv" { + if tc.transport { + return nil, errors.New("transport failed with secret-access-token") + } + return response(tc.status, tc.body), nil + } + return response(200, `{}`), nil + })} + core, logs := observer.New(zap.ErrorLevel) + defer zap.ReplaceGlobals(zap.New(core))() + status, result := invoke(t, s, "getEmotes", `{"provider":"twitch","channelId":"123"}`, "uid") + failure := object(result["error"]) + if status != 503 || failure["status"] != "UNAVAILABLE" || failure["message"] != "provider temporarily unavailable" { + t.Fatalf("client response changed: status=%d body=%v", status, result) + } + if logs.Len() != 1 { + t.Fatalf("got %d error logs, want one", logs.Len()) + } + message, _ := logs.All()[0].ContextMap()["error"].(string) + if !strings.Contains(message, "twitch users") || !strings.Contains(message, tc.want) { + t.Fatalf("upstream context missing: %q", message) + } + if strings.Contains(message, "secret-") { + t.Fatalf("provider credentials or response body leaked: %q", message) + } + }) + } +} + +func TestTwitchFailurePreservesErrorTypes(t *testing.T) { + s, _, _ := fixture() + s.HTTP = &http.Client{Transport: transport(func(*http.Request) (*http.Response, error) { + return response(503, `{}`), nil + })} + _, err := s.twitch(context.Background(), "users", "token", url.Values{}) + var apiError *callable.Error + var upstream *clients.HTTPError + if !errors.As(err, &apiError) || apiError.Code != "unavailable" || !errors.As(err, &upstream) || upstream.Status != 503 { + t.Fatalf("error types lost: %v", err) + } +} + +// A token-store failure must remain distinguishable from a Twitch HTTP failure. +type errorTokens struct { + Tokens + cause error +} + +func (f errorTokens) AccessToken(context.Context, string, string) (string, error) { + return "", f.cause +} + +func TestUserTokenFailurePreservesCause(t *testing.T) { + s, _, _ := fixture() + cause := errors.New("token storage unavailable") + s.Tokens = errorTokens{Tokens: s.Tokens, cause: cause} + _, err := s.userToken(context.Background(), "uid") + var apiError *callable.Error + if !errors.Is(err, cause) || !errors.As(err, &apiError) || apiError.Code != "unavailable" || !strings.Contains(err.Error(), "twitch access token") { + t.Fatalf("token failure context lost: %v", err) + } +} diff --git a/agent/internal/catalog/server.go b/agent/internal/catalog/server.go index 748b241dd..6e86ac099 100644 --- a/agent/internal/catalog/server.go +++ b/agent/internal/catalog/server.go @@ -49,6 +49,11 @@ func unavailable() error { return &callable.Error{Code: "unavailable", Message: "provider temporarily unavailable"} } +// Keep redacted provider context in logs while returning the public callable error. +func upstreamError(operation string, cause error) error { + return fmt.Errorf("%s: %w: %w", operation, unavailable(), cause) +} + var numericID = regexp.MustCompile(`^[0-9]{1,30}$`) func parseChannel(data json.RawMessage, requireChannel bool) (string, string, error) { @@ -120,7 +125,7 @@ func (s *Server) twitch(ctx context.Context, path, token string, q url.Values) ( if errors.As(err, &response) && (response.Status == 401 || response.Status == 403) { return nil, &callable.Error{Code: "permission-denied", Message: "provider did not authorize this request"} } - return nil, unavailable() + return nil, upstreamError("twitch "+path, err) } return result, nil } @@ -133,7 +138,7 @@ func (s *Server) userToken(ctx context.Context, uid string) (string, error) { return "", provider.ErrNoToken } if err != nil { - return "", unavailable() + return "", upstreamError("twitch access token", err) } return token, nil }