Skip to content

Commit 8d6fbf3

Browse files
davidwin93davidwin93
authored
Merge pull request #17 from zapier/error-handling-nats
Improve error handling and prevent unrecoverable situations
2 parents 1e69521 + 39081c8 commit 8d6fbf3

File tree

13 files changed

+426
-207
lines changed

13 files changed

+426
-207
lines changed

‎pkg/comment_actions/parsing.go

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,9 +2,12 @@ package comment_actions
22

33
import (
44
"errors"
5+
"fmt"
6+
"strings"
7+
58
"github.com/jessevdk/go-flags"
69
"github.com/rs/zerolog/log"
7-
"strings"
10+
"github.com/zapier/tfbuddy/pkg/utils"
811
)
912

1013
var (
@@ -36,7 +39,7 @@ func ParseCommentCommand(noteBody string) (*CommentOpts, error) {
3639
_, err := flags.ParseArgs(opts, words)
3740
if err != nil {
3841
log.Error().Err(err).Msg("error parsing comment as command")
39-
return nil, errors.New("could not parse comment as command")
42+
return nil, fmt.Errorf("could not parse comment as command. %w", utils.ErrPermanent)
4043
}
4144

4245
if opts.Args.Agent == "terraform" || opts.Args.Agent == "atlantis" {

‎pkg/git/git_repo.go

Lines changed: 11 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import (
1111
"github.com/go-git/go-git/v5/plumbing"
1212
"github.com/go-git/go-git/v5/plumbing/object"
1313
githttp "github.com/go-git/go-git/v5/plumbing/transport/http"
14+
"github.com/zapier/tfbuddy/pkg/utils"
1415
)
1516

1617
type Repository struct {
@@ -36,7 +37,7 @@ func (gr *Repository) FetchUpstreamBranch(branch string) error {
3637
Auth: gr.authentication,
3738
})
3839
if err != nil && err.Error() != git.NoErrAlreadyUpToDate.Error() {
39-
return err
40+
return utils.CreatePermanentError(err)
4041
}
4142
return nil
4243
}
@@ -45,51 +46,51 @@ func (gr *Repository) GetMergeBase(oldest, newest string) (string, error) {
4546
for _, rev := range []string{oldest, newest} {
4647
hash, err := gr.ResolveRevision(plumbing.Revision(rev))
4748
if err != nil {
48-
return "", err
49+
return "", utils.CreatePermanentError(err)
4950
}
5051
hashes = append(hashes, hash)
5152
}
5253
var commits []*object.Commit
5354
for _, hash := range hashes {
5455
commit, err := gr.CommitObject(*hash)
5556
if err != nil {
56-
return "", err
57+
return "", utils.CreatePermanentError(err)
5758
}
5859
commits = append(commits, commit)
5960
}
6061
res, err := commits[0].MergeBase(commits[1])
6162
if err != nil {
62-
return "", err
63+
return "", utils.CreatePermanentError(err)
6364
}
6465

6566
if len(res) > 0 {
6667
println(res)
6768
return res[0].Hash.String(), nil
6869

6970
}
70-
return "", fmt.Errorf("could not find merge base")
71+
return "", utils.CreatePermanentError(fmt.Errorf("could not find merge base"))
7172
}
7273
func (gr *Repository) GetModifiedFileNamesBetweenCommits(oldest, newest string) ([]string, error) {
7374

7475
oldestSha, err := gr.ResolveRevision(plumbing.Revision(oldest))
7576
if err != nil {
76-
return nil, err
77+
return nil, utils.CreatePermanentError(err)
7778
}
7879
newestSha, err := gr.ResolveRevision(plumbing.Revision(newest))
7980
if err != nil {
80-
return nil, err
81+
return nil, utils.CreatePermanentError(err)
8182
}
8283
oldestCommit, err := gr.CommitObject(*oldestSha)
8384
if err != nil {
84-
return nil, err
85+
return nil, utils.CreatePermanentError(err)
8586
}
8687
newestCommit, err := gr.CommitObject(*newestSha)
8788
if err != nil {
88-
return nil, err
89+
return nil, utils.CreatePermanentError(err)
8990
}
9091
patch, err := oldestCommit.Patch(newestCommit)
9192
if err != nil {
92-
return nil, err
93+
return nil, utils.CreatePermanentError(err)
9394
}
9495
filePatches := patch.FilePatches()
9596

‎pkg/github/client.go

Lines changed: 71 additions & 49 deletions
Original file line numberDiff line numberDiff line change
@@ -2,19 +2,21 @@ package github
22

33
import (
44
"context"
5-
"errors"
65
"fmt"
76
"os"
87
"path/filepath"
98
"strings"
9+
"time"
1010

11+
"github.com/cenkalti/backoff/v4"
1112
"github.com/go-git/go-git/v5"
1213
"github.com/go-git/go-git/v5/plumbing"
1314
"github.com/go-git/go-git/v5/plumbing/protocol/packp/sideband"
1415
githttp "github.com/go-git/go-git/v5/plumbing/transport/http"
1516
gogithub "github.com/google/go-github/v49/github"
1617
"github.com/rs/zerolog/log"
1718
zgit "github.com/zapier/tfbuddy/pkg/git"
19+
"github.com/zapier/tfbuddy/pkg/utils"
1820
"github.com/zapier/tfbuddy/pkg/vcs"
1921
"golang.org/x/oauth2"
2022
)
@@ -28,6 +30,14 @@ type Client struct {
2830
token string
2931
}
3032

33+
const DefaultMaxRetries = 3
34+
35+
func createBackOffWithRetries() backoff.BackOff {
36+
exp := backoff.NewExponentialBackOff()
37+
exp.MaxElapsedTime = 30 * time.Second
38+
return backoff.WithMaxRetries(exp, DefaultMaxRetries)
39+
40+
}
3141
func NewGithubClient() *Client {
3242
token := os.Getenv("GITHUB_TOKEN")
3343
ctx := context.Background()
@@ -78,17 +88,19 @@ func (c *Client) GetRepoFile(fullName string, file string, ref string) ([]byte,
7888
if err != nil {
7989
return nil, err
8090
}
81-
fileContent, _, _, err := c.client.Repositories.GetContents(c.ctx, parts[0], parts[1], file, &gogithub.RepositoryContentGetOptions{Ref: ref})
82-
if err != nil {
83-
return nil, err
84-
}
91+
return backoff.RetryWithData(func() ([]byte, error) {
92+
fileContent, _, _, err := c.client.Repositories.GetContents(c.ctx, parts[0], parts[1], file, &gogithub.RepositoryContentGetOptions{Ref: ref})
93+
if err != nil {
94+
return nil, utils.CreatePermanentError(err)
95+
}
8596

86-
contents, err := fileContent.GetContent()
87-
if err != nil {
88-
return nil, err
89-
}
97+
contents, err := fileContent.GetContent()
98+
if err != nil {
99+
return nil, utils.CreatePermanentError(err)
100+
}
90101

91-
return []byte(contents), nil
102+
return []byte(contents), nil
103+
}, createBackOffWithRetries())
92104
}
93105

94106
func (c *Client) GetMergeRequestModifiedFiles(prID int, fullName string) ([]string, error) {
@@ -102,19 +114,21 @@ func (c *Client) GetMergeRequestModifiedFiles(prID int, fullName string) ([]stri
102114
}
103115

104116
if pr.GetChangedFiles() > 0 {
105-
parts, err := splitFullName(fullName)
106-
if err != nil {
107-
return nil, err
108-
}
109-
files, _, err := c.client.PullRequests.ListFiles(c.ctx, parts[0], parts[1], prID, &opts)
110-
if err != nil {
111-
return nil, err
112-
}
113-
modifiedFiles := make([]string, len(files))
114-
for i, file := range files {
115-
modifiedFiles[i] = file.GetFilename()
116-
}
117-
return modifiedFiles, nil
117+
return backoff.RetryWithData(func() ([]string, error) {
118+
parts, err := splitFullName(fullName)
119+
if err != nil {
120+
return nil, utils.CreatePermanentError(err)
121+
}
122+
files, _, err := c.client.PullRequests.ListFiles(c.ctx, parts[0], parts[1], prID, &opts)
123+
if err != nil {
124+
return nil, utils.CreatePermanentError(err)
125+
}
126+
modifiedFiles := make([]string, len(files))
127+
for i, file := range files {
128+
modifiedFiles[i] = file.GetFilename()
129+
}
130+
return modifiedFiles, nil
131+
}, createBackOffWithRetries())
118132
}
119133
return []string{}, nil
120134
}
@@ -129,7 +143,7 @@ func (c *Client) CloneMergeRequest(project string, mr vcs.MR, dest string) (vcs.
129143

130144
repo, _, err := c.client.Repositories.Get(context.Background(), parts[0], parts[1])
131145
if err != nil {
132-
return nil, err
146+
return nil, utils.CreatePermanentError(err)
133147
}
134148
log.Debug().Msg(*repo.CloneURL)
135149
ref := plumbing.NewBranchReferenceName(mr.GetSourceBranch())
@@ -208,50 +222,58 @@ func (c *Client) GetPipelinesForCommit(projectWithNS string, commitSHA string) (
208222
func (c *Client) GetIssue(owner *gogithub.User, repo string, issueId int) (*gogithub.Issue, error) {
209223
owName, err := ResolveOwnerName(owner)
210224
if err != nil {
211-
return nil, err
225+
return nil, utils.CreatePermanentError(err)
212226
}
213-
iss, _, err := c.client.Issues.Get(context.Background(), owName, repo, issueId)
214-
return iss, err
227+
return backoff.RetryWithData(func() (*gogithub.Issue, error) {
228+
iss, _, err := c.client.Issues.Get(context.Background(), owName, repo, issueId)
229+
return iss, utils.CreatePermanentError(err)
230+
}, createBackOffWithRetries())
215231
}
216232

217233
func (c *Client) GetPullRequest(fullName string, prID int) (*GithubPR, error) {
218234
parts, err := splitFullName(fullName)
219235
if err != nil {
220236
return nil, err
221237
}
222-
pr, _, err := c.client.PullRequests.Get(c.ctx, parts[0], parts[1], prID)
223-
return &GithubPR{pr}, err
238+
return backoff.RetryWithData(func() (*GithubPR, error) {
239+
pr, _, err := c.client.PullRequests.Get(c.ctx, parts[0], parts[1], prID)
240+
return &GithubPR{pr}, utils.CreatePermanentError(err)
241+
}, createBackOffWithRetries())
224242
}
225243

226244
// PostIssueComment adds a comment to an existing Pull Request
227245
func (c *Client) PostIssueComment(prId int, fullName string, body string) (*gogithub.IssueComment, error) {
228246
projectParts, err := splitFullName(fullName)
229247
if err != nil {
230-
return nil, err
231-
}
232-
comment := &gogithub.IssueComment{
233-
Body: String(body),
234-
}
235-
iss, _, err := c.client.Issues.CreateComment(context.Background(), projectParts[0], projectParts[1], prId, comment)
236-
if err != nil {
237-
log.Error().Err(err).Msg("github client: could not post issue comment")
248+
return nil, utils.CreatePermanentError(err)
238249
}
250+
return backoff.RetryWithData(func() (*gogithub.IssueComment, error) {
251+
comment := &gogithub.IssueComment{
252+
Body: String(body),
253+
}
254+
iss, _, err := c.client.Issues.CreateComment(context.Background(), projectParts[0], projectParts[1], prId, comment)
255+
if err != nil {
256+
log.Error().Err(err).Msg("github client: could not post issue comment")
257+
}
239258

240-
return iss, err
259+
return iss, utils.CreatePermanentError(err)
260+
}, createBackOffWithRetries())
241261
}
242262

243263
// PostPullRequestComment adds a review comment to an existing PullRequest
244264
func (c *Client) PostPullRequestComment(owner, repo string, prId int, body string) error {
245265
// TODO: this is broken
246-
comment := &gogithub.PullRequestComment{
247-
//InReplyTo: nil,
248-
Body: String(body),
249-
}
250-
_, _, err := c.client.PullRequests.CreateComment(c.ctx, owner, repo, int(prId), comment)
251-
if err != nil {
252-
log.Error().Err(err).Msg("could not post pull request comment")
253-
}
254-
return err
266+
return backoff.Retry(func() error {
267+
comment := &gogithub.PullRequestComment{
268+
//InReplyTo: nil,
269+
Body: String(body),
270+
}
271+
_, _, err := c.client.PullRequests.CreateComment(c.ctx, owner, repo, int(prId), comment)
272+
if err != nil {
273+
log.Error().Err(err).Msg("could not post pull request comment")
274+
}
275+
return utils.CreatePermanentError(err)
276+
}, createBackOffWithRetries())
255277
}
256278

257279
func String(str string) *string {
@@ -266,7 +288,7 @@ func ResolveOwnerName(owner *gogithub.User) (string, error) {
266288
name = owner.Login
267289
if name == nil {
268290
log.Error().Msg("owner name/login is nil")
269-
return "", errors.New("owner name/login is nil")
291+
return "", fmt.Errorf("owner name/login is nil. %w", utils.ErrPermanent)
270292
}
271293
}
272294
return *name, nil
@@ -275,7 +297,7 @@ func ResolveOwnerName(owner *gogithub.User) (string, error) {
275297
func splitFullName(fullName string) ([]string, error) {
276298
parts := strings.Split(fullName, "/")
277299
if len(parts) != 2 {
278-
return nil, fmt.Errorf("github client: invalid repo format")
300+
return nil, fmt.Errorf("github client: invalid repo format. %w", utils.ErrPermanent)
279301
}
280302
return parts, nil
281303
}

‎pkg/github/hooks/stream_worker.go

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,9 +10,24 @@ import (
1010
"github.com/zapier/tfbuddy/pkg/comment_actions"
1111
"github.com/zapier/tfbuddy/pkg/github"
1212
"github.com/zapier/tfbuddy/pkg/tfc_trigger"
13+
"github.com/zapier/tfbuddy/pkg/utils"
1314
)
1415

1516
func (h *GithubHooksHandler) processIssueCommentEvent(msg *GithubIssueCommentEventMsg) error {
17+
var commentErr error
18+
defer func() {
19+
if r := recover(); r != nil {
20+
log.Error().Msgf("Unrecoverable error in issue comment event processing %v", r)
21+
commentErr = nil
22+
}
23+
}()
24+
commentErr = h.processIssueComment(msg)
25+
return utils.EmitPermanentError(commentErr, func(err error) {
26+
log.Error().Msgf("got a permanent error attempting to process comment event: %s", err.Error())
27+
})
28+
}
29+
30+
func (h *GithubHooksHandler) processIssueComment(msg *GithubIssueCommentEventMsg) error {
1631
if msg == nil || msg.payload == nil {
1732
return errors.New("msg is nil")
1833
}
@@ -102,7 +117,6 @@ func (h *GithubHooksHandler) processIssueCommentEvent(msg *GithubIssueCommentEve
102117
return nil
103118
}
104119
return tfError
105-
106120
}
107121

108122
func (h *GithubHooksHandler) postPullRequestComment(event *gogithub.IssueCommentEvent, body string) error {

0 commit comments

Comments
 (0)