Skip to content

Conversation

@omgitsads
Copy link
Member

Add error wrappers to errors returned from the GitHub API and GitHub GraphQL API

@omgitsads omgitsads requested a review from a team as a code owner December 10, 2025 13:35
Copilot AI review requested due to automatic review settings December 10, 2025 13:35
@tanduc040302

This comment has been minimized.

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR adds error annotation wrappers to errors returned from the GitHub API and GraphQL API in the issues functionality. The changes introduce a new helper function NewGitHubGraphQLErrorToCtx and apply error wrapping patterns to several error handling locations in the issues code.

Key Changes:

  • Added NewGitHubGraphQLErrorToCtx function in pkg/errors/error.go to add GraphQL errors to context
  • Updated error handling in CreateIssue, ListIssues, and AssignCopilotToIssue to use new error wrapper functions

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
pkg/errors/error.go Added NewGitHubGraphQLErrorToCtx function to store GraphQL errors in context, following the pattern of the existing NewGitHubAPIErrorToCtx function
pkg/github/issues.go Updated error handling in multiple functions to use NewGitHubAPIErrorResponse and NewGitHubGraphQLErrorResponse for consistent error annotation

Comment on lines 1664 to 1665
_, _ = ghErrors.NewGitHubGraphQLErrorToCtx(ctx, "failed to get suggested actors", err)
return nil, nil, err
Copy link

Copilot AI Dec 10, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The error at line 1664 is inconsistently handled compared to other GraphQL errors in this file. This code uses NewGitHubGraphQLErrorToCtx which only adds the error to the context, but then returns the raw error without wrapping it in a tool result.

This is inconsistent with other error handling patterns in this function (e.g., line 1711) and throughout the file. The function should return a *mcp.CallToolResult as the first return value when an error occurs, not nil.

Consider changing this to:

return ghErrors.NewGitHubGraphQLErrorResponse(ctx, "failed to get suggested actors", err), nil, nil

This matches the pattern used at line 1711 and other locations in the file.

Suggested change
_, _ = ghErrors.NewGitHubGraphQLErrorToCtx(ctx, "failed to get suggested actors", err)
return nil, nil, err
return ghErrors.NewGitHubGraphQLErrorResponse(ctx, "failed to get suggested actors", err), nil, nil

Copilot uses AI. Check for mistakes.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

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.

4 participants