Skip to content

[CXP-204] improve rate limit and temporary error handling#121

Open
johnallers wants to merge 1 commit intomainfrom
jallers/cxp-204
Open

[CXP-204] improve rate limit and temporary error handling#121
johnallers wants to merge 1 commit intomainfrom
jallers/cxp-204

Conversation

@johnallers
Copy link
Contributor

@johnallers johnallers commented Feb 4, 2026

Summary

  • Fix rate limit errors being misclassified as PermissionDenied instead of Unavailable, causing syncs to fail instead of retry
  • Add proper handling for *github.RateLimitError and *github.AbuseRateLimitError error types using errors.As()
  • Extract rate limit data (reset time, limit, remaining) and attach to gRPC status details for proper backoff
  • Add handling for 503/502/504 temporary server errors as Unavailable (retryable)

Problem

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 X-Ratelimit-Remaining header to equal "0", but the synthetic response has no headers. This caused rate limit errors to be misclassified as PermissionDenied, which is not retried by the SDK.

Solution

Check for go-github error types before checking HTTP status codes:

  1. *github.RateLimitError → extract Rate.Reset, Rate.Limit, Rate.Remainingcodes.Unavailable with rate limit details
  2. *github.AbuseRateLimitError → use RetryAfter duration → codes.Unavailable with rate limit details
  3. 503/502/504 responses → codes.Unavailable (retryable)

The SDK's retry logic now receives proper rate limit data to calculate appropriate backoff times.

Test plan

  • Build passes
  • Tests pass
  • Lint passes
  • Test with a GitHub org that hits rate limits in service mode

🤖 Generated with Claude Code

Summary by CodeRabbit

  • Bug Fixes
    • Improved detection and messaging for rate limit errors, including clearer retry timing and guidance.
    • Better handling of temporary service outages (502/503/504), with clearer "service unavailable" behavior for affected requests.
    • Wrapped rate-limit details into error responses so users receive more actionable retry information.

@johnallers johnallers requested a review from a team February 4, 2026 12:34
@linear
Copy link

linear bot commented Feb 4, 2026

@coderabbitai
Copy link

coderabbitai bot commented Feb 4, 2026

Walkthrough

Adds rate-limit detection and wrapping utilities to connector helpers: functions to derive rate-limit details from GitHub errors and Retry-After, expanded wrapGitHubError to attach RateLimitDescription to gRPC errors, and a helper to classify temporary service-unavailable responses (502/503/504).

Changes

Cohort / File(s) Summary
Rate-limit & error-wrapping helpers
pkg/connector/helpers.go
Added rateLimitDescriptionFromRate and rateLimitDescriptionFromRetryAfter, wrapErrorWithRateLimitDetails, isTemporarilyUnavailable; enhanced wrapGitHubError to detect RateLimitError and AbuseRateLimitError, attach rate-limit details to gRPC status, preserve existing rate-limit checks and temporary-server handling.

Sequence Diagram(s)

sequenceDiagram
  participant Client
  participant Connector
  participant GitHubAPI
  participant gRPCStatus

  Client->>Connector: API request
  Connector->>GitHubAPI: Call GitHub
  alt RateLimitError
    GitHubAPI-->>Connector: RateLimitError (with Rate)
    Connector->>Connector: rateLimitDescriptionFromRate()
    Connector->>gRPCStatus: attach RateLimitDescription
    gRPCStatus-->>Connector: gRPC error with details
    Connector-->>Client: gRPC rate-limited error
  else AbuseRateLimitError
    GitHubAPI-->>Connector: AbuseRateLimitError (with Retry-After)
    Connector->>Connector: rateLimitDescriptionFromRetryAfter()
    Connector->>gRPCStatus: attach RateLimitDescription
    gRPCStatus-->>Connector: gRPC error with details
    Connector-->>Client: gRPC rate-limited error
  else TemporaryServerError (502/503/504)
    GitHubAPI-->>Connector: 502/503/504
    Connector->>Connector: isTemporarilyUnavailable()
    Connector-->>Client: treated as unavailable error
  else OtherError
    GitHubAPI-->>Connector: Other error
    Connector->>Connector: existing auth/permission checks
    Connector-->>Client: wrapped error
  end
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Poem

🐰 I nibble logs in moonlit code,
I stitch rate-limits on the road,
Retry-after crumbs I weave,
So gRPC errors gently leave —
Hoppity, wrapped and neatly showed. 🥕

🚥 Pre-merge checks | ✅ 2 | ❌ 1
❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 71.43% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The pull request title accurately describes the main change: improving rate limit and temporary error handling as detailed in the objectives.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch jallers/cxp-204

Comment @coderabbitai help to get the list of available commands and usage tips.

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>
@johnallers johnallers requested a review from a team February 4, 2026 19:55
@johnallers johnallers changed the title fix: improve rate limit and temporary error handling [CXP-204] improve rate limit and temporary error handling Feb 4, 2026
@johnallers johnallers self-assigned this Feb 4, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants