-
Notifications
You must be signed in to change notification settings - Fork 3.2k
Add API Error annotations to GitHub issue errors #1566
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this 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
NewGitHubGraphQLErrorToCtxfunction inpkg/errors/error.goto add GraphQL errors to context - Updated error handling in
CreateIssue,ListIssues, andAssignCopilotToIssueto 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 |
pkg/github/issues.go
Outdated
| _, _ = ghErrors.NewGitHubGraphQLErrorToCtx(ctx, "failed to get suggested actors", err) | ||
| return nil, nil, err |
Copilot
AI
Dec 10, 2025
There was a problem hiding this comment.
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, nilThis matches the pattern used at line 1711 and other locations in the file.
| _, _ = 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 |
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
Add error wrappers to errors returned from the GitHub API and GitHub GraphQL API