Skip to content

Commit 89fc626

Browse files
johnallersclaude
andauthored
[CXP-204] improve rate limit and temporary error handling (#121)
* 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 <noreply@anthropic.com> * 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 <noreply@anthropic.com> * remove embedded field --------- Co-authored-by: Claude Opus 4.5 <noreply@anthropic.com>
1 parent 54ab119 commit 89fc626

1 file changed

Lines changed: 69 additions & 0 deletions

File tree

pkg/connector/helpers.go

Lines changed: 69 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,10 +2,12 @@ package connector
22

33
import (
44
"context"
5+
"errors"
56
"fmt"
67
"net/http"
78
"strconv"
89
"strings"
10+
"time"
911

1012
v2 "github.com/conductorone/baton-sdk/pb/c1/connector/v2"
1113
"github.com/conductorone/baton-sdk/pkg/annotations"
@@ -18,6 +20,7 @@ import (
1820
"golang.org/x/text/cases"
1921
"golang.org/x/text/language"
2022
"google.golang.org/grpc/codes"
23+
"google.golang.org/grpc/status"
2124
"google.golang.org/protobuf/types/known/timestamppb"
2225
)
2326

@@ -188,6 +191,41 @@ func extractRateLimitData(response *github.Response) (*v2.RateLimitDescription,
188191
}, nil
189192
}
190193

194+
// rateLimitDescriptionFromRate creates a RateLimitDescription from a github.Rate struct.
195+
// This is used when go-github returns a RateLimitError with rate info but a synthetic response.
196+
func rateLimitDescriptionFromRate(rate github.Rate) *v2.RateLimitDescription {
197+
desc := &v2.RateLimitDescription{
198+
Status: v2.RateLimitDescription_STATUS_OVERLIMIT,
199+
Limit: int64(rate.Limit),
200+
Remaining: int64(rate.Remaining),
201+
}
202+
if !rate.Reset.IsZero() {
203+
desc.ResetAt = timestamppb.New(rate.Reset.Time)
204+
}
205+
return desc
206+
}
207+
208+
// rateLimitDescriptionFromRetryAfter creates a RateLimitDescription from a retry-after duration.
209+
// This is used for AbuseRateLimitError which provides a RetryAfter duration.
210+
func rateLimitDescriptionFromRetryAfter(retryAfter *time.Duration) *v2.RateLimitDescription {
211+
desc := &v2.RateLimitDescription{
212+
Status: v2.RateLimitDescription_STATUS_OVERLIMIT,
213+
}
214+
if retryAfter != nil {
215+
desc.ResetAt = timestamppb.New(time.Now().Add(*retryAfter))
216+
}
217+
return desc
218+
}
219+
220+
// wrapErrorWithRateLimitDetails creates a gRPC error with rate limit details attached.
221+
func wrapErrorWithRateLimitDetails(code codes.Code, msg string, rlDesc *v2.RateLimitDescription, err error) error {
222+
st := status.New(code, msg)
223+
if rlDesc != nil {
224+
st, _ = st.WithDetails(rlDesc)
225+
}
226+
return errors.Join(st.Err(), err)
227+
}
228+
191229
type listUsersQuery struct {
192230
Organization struct {
193231
SamlIdentityProvider struct {
@@ -256,6 +294,15 @@ func isPermissionError(resp *github.Response) bool {
256294
return resp.StatusCode == http.StatusForbidden
257295
}
258296

297+
func isTemporarilyUnavailable(resp *github.Response) bool {
298+
if resp == nil {
299+
return false
300+
}
301+
return resp.StatusCode == http.StatusServiceUnavailable ||
302+
resp.StatusCode == http.StatusBadGateway ||
303+
resp.StatusCode == http.StatusGatewayTimeout
304+
}
305+
259306
// wrapGitHubError wraps GitHub API errors with appropriate gRPC status codes based on the HTTP response.
260307
// It handles rate limiting, authentication errors, permission errors, and generic errors.
261308
// The contextMsg parameter should describe the operation that failed (e.g., "failed to list teams").
@@ -264,9 +311,31 @@ func wrapGitHubError(err error, resp *github.Response, contextMsg string) error
264311
return nil
265312
}
266313

314+
// Check for go-github rate limit error types FIRST.
315+
// These may have synthetic responses with empty headers when the client
316+
// blocks requests without making an actual HTTP call.
317+
var rateLimitErr *github.RateLimitError
318+
if errors.As(err, &rateLimitErr) {
319+
rlDesc := rateLimitDescriptionFromRate(rateLimitErr.Rate)
320+
return wrapErrorWithRateLimitDetails(codes.Unavailable, "rate limit exceeded", rlDesc, err)
321+
}
322+
323+
var abuseRateLimitErr *github.AbuseRateLimitError
324+
if errors.As(err, &abuseRateLimitErr) {
325+
rlDesc := rateLimitDescriptionFromRetryAfter(abuseRateLimitErr.RetryAfter)
326+
return wrapErrorWithRateLimitDetails(codes.Unavailable, "secondary rate limit exceeded", rlDesc, err)
327+
}
328+
329+
// Check response-based rate limiting (real 429 or 403 with header)
267330
if isRatelimited(resp) {
268331
return uhttp.WrapErrors(codes.Unavailable, "too many requests", err)
269332
}
333+
334+
// Check for temporary server errors (503, 502, 504)
335+
if isTemporarilyUnavailable(resp) {
336+
return uhttp.WrapErrors(codes.Unavailable, "service temporarily unavailable", err)
337+
}
338+
270339
if isAuthError(resp) {
271340
return uhttp.WrapErrors(codes.Unauthenticated, contextMsg, err)
272341
}

0 commit comments

Comments
 (0)