- Notifications
You must be signed in to change notification settings - Fork3.1k
Report error when API silently fails to add review comment#1441
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
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 detection and reporting when the GitHub GraphQL API silently fails to add a pull request review comment (indicated by an empty thread ID in the response). This commonly occurs when parameters like line numbers are out of range.
- Added validation to check if the thread ID is nil after mutation
- Returns a descriptive error message with hints for fixing common issues
- Updated existing test mock to return a valid thread ID
- Added new test case to verify the error handling for nil thread ID
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| pkg/github/pullrequests.go | Added nil check for thread ID afteraddPullRequestReviewThread mutation with helpful error message explaining possible causes (line number out of range, incorrect file path, invalid side) |
| pkg/github/pullrequests_test.go | Updated existing test mock to return a valid thread ID and added new test case validating error handling when thread ID is nil |
tommaso-moro left a comment
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.
🚀
Love the hint for the LLM! great touch 💅
f3b9a63 intomainUh oh!
There was an error while loading.Please reload this page.
When line is out of range GraphQL API doesn't return error and we consider adding PR comment successful though it wasn't posted. The indication that something like this happened is empty thread id. This change adds thread id check and returns error with a hint to LLM how to fix the parameters in the next iteration.
Closes:#1214