Skip to content

Commit e46222c

Browse files
la3renceclaude
andcommitted
Improve error handling and code robustness
- Enhance error handling in all API functions - Add proper error returns and logging - Improve validation in utility functions - Add panic recovery in HTTP handler - Revert package name to handler to comply with Vercel Serverless requirements Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
1 parent c774695 commit e46222c

2 files changed

Lines changed: 182 additions & 70 deletions

File tree

api/index.go

Lines changed: 142 additions & 50 deletions
Original file line numberDiff line numberDiff line change
@@ -79,13 +79,16 @@ var labelColorMapping = map[string]string{
7979
var ctx = context.Background()
8080
var secret = os.Getenv("WEBHOOK_SECRET")
8181

82-
func getGitHubClient() *github.Client {
82+
func getGitHubClient() (*github.Client, error) {
8383
token := os.Getenv("BOT_TOKEN")
84+
if token == "" {
85+
return nil, fmt.Errorf("BOT_TOKEN environment variable is not set")
86+
}
8487
ts := oauth2.StaticTokenSource(
8588
&oauth2.Token{AccessToken: token},
8689
)
8790
tc := oauth2.NewClient(ctx, ts)
88-
return github.NewClient(tc)
91+
return github.NewClient(tc), nil
8992
}
9093

9194
// this Handler used to be the serverless function
@@ -102,19 +105,41 @@ func Handler(w http.ResponseWriter, r *http.Request) {
102105
log.Printf("could not parse webhook: %s\n", parseErr)
103106
return
104107
}
105-
githubClient := getGitHubClient()
108+
githubClient, err := getGitHubClient()
109+
if err != nil {
110+
http.Error(w, "Internal server error", http.StatusInternalServerError)
111+
log.Printf("error getting GitHub client: %s\n", err)
112+
return
113+
}
114+
115+
// Add proper error handling for all operations
116+
defer func() {
117+
if r := recover(); r != nil {
118+
log.Printf("Recovered from panic: %v", r)
119+
http.Error(w, "Internal server error", http.StatusInternalServerError)
120+
}
121+
}()
122+
106123
switch e := event.(type) {
107124
case *github.PushEvent:
108125
// this is a commit push, do something with it
109126
case *github.PullRequestEvent:
110127
pullRequestEvent := *e
111-
addLabelIfPROpen(githubClient, pullRequestEvent)
112-
requestReviewIfPROpen(githubClient, pullRequestEvent)
128+
if err := addLabelIfPROpen(githubClient, pullRequestEvent); err != nil {
129+
log.Printf("Error adding label to PR: %v", err)
130+
}
131+
if err := requestReviewIfPROpen(githubClient, pullRequestEvent); err != nil {
132+
log.Printf("Error requesting review for PR: %v", err)
133+
}
113134
case *github.PullRequestReviewEvent:
114-
addApproveLabelIfApproved(githubClient, *e)
135+
if err := addApproveLabelIfApproved(githubClient, *e); err != nil {
136+
log.Printf("Error adding approve label: %v", err)
137+
}
115138
case *github.IssuesEvent:
116139
issueEvent := *e
117-
addLabelIfIssueOpen(githubClient, issueEvent)
140+
if err := addLabelIfIssueOpen(githubClient, issueEvent); err != nil {
141+
log.Printf("Error adding label to issue: %v", err)
142+
}
118143
case *github.WatchEvent:
119144
if e.Action != nil && *e.Action == "starred" {
120145
fmt.Printf("%s starred repository %s\n",
@@ -132,28 +157,44 @@ func Handler(w http.ResponseWriter, r *http.Request) {
132157
return
133158
}
134159
if strings.Contains(commentBody, Label) {
135-
addLabelsByComment(commentBody, githubClient, issueCommentEvent)
160+
if err := addLabelsByComment(commentBody, githubClient, issueCommentEvent); err != nil {
161+
log.Printf("Error adding labels: %v", err)
162+
}
136163
}
137164
if strings.Contains(commentBody, UnLabel) {
138-
removeLabelFromIssue(commentBody, githubClient, issueCommentEvent)
165+
if err := removeLabelFromIssue(commentBody, githubClient, issueCommentEvent); err != nil {
166+
log.Printf("Error removing labels: %v", err)
167+
}
139168
}
140169
if strings.Contains(commentBody, Approve) {
141-
approvePullRequest(githubClient, issueCommentEvent)
170+
if err := approvePullRequest(githubClient, issueCommentEvent); err != nil {
171+
log.Printf("Error approving PR: %v", err)
172+
}
142173
}
143174
if strings.Contains(commentBody, LGTM) {
144-
mergePullRequest(githubClient, issueCommentEvent, "rebase")
175+
if err := mergePullRequest(githubClient, issueCommentEvent, "rebase"); err != nil {
176+
log.Printf("Error merging PR with rebase: %v", err)
177+
}
145178
}
146179
if strings.Contains(commentBody, Merge) {
147-
mergePullRequest(githubClient, issueCommentEvent, "merge")
180+
if err := mergePullRequest(githubClient, issueCommentEvent, "merge"); err != nil {
181+
log.Printf("Error merging PR: %v", err)
182+
}
148183
}
149184
if strings.Contains(commentBody, Close) {
150-
closeOrOpenIssue(githubClient, issueCommentEvent, false)
185+
if err := closeOrOpenIssue(githubClient, issueCommentEvent, false); err != nil {
186+
log.Printf("Error closing issue: %v", err)
187+
}
151188
}
152189
if strings.Contains(commentBody, Reopen) || strings.Contains(commentBody, ReOpen) {
153-
closeOrOpenIssue(githubClient, issueCommentEvent, true)
190+
if err := closeOrOpenIssue(githubClient, issueCommentEvent, true); err != nil {
191+
log.Printf("Error reopening issue: %v", err)
192+
}
154193
}
155194
if strings.Contains(commentBody, Update) {
156-
updatePullRequest(githubClient, issueCommentEvent)
195+
if err := updatePullRequest(githubClient, issueCommentEvent); err != nil {
196+
log.Printf("Error updating PR: %v", err)
197+
}
157198
}
158199
}
159200
default:
@@ -172,12 +213,15 @@ func ackByReaction(client *github.Client, issueCommentEvent github.IssueCommentE
172213
_, _, _ = client.Reactions.CreateIssueCommentReaction(ctx, owner, repo, commentId, "+1")
173214
}
174215

175-
func updatePullRequest(client *github.Client, issueCommentEvent github.IssueCommentEvent) {
216+
func updatePullRequest(client *github.Client, issueCommentEvent github.IssueCommentEvent) error {
176217
ackByReaction(client, issueCommentEvent)
177218
repo := *issueCommentEvent.GetRepo().Name
178219
owner := *issueCommentEvent.GetRepo().Owner.Login
179220
number := *issueCommentEvent.GetIssue().Number
180-
pullRequest, _, _ := client.PullRequests.Get(ctx, owner, repo, number)
221+
pullRequest, _, err := client.PullRequests.Get(ctx, owner, repo, number)
222+
if err != nil {
223+
return fmt.Errorf("failed to get pull request: %w", err)
224+
}
181225
sourceBranchSha := pullRequest.GetHead().GetSHA()
182226
// https://docs.github.com/cn/rest/reference/pulls#update-a-pull-request-branch
183227
_, res, err := client.PullRequests.UpdateBranch(ctx, owner, repo, number,
@@ -191,10 +235,12 @@ func updatePullRequest(client *github.Client, issueCommentEvent github.IssueComm
191235
} else {
192236
sendCommentWithDetailsDom(client, owner, repo, number, "Error", err.Error())
193237
}
238+
return fmt.Errorf("failed to update PR branch: %w", err)
194239
}
240+
return nil
195241
}
196242

197-
func approvePullRequest(client *github.Client, event github.IssueCommentEvent) {
243+
func approvePullRequest(client *github.Client, event github.IssueCommentEvent) error {
198244
approveEventName := "APPROVE"
199245
owner := event.GetRepo().GetOwner().GetLogin()
200246
repo := event.GetRepo().GetName()
@@ -203,33 +249,47 @@ func approvePullRequest(client *github.Client, event github.IssueCommentEvent) {
203249
&github.PullRequestReviewRequest{
204250
Event: &approveEventName,
205251
})
206-
if err == nil {
207-
submitReview, _, _ := client.PullRequests.SubmitReview(ctx, owner, repo, issueNumber,
208-
review.GetID(),
209-
&github.PullRequestReviewRequest{
210-
Event: &approveEventName,
211-
},
212-
)
213-
log.Println(submitReview)
214-
// labels := []string{"approved"}
215-
// addLabelsToIssue(labels, client, owner, repo, issueNumber)
216-
} else {
252+
if err != nil {
217253
log.Println("CreateReview Error" + err.Error())
254+
return fmt.Errorf("failed to create review: %w", err)
255+
}
256+
257+
submitReview, _, err := client.PullRequests.SubmitReview(ctx, owner, repo, issueNumber,
258+
review.GetID(),
259+
&github.PullRequestReviewRequest{
260+
Event: &approveEventName,
261+
},
262+
)
263+
if err != nil {
264+
log.Println("SubmitReview Error" + err.Error())
265+
return fmt.Errorf("failed to submit review: %w", err)
218266
}
267+
268+
log.Println(submitReview)
269+
// labels := []string{"approved"}
270+
// addLabelsToIssue(labels, client, owner, repo, issueNumber)
271+
return nil
219272
}
220273

221-
func addLabelsByComment(commentBody string, githubClient *github.Client, issueCommentEvent github.IssueCommentEvent) {
274+
func addLabelsByComment(commentBody string, githubClient *github.Client, issueCommentEvent github.IssueCommentEvent) error {
222275
ackByReaction(githubClient, issueCommentEvent)
223276
labelsToAdd := utils.GetTagNextAllParams(commentBody, Label)
224-
addLabelsToIssue(labelsToAdd, githubClient,
277+
err := addLabelsToIssue(labelsToAdd, githubClient,
225278
issueCommentEvent.GetRepo().GetOwner().GetLogin(),
226279
issueCommentEvent.GetRepo().GetName(),
227280
issueCommentEvent.GetIssue().GetNumber())
281+
if err != nil {
282+
return fmt.Errorf("failed to add labels by comment: %w", err)
283+
}
284+
return nil
228285
}
229286

230-
func addLabelsToIssue(labelsToAdd []string, githubClient *github.Client, owner string, repo string, issueNumber int) {
287+
func addLabelsToIssue(labelsToAdd []string, githubClient *github.Client, owner string, repo string, issueNumber int) error {
231288
// check if label exists, if yes, add it
232-
labels, _, _ := githubClient.Issues.ListLabelsByIssue(ctx, owner, repo, issueNumber, nil)
289+
labels, _, err := githubClient.Issues.ListLabelsByIssue(ctx, owner, repo, issueNumber, nil)
290+
if err != nil {
291+
return fmt.Errorf("failed to list labels for issue: %w", err)
292+
}
233293
for _, param := range labelsToAdd {
234294
labelExists := false
235295
for _, label := range labels {
@@ -244,17 +304,24 @@ func addLabelsToIssue(labelsToAdd []string, githubClient *github.Client, owner s
244304
if color == "" {
245305
color = labelColorMapping["default"]
246306
}
247-
_, _, _ = githubClient.Issues.CreateLabel(ctx, owner, repo, &github.Label{
307+
_, _, err := githubClient.Issues.CreateLabel(ctx, owner, repo, &github.Label{
248308
Name: &param,
249309
Color: &color,
250310
})
311+
if err != nil {
312+
log.Printf("Failed to create label %s: %v", param, err)
313+
}
251314
}
252315
}
253316
issue, response, githubErr := githubClient.Issues.AddLabelsToIssue(ctx, owner, repo, issueNumber, labelsToAdd)
317+
if githubErr != nil {
318+
return fmt.Errorf("failed to add labels to issue: %w", githubErr)
319+
}
254320
log.Println(response, issue, githubErr)
321+
return nil
255322
}
256323

257-
func removeLabelFromIssue(commentBody string, githubClient *github.Client, issueCommentEvent github.IssueCommentEvent) {
324+
func removeLabelFromIssue(commentBody string, githubClient *github.Client, issueCommentEvent github.IssueCommentEvent) error {
258325
ackByReaction(githubClient, issueCommentEvent)
259326
params := utils.GetTagNextAllParams(commentBody, UnLabel)
260327
for _, param := range params {
@@ -264,13 +331,15 @@ func removeLabelFromIssue(commentBody string, githubClient *github.Client, issue
264331
*issueCommentEvent.GetIssue().Number,
265332
param)
266333
if githubErr != nil {
267-
log.Print(githubErr)
334+
log.Printf("Error removing label %s: %v", param, githubErr)
335+
return fmt.Errorf("failed to remove label %s: %w", param, githubErr)
268336
}
269337
log.Println(response)
270338
}
339+
return nil
271340
}
272341

273-
func requestReviewIfPROpen(githubClient *github.Client, pullRequestEvent github.PullRequestEvent) {
342+
func requestReviewIfPROpen(githubClient *github.Client, pullRequestEvent github.PullRequestEvent) error {
274343
action := *pullRequestEvent.Action
275344
if action == "opened" || action == "reopened" {
276345
reviewers, response, err := githubClient.PullRequests.RequestReviewers(ctx,
@@ -283,27 +352,33 @@ func requestReviewIfPROpen(githubClient *github.Client, pullRequestEvent github.
283352
)
284353
if err != nil {
285354
log.Print(err)
355+
return fmt.Errorf("failed to request reviewers: %w", err)
286356
}
287357
log.Println(response, reviewers)
288358
}
359+
return nil
289360
}
290361

291-
func addLabelIfPROpen(githubClient *github.Client, pullRequestEvent github.PullRequestEvent) {
362+
func addLabelIfPROpen(githubClient *github.Client, pullRequestEvent github.PullRequestEvent) error {
292363
action := *pullRequestEvent.Action
293364
title := pullRequestEvent.GetPullRequest().GetTitle()
294365
if action == "edited" || action == "opened" {
295366
for titleKey, labelValue := range titleLabelMapping {
296367
if strings.Contains(strings.ToLower(title), strings.ToLower(titleKey)) {
297-
addLabelsToIssue([]string{labelValue}, githubClient,
368+
err := addLabelsToIssue([]string{labelValue}, githubClient,
298369
*pullRequestEvent.GetRepo().Owner.Login,
299370
*pullRequestEvent.GetRepo().Name,
300371
*pullRequestEvent.GetPullRequest().Number)
372+
if err != nil {
373+
return fmt.Errorf("failed to add label to PR: %w", err)
374+
}
301375
}
302376
}
303377
}
378+
return nil
304379
}
305380

306-
func addApproveLabelIfApproved(githubClient *github.Client, reviewEvent github.PullRequestReviewEvent) {
381+
func addApproveLabelIfApproved(githubClient *github.Client, reviewEvent github.PullRequestReviewEvent) error {
307382
review := *reviewEvent.Review
308383
state := review.GetState()
309384
approvedString := "approved"
@@ -312,26 +387,34 @@ func addApproveLabelIfApproved(githubClient *github.Client, reviewEvent github.P
312387
owner := reviewEvent.GetRepo().GetOwner().GetLogin()
313388
repo := reviewEvent.GetRepo().GetName()
314389
issueNumber := reviewEvent.GetPullRequest().GetNumber()
315-
addLabelsToIssue(labels, githubClient, owner, repo, issueNumber)
390+
err := addLabelsToIssue(labels, githubClient, owner, repo, issueNumber)
391+
if err != nil {
392+
return fmt.Errorf("failed to add approve label: %w", err)
393+
}
316394
}
395+
return nil
317396
}
318397

319-
func addLabelIfIssueOpen(githubClient *github.Client, issuesEvent github.IssuesEvent) {
398+
func addLabelIfIssueOpen(githubClient *github.Client, issuesEvent github.IssuesEvent) error {
320399
action := *issuesEvent.Action
321400
title := issuesEvent.GetIssue().GetTitle()
322401
if action == "edited" || action == "opened" {
323402
for titleKey, labelValue := range titleLabelMapping {
324403
if strings.Contains(strings.ToLower(title), strings.ToLower(titleKey)) {
325-
addLabelsToIssue([]string{labelValue}, githubClient,
404+
err := addLabelsToIssue([]string{labelValue}, githubClient,
326405
*issuesEvent.GetRepo().Owner.Login,
327406
*issuesEvent.GetRepo().Name,
328407
*issuesEvent.GetIssue().Number)
408+
if err != nil {
409+
return fmt.Errorf("failed to add label to issue: %w", err)
410+
}
329411
}
330412
}
331413
}
414+
return nil
332415
}
333416

334-
func mergePullRequest(githubClient *github.Client, issueCommentEvent github.IssueCommentEvent, mergeMethod string) {
417+
func mergePullRequest(githubClient *github.Client, issueCommentEvent github.IssueCommentEvent, mergeMethod string) error {
335418
ackByReaction(githubClient, issueCommentEvent)
336419
owner := issueCommentEvent.GetRepo().GetOwner().GetLogin()
337420
senderName := issueCommentEvent.GetSender().GetLogin()
@@ -340,9 +423,12 @@ func mergePullRequest(githubClient *github.Client, issueCommentEvent github.Issu
340423
if owner != senderName {
341424
sendComment(githubClient, owner, repo, number,
342425
fmt.Sprintf("Sorry, @%s - This pull request can only be merged by the owner (@%s).", senderName, owner))
343-
return
426+
return nil
427+
}
428+
mergedBefore, _, err := githubClient.PullRequests.IsMerged(ctx, owner, repo, number)
429+
if err != nil {
430+
return fmt.Errorf("failed to check if PR is merged: %w", err)
344431
}
345-
mergedBefore, _, _ := githubClient.PullRequests.IsMerged(ctx, owner, repo, number)
346432
mergeComment := fmt.Sprintf("PR #%d was merged. Thanks for your contribution.", number)
347433
commitMsg := fmt.Sprintf("merge: PR(#%d)", number)
348434
if mergedBefore {
@@ -356,6 +442,7 @@ func mergePullRequest(githubClient *github.Client, issueCommentEvent github.Issu
356442
if err != nil {
357443
log.Println(err)
358444
sendCommentWithDetailsDom(githubClient, owner, repo, number, "Error", err.Error())
445+
return fmt.Errorf("failed to merge PR: %w", err)
359446
} else {
360447
log.Println(mergeResult)
361448
merged := mergeResult.GetMerged()
@@ -366,9 +453,11 @@ func mergePullRequest(githubClient *github.Client, issueCommentEvent github.Issu
366453
failMsg := fmt.Sprintf("Fail to merge this PR #%d", number)
367454
sendCommentWithDetailsDom(githubClient, owner, repo, number, "Debug", failMsg)
368455
log.Println(failMsg)
456+
return fmt.Errorf("PR merge result was not successful: %s", failMsg)
369457
}
370458
}
371459
}
460+
return nil
372461
}
373462

374463
func sendComment(githubClient *github.Client, owner string, repo string, number int, comment string) *github.IssueComment {
@@ -389,7 +478,7 @@ func sendCommentWithDetailsDom(githubClient *github.Client, owner string, repo s
389478
`<details><summary>`+detailSummary+`</summary><p>`+detailBody+`</p></details>`)
390479
}
391480

392-
func closeOrOpenIssue(githubClient *github.Client, issueCommentEvent github.IssueCommentEvent, open bool) {
481+
func closeOrOpenIssue(githubClient *github.Client, issueCommentEvent github.IssueCommentEvent, open bool) error {
393482
ackByReaction(githubClient, issueCommentEvent)
394483
owner := issueCommentEvent.GetRepo().GetOwner().GetLogin()
395484
repo := issueCommentEvent.GetRepo().GetName()
@@ -403,8 +492,11 @@ func closeOrOpenIssue(githubClient *github.Client, issueCommentEvent github.Issu
403492
edit, response, err := githubClient.Issues.Edit(ctx, owner, repo, number, &github.IssueRequest{
404493
State: &state,
405494
})
406-
if err == nil {
407-
log.Println(response)
408-
log.Println(edit)
495+
if err != nil {
496+
log.Println(err)
497+
return fmt.Errorf("failed to close or open issue: %w", err)
409498
}
499+
log.Println(response)
500+
log.Println(edit)
501+
return nil
410502
}

0 commit comments

Comments
 (0)