automerge failure patch #1917
automerge failure patch #1917
Conversation
WalkthroughThis pull request revises the error handling approach in the project controller. When automerging a pull request fails, the system now posts a comment on the associated batch with error details instead of directly returning an internal server error to the client. A new utility function, Changes
Sequence Diagram(s)sequenceDiagram
participant Controller as SetJobStatusForProject
participant AutoMerge as AutomergePRforBatchIfEnabled
participant CommentUtil as PostCommentForBatch
participant GitHubCP as GithubClientProvider
participant GitHubSvc as GitHub Service
participant Logger as Logger
Controller ->> AutoMerge: Attempt auto merge PR
AutoMerge -->> Controller: Return error if merge fails
Controller ->> CommentUtil: Call PostCommentForBatch(batch, comment, GitHubCP)
CommentUtil ->> CommentUtil: Check if VCS is GitHub
CommentUtil ->> GitHubCP: Retrieve GitHub service using batch details
GitHubCP -->> CommentUtil: Return service or error
alt Service retrieved
CommentUtil ->> GitHubSvc: Post comment on PR
GitHubSvc -->> CommentUtil: Return success/error
else Service retrieval error
CommentUtil ->> Logger: Log retrieval error
end
alt Comment post error
CommentUtil ->> Logger: Log posting error
end
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms (3)
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
backend/utils/batch_utils.go (2)
9-25: Function implementation looks good, with minor improvements neededThe function correctly posts comments on GitHub PRs with proper error handling. However, there are a few issues to address:
- There's a grammatical error in the error message on line 20.
- The function lacks documentation.
- The function has redundant return statements.
- It silently returns nil for non-GitHub VCS without any warning.
+// PostCommentForBatch posts a comment on the pull request associated with the given batch. +// It currently only supports GitHub repositories, with other VCS support planned. +// Returns an error if the GitHub service cannot be retrieved or if posting the comment fails. func PostCommentForBatch(batch *models.DiggerBatch, comment string, githubClientProvider GithubClientProvider) error { // todo: perform for rest of vcs as well if batch.VCS == models.DiggerVCSGithub { ghService, _, err := GetGithubService(githubClientProvider, batch.GithubInstallationId, batch.RepoFullName, batch.RepoOwner, batch.RepoName) if err != nil { log.Printf("error getting ghService: %v", err) return fmt.Errorf("error getting ghService: %v", err) } _, err = ghService.PublishComment(batch.PrNumber, comment) if err != nil { log.Printf("error publishing comment (%v): %v", comment, err) - return fmt.Errorf("error getting publishing comment (%v): %v", comment, err) + return fmt.Errorf("error publishing comment (%v): %v", comment, err) } return nil } + log.Printf("skipping comment posting for non-GitHub VCS: %s", batch.VCS) return nil }
19-20: Fix error message grammatical errorThe error message "error getting publishing comment" has a grammatical issue.
- return fmt.Errorf("error getting publishing comment (%v): %v", comment, err) + return fmt.Errorf("error publishing comment (%v): %v", comment, err)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
go.work.sumis excluded by!**/*.sum
📒 Files selected for processing (2)
backend/controllers/projects.go(1 hunks)backend/utils/batch_utils.go(1 hunks)
🧰 Additional context used
🧬 Code Definitions (1)
backend/controllers/projects.go (1)
backend/utils/github.go (3)
err(174-174)err(189-189)GithubClientProvider(69-73)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: Build
- GitHub Check: Build
🔇 Additional comments (2)
backend/controllers/projects.go (1)
493-500: Improved error handling with user notificationThe code now posts a GitHub comment when automerge fails instead of silently logging the error. This is a great improvement as it actively notifies users about the failure and reminds them to merge manually.
backend/utils/batch_utils.go (1)
10-10: Consider implementing support for other VCS systemsThe TODO comment correctly flags the need to support other version control systems, but there's no defined timeline or issue tracking this work.
Could you create an issue to track this enhancement for other VCS platforms like GitLab which is already supported elsewhere in the codebase?
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
PR Summary
Based on the provided information, I'll summarize the key changes in this pull request focusing only on the most recent updates:
Enhanced error handling for automerge failures in Digger with improved user feedback:
- Added new
PostCommentForBatchfunction in/backend/utils/batch_utils.goto handle GitHub PR comment posting with proper error handling - Modified
SetJobStatusForProjectin/backend/controllers/projects.goto post warning comments instead of failing jobs when automerge fails - Improved error message formatting and logging for better clarity when automerge operations encounter issues
The changes effectively address issue #1906 by ensuring jobs complete successfully while providing clear feedback about merge failures.
3 file(s) reviewed, 2 comment(s)
Edit PR Review Bot Settings | Greptile
backend/utils/batch_utils.go
Outdated
| _, err = ghService.PublishComment(batch.PrNumber, comment) | ||
| if err != nil { | ||
| log.Printf("error publishing comment (%v): %v", comment, err) | ||
| return fmt.Errorf("error getting publishing comment (%v): %v", comment, err) |
There was a problem hiding this comment.
syntax: Error message has incorrect wording 'error getting publishing comment' - should be 'error publishing comment'
| return fmt.Errorf("error getting publishing comment (%v): %v", comment, err) | |
| return fmt.Errorf("error publishing comment (%v): %v", comment, err) |
| } | ||
| return nil | ||
| } | ||
| return nil |
There was a problem hiding this comment.
style: Silent failure for non-GitHub VCS types. Consider logging a warning or returning an error to indicate unsupported VCS type
temporary patch for failure to merge in #1906
Summary by CodeRabbit