Skip to content

Commit

Permalink
Update github_client.go
Browse files Browse the repository at this point in the history 
Fix potential nil pointers. See runatlantis#4081 for context.
  • Loading branch information
@adkafka
adkafka authored Dec 20, 2023
1 parent ab110c0 commit 5ee9d7b
Showing 1 changed file with 48 additions and 16 deletions.
64 changes: 48 additions & 16 deletions server/events/vcs/github_client.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -141,7 +141,9 @@ listloop:
attemptDelay = 2*attemptDelay + 1*time.Second

pageFiles, resp, err := g.client.PullRequests.ListFiles(g.ctx, repo.Owner, repo.Name, pull.Num, &opts)
g.logger.Debug("[attempt %d] GET /repos/%v/%v/pulls/%d/files returned: %v", i+1, repo.Owner, repo.Name, pull.Num, resp.StatusCode)
if resp != nil {
g.logger.Debug("[attempt %d] GET /repos/%v/%v/pulls/%d/files returned: %v", i+1, repo.Owner, repo.Name, pull.Num, resp.StatusCode)
}
if err != nil {
ghErr, ok := err.(*github.ErrorResponse)
if ok && ghErr.Response.StatusCode == 404 {
Expand Down Expand Up @@ -190,7 +192,9 @@ func (g *GithubClient) CreateComment(repo models.Repo, pullNum int, comment stri
comments := common.SplitComment(comment, maxCommentLength, sepEnd, sepStart)
for i := range comments {
_, resp, err := g.client.Issues.CreateComment(g.ctx, repo.Owner, repo.Name, pullNum, &github.IssueComment{Body: &comments[i]})
g.logger.Debug("POST /repos/%v/%v/issues/%d/comments returned: %v", repo.Owner, repo.Name, pullNum, resp.StatusCode)
if resp != nil {
g.logger.Debug("POST /repos/%v/%v/issues/%d/comments returned: %v", repo.Owner, repo.Name, pullNum, resp.StatusCode)
}
if err != nil {
return err
}
Expand All @@ -201,7 +205,9 @@ func (g *GithubClient) CreateComment(repo models.Repo, pullNum int, comment stri
// ReactToComment adds a reaction to a comment.
func (g *GithubClient) ReactToComment(repo models.Repo, _ int, commentID int64, reaction string) error {
_, resp, err := g.client.Reactions.CreateIssueCommentReaction(g.ctx, repo.Owner, repo.Name, commentID, reaction)
g.logger.Debug("POST /repos/%v/%v/issues/comments/%d/reactions returned: %v", repo.Owner, repo.Name, commentID, resp.StatusCode)
if resp != nil {
g.logger.Debug("POST /repos/%v/%v/issues/comments/%d/reactions returned: %v", repo.Owner, repo.Name, commentID, resp.StatusCode)
}
return err
}

Expand All @@ -214,7 +220,9 @@ func (g *GithubClient) HidePrevCommandComments(repo models.Repo, pullNum int, co
Direction: github.String("asc"),
ListOptions: github.ListOptions{Page: nextPage},
})
g.logger.Debug("GET /repos/%v/%v/issues/%d/comments returned: %v", repo.Owner, repo.Name, pullNum, resp.StatusCode)
if resp != nil {
g.logger.Debug("GET /repos/%v/%v/issues/%d/comments returned: %v", repo.Owner, repo.Name, pullNum, resp.StatusCode)
}
if err != nil {
return errors.Wrap(err, "listing comments")
}
Expand Down Expand Up @@ -328,7 +336,9 @@ func (g *GithubClient) PullIsApproved(repo models.Repo, pull models.PullRequest)
opts.Page = nextPage
}
pageReviews, resp, err := g.client.PullRequests.ListReviews(g.ctx, repo.Owner, repo.Name, pull.Num, &opts)
g.logger.Debug("GET /repos/%v/%v/pulls/%d/reviews returned: %v", repo.Owner, repo.Name, pull.Num, resp.StatusCode)
if resp != nil {
g.logger.Debug("GET /repos/%v/%v/pulls/%d/reviews returned: %v", repo.Owner, repo.Name, pull.Num, resp.StatusCode)
}
if err != nil {
return approvalStatus, errors.Wrap(err, "getting reviews")
}
Expand Down Expand Up @@ -398,7 +408,9 @@ func isRequiredCheck(check string, required []string) bool {
func (g *GithubClient) GetCombinedStatusMinusApply(repo models.Repo, pull *github.PullRequest, vcstatusname string) (bool, error) {
//check combined status api
status, resp, err := g.client.Repositories.GetCombinedStatus(g.ctx, *pull.Head.Repo.Owner.Login, repo.Name, *pull.Head.Ref, nil)
g.logger.Debug("GET /repos/%v/%v/commits/%s/status returned: %v", *pull.Head.Repo.Owner.Login, repo.Name, *pull.Head.Ref, resp.StatusCode)
if resp != nil {
g.logger.Debug("GET /repos/%v/%v/commits/%s/status returned: %v", *pull.Head.Repo.Owner.Login, repo.Name, *pull.Head.Ref, resp.StatusCode)
}
if err != nil {
return false, errors.Wrap(err, "getting combined status")
}
Expand All @@ -415,7 +427,9 @@ func (g *GithubClient) GetCombinedStatusMinusApply(repo models.Repo, pull *githu

//get required status checks
required, resp, err := g.client.Repositories.GetBranchProtection(context.Background(), repo.Owner, repo.Name, *pull.Base.Ref)
g.logger.Debug("GET /repos/%v/%v/branches/%s/protection returned: %v", repo.Owner, repo.Name, *pull.Base.Ref, resp.StatusCode)
if resp != nil {
g.logger.Debug("GET /repos/%v/%v/branches/%s/protection returned: %v", repo.Owner, repo.Name, *pull.Base.Ref, resp.StatusCode)
}
if err != nil {
return false, errors.Wrap(err, "getting required status checks")
}
Expand All @@ -426,7 +440,9 @@ func (g *GithubClient) GetCombinedStatusMinusApply(repo models.Repo, pull *githu

//check check suite/check run api
checksuites, resp, err := g.client.Checks.ListCheckSuitesForRef(context.Background(), *pull.Head.Repo.Owner.Login, repo.Name, *pull.Head.Ref, nil)
g.logger.Debug("GET /repos/%v/%v/commits/%s/check-suites returned: %v", *pull.Head.Repo.Owner.Login, repo.Name, *pull.Head.Ref, resp.StatusCode)
if resp != nil {
g.logger.Debug("GET /repos/%v/%v/commits/%s/check-suites returned: %v", *pull.Head.Repo.Owner.Login, repo.Name, *pull.Head.Ref, resp.StatusCode)
}
if err != nil {
return false, errors.Wrap(err, "getting check suites for ref")
}
Expand All @@ -436,7 +452,9 @@ func (g *GithubClient) GetCombinedStatusMinusApply(repo models.Repo, pull *githu
if *c.Status == "completed" {
//iterate over the runs inside the suite
suite, resp, err := g.client.Checks.ListCheckRunsCheckSuite(context.Background(), *pull.Head.Repo.Owner.Login, repo.Name, *c.ID, nil)
g.logger.Debug("GET /repos/%v/%v/check-suites/%d/check-runs returned: %v", *pull.Head.Repo.Owner.Login, repo.Name, *c.ID, resp.StatusCode)
if resp != nil {
g.logger.Debug("GET /repos/%v/%v/check-suites/%d/check-runs returned: %v", *pull.Head.Repo.Owner.Login, repo.Name, *c.ID, resp.StatusCode)
}
if err != nil {
return false, errors.Wrap(err, "getting check runs for check suite")
}
Expand Down Expand Up @@ -548,7 +566,9 @@ func (g *GithubClient) GetPullRequest(repo models.Repo, num int) (*github.PullRe
attemptDelay = 2*attemptDelay + 1*time.Second

pull, resp, err := g.client.PullRequests.Get(g.ctx, repo.Owner, repo.Name, num)
g.logger.Debug("GET /repos/%v/%v/pulls/%d returned: %v", repo.Owner, repo.Name, num, resp.StatusCode)
if resp != nil {
g.logger.Debug("GET /repos/%v/%v/pulls/%d returned: %v", repo.Owner, repo.Name, num, resp.StatusCode)
}
if err == nil {
return pull, nil
}
Expand Down Expand Up @@ -580,7 +600,9 @@ func (g *GithubClient) UpdateStatus(repo models.Repo, pull models.PullRequest, s
TargetURL: &url,
}
_, resp, err := g.client.Repositories.CreateStatus(g.ctx, repo.Owner, repo.Name, pull.HeadCommit, status)
g.logger.Debug("POST /repos/%v/%v/statuses/%s returned: %v", repo.Owner, repo.Name, pull.HeadCommit, resp.StatusCode)
if resp != nil {
g.logger.Debug("POST /repos/%v/%v/statuses/%s returned: %v", repo.Owner, repo.Name, pull.HeadCommit, resp.StatusCode)
}
return err
}

Expand Down Expand Up @@ -621,7 +643,9 @@ func (g *GithubClient) MergePull(pull models.PullRequest, _ models.PullRequestOp
// the commit message as it normally would.
"",
options)
g.logger.Debug("POST /repos/%v/%v/pulls/%d/merge returned: %v", repo.Owner, repo.Name, pull.Num, resp.StatusCode)
if resp != nil {
g.logger.Debug("POST /repos/%v/%v/pulls/%d/merge returned: %v", repo.Owner, repo.Name, pull.Num, resp.StatusCode)
}
if err != nil {
return errors.Wrap(err, "merging pull request")
}
Expand Down Expand Up @@ -683,7 +707,9 @@ func (g *GithubClient) GetTeamNamesForUser(repo models.Repo, user models.User) (
func (g *GithubClient) ExchangeCode(code string) (*GithubAppTemporarySecrets, error) {
ctx := context.Background()
cfg, resp, err := g.client.Apps.CompleteAppManifest(ctx, code)
g.logger.Debug("POST /app-manifests/%s/conversions returned: %v", code, resp.StatusCode)
if resp != nil {
g.logger.Debug("POST /app-manifests/%s/conversions returned: %v", code, resp.StatusCode)
}
data := &GithubAppTemporarySecrets{
ID: cfg.GetID(),
Key: cfg.GetPEM(),
Expand All @@ -701,7 +727,9 @@ func (g *GithubClient) ExchangeCode(code string) (*GithubAppTemporarySecrets, er
func (g *GithubClient) GetFileContent(pull models.PullRequest, fileName string) (bool, []byte, error) {
opt := github.RepositoryContentGetOptions{Ref: pull.HeadBranch}
fileContent, _, resp, err := g.client.Repositories.GetContents(g.ctx, pull.BaseRepo.Owner, pull.BaseRepo.Name, fileName, &opt)
g.logger.Debug("GET /repos/%v/%v/contents/%s returned: %v", pull.BaseRepo.Owner, pull.BaseRepo.Name, fileName, resp.StatusCode)
if resp != nil {
g.logger.Debug("GET /repos/%v/%v/contents/%s returned: %v", pull.BaseRepo.Owner, pull.BaseRepo.Name, fileName, resp.StatusCode)
}

if resp.StatusCode == http.StatusNotFound {
return false, []byte{}, nil
Expand All @@ -725,7 +753,9 @@ func (g *GithubClient) SupportsSingleFileDownload(_ models.Repo) bool {
func (g *GithubClient) GetCloneURL(_ models.VCSHostType, repo string) (string, error) {
parts := strings.Split(repo, "/")
repository, resp, err := g.client.Repositories.Get(g.ctx, parts[0], parts[1])
g.logger.Debug("GET /repos/%v/%v returned: %v", parts[0], parts[1], resp.StatusCode)
if resp != nil {
g.logger.Debug("GET /repos/%v/%v returned: %v", parts[0], parts[1], resp.StatusCode)
}
if err != nil {
return "", err
}
Expand All @@ -734,7 +764,9 @@ func (g *GithubClient) GetCloneURL(_ models.VCSHostType, repo string) (string, e

func (g *GithubClient) GetPullLabels(repo models.Repo, pull models.PullRequest) ([]string, error) {
pullDetails, resp, err := g.client.PullRequests.Get(g.ctx, repo.Owner, repo.Name, pull.Num)
g.logger.Debug("GET /repos/%v/%v/pulls/%d returned: %v", repo.Owner, repo.Name, pull.Num, resp.StatusCode)
if resp != nil {
g.logger.Debug("GET /repos/%v/%v/pulls/%d returned: %v", repo.Owner, repo.Name, pull.Num, resp.StatusCode)
}
if err != nil {
return nil, err
}
Expand Down

0 comments on commit 5ee9d7b

Please sign in to comment.