From b125940cffa77cc21775e8ff4029f7b5ac486944 Mon Sep 17 00:00:00 2001 From: John Allers Date: Wed, 4 Feb 2026 07:20:40 -0500 Subject: [PATCH 1/3] fix: improve rate limit and temporary error handling in wrapGitHubError When go-github blocks requests client-side due to rate limits, it returns a RateLimitError with a synthetic 403 response that has empty headers. The existing isRatelimited() check failed because it expects the X-Ratelimit-Remaining header to equal "0", but the synthetic response has no headers at all. This caused rate limit errors to be misclassified as PermissionDenied, which is not retried by the SDK. Changes: - Check for *github.RateLimitError and *github.AbuseRateLimitError types using errors.As() before checking HTTP status codes - Extract rate limit data (reset time, limit, remaining) from the error and attach it to the gRPC status details for proper backoff - Add isTemporarilyUnavailable() to handle 503, 502, and 504 errors - Return codes.Unavailable for all these cases, enabling SDK retry logic Co-Authored-By: Claude Opus 4.5 --- pkg/connector/helpers.go | 66 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 66 insertions(+) diff --git a/pkg/connector/helpers.go b/pkg/connector/helpers.go index d144862e..c11fd51b 100644 --- a/pkg/connector/helpers.go +++ b/pkg/connector/helpers.go @@ -2,10 +2,12 @@ package connector import ( "context" + "errors" "fmt" "net/http" "strconv" "strings" + "time" v2 "github.com/conductorone/baton-sdk/pb/c1/connector/v2" "github.com/conductorone/baton-sdk/pkg/annotations" @@ -18,6 +20,7 @@ import ( "golang.org/x/text/cases" "golang.org/x/text/language" "google.golang.org/grpc/codes" + "google.golang.org/grpc/status" "google.golang.org/protobuf/types/known/timestamppb" ) @@ -188,6 +191,38 @@ func extractRateLimitData(response *github.Response) (*v2.RateLimitDescription, }, nil } +// rateLimitDescriptionFromRate creates a RateLimitDescription from a github.Rate struct. +// This is used when go-github returns a RateLimitError with rate info but a synthetic response. +func rateLimitDescriptionFromRate(rate github.Rate) *v2.RateLimitDescription { + return &v2.RateLimitDescription{ + Status: v2.RateLimitDescription_STATUS_OVERLIMIT, + Limit: int64(rate.Limit), + Remaining: int64(rate.Remaining), + ResetAt: timestamppb.New(rate.Reset.Time), + } +} + +// rateLimitDescriptionFromRetryAfter creates a RateLimitDescription from a retry-after duration. +// This is used for AbuseRateLimitError which provides a RetryAfter duration. +func rateLimitDescriptionFromRetryAfter(retryAfter *time.Duration) *v2.RateLimitDescription { + desc := &v2.RateLimitDescription{ + Status: v2.RateLimitDescription_STATUS_OVERLIMIT, + } + if retryAfter != nil { + desc.ResetAt = timestamppb.New(time.Now().Add(*retryAfter)) + } + return desc +} + +// wrapErrorWithRateLimitDetails creates a gRPC error with rate limit details attached. +func wrapErrorWithRateLimitDetails(code codes.Code, msg string, rlDesc *v2.RateLimitDescription, err error) error { + st := status.New(code, msg) + if rlDesc != nil { + st, _ = st.WithDetails(rlDesc) + } + return errors.Join(st.Err(), err) +} + type listUsersQuery struct { Organization struct { SamlIdentityProvider struct { @@ -256,6 +291,15 @@ func isPermissionError(resp *github.Response) bool { return resp.StatusCode == http.StatusForbidden } +func isTemporarilyUnavailable(resp *github.Response) bool { + if resp == nil { + return false + } + return resp.StatusCode == http.StatusServiceUnavailable || + resp.StatusCode == http.StatusBadGateway || + resp.StatusCode == http.StatusGatewayTimeout +} + // wrapGitHubError wraps GitHub API errors with appropriate gRPC status codes based on the HTTP response. // It handles rate limiting, authentication errors, permission errors, and generic errors. // The contextMsg parameter should describe the operation that failed (e.g., "failed to list teams"). @@ -264,9 +308,31 @@ func wrapGitHubError(err error, resp *github.Response, contextMsg string) error return nil } + // Check for go-github rate limit error types FIRST. + // These may have synthetic responses with empty headers when the client + // blocks requests without making an actual HTTP call. + var rateLimitErr *github.RateLimitError + if errors.As(err, &rateLimitErr) { + rlDesc := rateLimitDescriptionFromRate(rateLimitErr.Rate) + return wrapErrorWithRateLimitDetails(codes.Unavailable, "rate limit exceeded", rlDesc, err) + } + + var abuseRateLimitErr *github.AbuseRateLimitError + if errors.As(err, &abuseRateLimitErr) { + rlDesc := rateLimitDescriptionFromRetryAfter(abuseRateLimitErr.RetryAfter) + return wrapErrorWithRateLimitDetails(codes.Unavailable, "secondary rate limit exceeded", rlDesc, err) + } + + // Check response-based rate limiting (real 429 or 403 with header) if isRatelimited(resp) { return uhttp.WrapErrors(codes.Unavailable, "too many requests", err) } + + // Check for temporary server errors (503, 502, 504) + if isTemporarilyUnavailable(resp) { + return uhttp.WrapErrors(codes.Unavailable, "service temporarily unavailable", err) + } + if isAuthError(resp) { return uhttp.WrapErrors(codes.Unauthenticated, contextMsg, err) } From 927707fe3df0110ae6f5e0198eb6bf46816dd0b6 Mon Sep 17 00:00:00 2001 From: John Allers Date: Thu, 5 Feb 2026 09:21:07 -0500 Subject: [PATCH 2/3] fix: add zero-time check in rateLimitDescriptionFromRate Prevent creating misleading timestamps when rate.Reset.Time is zero, matching the defensive pattern used in other rate limit functions. Co-Authored-By: Claude Opus 4.5 --- pkg/connector/helpers.go | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/pkg/connector/helpers.go b/pkg/connector/helpers.go index c11fd51b..4d97bfb6 100644 --- a/pkg/connector/helpers.go +++ b/pkg/connector/helpers.go @@ -194,12 +194,15 @@ func extractRateLimitData(response *github.Response) (*v2.RateLimitDescription, // rateLimitDescriptionFromRate creates a RateLimitDescription from a github.Rate struct. // This is used when go-github returns a RateLimitError with rate info but a synthetic response. func rateLimitDescriptionFromRate(rate github.Rate) *v2.RateLimitDescription { - return &v2.RateLimitDescription{ + desc := &v2.RateLimitDescription{ Status: v2.RateLimitDescription_STATUS_OVERLIMIT, Limit: int64(rate.Limit), Remaining: int64(rate.Remaining), - ResetAt: timestamppb.New(rate.Reset.Time), } + if !rate.Reset.Time.IsZero() { + desc.ResetAt = timestamppb.New(rate.Reset.Time) + } + return desc } // rateLimitDescriptionFromRetryAfter creates a RateLimitDescription from a retry-after duration. From aa1a7d97da07f9d9ac4ed68991c4285f6946c921 Mon Sep 17 00:00:00 2001 From: John Allers Date: Thu, 5 Feb 2026 11:06:07 -0500 Subject: [PATCH 3/3] remove embedded field --- pkg/connector/helpers.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/connector/helpers.go b/pkg/connector/helpers.go index 4d97bfb6..76ce3ad0 100644 --- a/pkg/connector/helpers.go +++ b/pkg/connector/helpers.go @@ -199,7 +199,7 @@ func rateLimitDescriptionFromRate(rate github.Rate) *v2.RateLimitDescription { Limit: int64(rate.Limit), Remaining: int64(rate.Remaining), } - if !rate.Reset.Time.IsZero() { + if !rate.Reset.IsZero() { desc.ResetAt = timestamppb.New(rate.Reset.Time) } return desc