Skip to content

Conversation

Copy link
Contributor

Copilot AI commented Nov 20, 2025

GitHub's GraphQL API returns rate limit errors in the response body (errors[].type === 'RATE_LIMIT'), but we only checked HTTP headers. This caused silent failures when rate limits were hit.

Changes

Type system

  • Added GraphQLError and GraphQLResponse<T> interfaces for proper response typing
  • Added isRateLimitError() helper to detect rate limit errors in response body
  • Added RATE_LIMIT constant for distinct rate limit error handling

API functions

  • makeGitHubGraphQLRequest<T>: Now returns typed GraphQLResponse<T>, detects and logs rate limit errors from response body, sends telemetry
  • makeSearchGraphQLRequest: Returns RATE_LIMIT constant on rate limit (instead of empty array), allowing callers to distinguish rate limits from empty results
  • getPullRequestFromGlobalId: Returns RATE_LIMIT constant on rate limit, allowing callers to distinguish from null (not found)
  • addPullRequestCommentGraphQLRequest: Returns RATE_LIMIT constant on rate limit, allowing callers to distinguish from null (failure)

Return types

  • makeSearchGraphQLRequest: Promise<PullRequestSearchItem[] | typeof RATE_LIMIT>
  • getPullRequestFromGlobalId: Promise<PullRequestSearchItem | null | typeof RATE_LIMIT>
  • addPullRequestCommentGraphQLRequest: Promise<PullRequestComment | null | typeof RATE_LIMIT>

Example usage

import { RATE_LIMIT } from './githubAPI';

// Distinct rate limit handling for search
const result = await makeSearchGraphQLRequest(...);
if (result === RATE_LIMIT) {
  // Handle rate limit specifically
  showRateLimitWarning();
  scheduleRetry();
  return;
}
// Process normal results
processPullRequests(result);

// Distinguish rate limit from not found
const pr = await getPullRequestFromGlobalId(...);
if (pr === RATE_LIMIT) {
  showRateLimitWarning();
} else if (pr === null) {
  showNotFoundError();
} else {
  displayPullRequest(pr);
}

Callers can now explicitly check for rate limits using result === RATE_LIMIT, enabling distinct handling separate from null/empty responses.

Created from VS Code.

Original prompt

Check for the ratelimit in the graphql and surface that for all callers to be handled appropriately

The user has attached the following file paths as relevant context:

  • src/platform/github/common/githubAPI.ts
  • .github/copilot-instructions.md

Created from VS Code.


💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.

Copilot AI changed the title [WIP] Check for ratelimit in GraphQL and surface to callers Add GraphQL rate limit error detection and propagation to API callers Nov 20, 2025
Copilot AI requested a review from joshspicer November 20, 2025 16:46
Copilot finished work on behalf of joshspicer November 20, 2025 16:46
@joshspicer
Copy link
Member

@copilot add 'RATE_LIMIT' as a part of the return types so that the callers can handle this distinctly from just null. Only add minimal tests necesary

Copy link
Contributor Author

Copilot AI commented Nov 20, 2025

@copilot add 'RATE_LIMIT' as a part of the return types so that the callers can handle this distinctly from just null. Only add minimal tests necesary

Added RATE_LIMIT constant to return types. Callers can now distinguish rate limit errors from null/empty results:

  • makeSearchGraphQLRequest: Returns PullRequestSearchItem[] | typeof RATE_LIMIT
  • getPullRequestFromGlobalId: Returns PullRequestSearchItem | null | typeof RATE_LIMIT
  • addPullRequestCommentGraphQLRequest: Returns PullRequestComment | null | typeof RATE_LIMIT

Example usage:

const result = await makeSearchGraphQLRequest(...);
if (result === RATE_LIMIT) {
  // Handle rate limit specifically
}

All minimal tests updated and passing. Commit: 7634613

Copilot AI changed the title Add GraphQL rate limit error detection and propagation to API callers Add GraphQL rate limit error detection with distinct return type for callers Nov 20, 2025
Copilot finished work on behalf of joshspicer November 20, 2025 17:33
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