Skip to content

Commit

Permalink
Retry /files/ requests to github (runatlantis#2002)
Browse files Browse the repository at this point in the history
Similar to runatlantis#1131, we see this for /files/ too, resulting in a plan
error.

Closes runatlantis#1905
  • Loading branch information
iainlane authored and krrrr38 committed Dec 16, 2022
1 parent 95b5d82 commit f046361
Show file tree
Hide file tree
Showing 2 changed files with 80 additions and 14 deletions.
48 changes: 34 additions & 14 deletions server/events/vcs/github_client.go
Original file line number Diff line number Diff line change
Expand Up @@ -112,31 +112,51 @@ func NewGithubClient(hostname string, credentials GithubCredentials, logger logg
func (g *GithubClient) GetModifiedFiles(repo models.Repo, pull models.PullRequest) ([]string, error) {
var files []string
nextPage := 0

listloop:
for {
opts := github.ListOptions{
PerPage: 300,
}
if nextPage != 0 {
opts.Page = nextPage
}
g.logger.Debug("GET /repos/%v/%v/pulls/%d/files", repo.Owner, repo.Name, pull.Num)
pageFiles, resp, err := g.client.PullRequests.ListFiles(g.ctx, repo.Owner, repo.Name, pull.Num, &opts)
if err != nil {
return files, err
}
for _, f := range pageFiles {
files = append(files, f.GetFilename())
// GitHub has started to return 404's sometimes. They've got some
// eventual consistency issues going on so we're just going to attempt
// up to 5 times for each page with exponential backoff.
maxAttempts := 5
attemptDelay := 0 * time.Second
for i := 0; i < maxAttempts; i++ {
// First don't sleep, then sleep 1, 3, 7, etc.
time.Sleep(attemptDelay)
attemptDelay = 2*attemptDelay + 1*time.Second

g.logger.Debug("[attempt %d] GET /repos/%v/%v/pulls/%d/files", i+1, repo.Owner, repo.Name, pull.Num)
pageFiles, resp, err := g.client.PullRequests.ListFiles(g.ctx, repo.Owner, repo.Name, pull.Num, &opts)
if err != nil {
ghErr, ok := err.(*github.ErrorResponse)
if ok && ghErr.Response.StatusCode == 404 {
// (hopefully) transient 404, retry after backoff
continue
}
// something else, give up
return files, err
}
for _, f := range pageFiles {
files = append(files, f.GetFilename())

// If the file was renamed, we'll want to run plan in the directory
// it was moved from as well.
if f.GetStatus() == "renamed" {
files = append(files, f.GetPreviousFilename())
// If the file was renamed, we'll want to run plan in the directory
// it was moved from as well.
if f.GetStatus() == "renamed" {
files = append(files, f.GetPreviousFilename())
}
}
}
if resp.NextPage == 0 {
if resp.NextPage == 0 {
break listloop
}
nextPage = resp.NextPage
break
}
nextPage = resp.NextPage
}
return files, nil
}
Expand Down
46 changes: 46 additions & 0 deletions server/events/vcs/github_client_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -907,3 +907,49 @@ func TestGithubClient_Retry404(t *testing.T) {
Ok(t, err)
Equals(t, 3, numCalls)
}

// Test that we retry the get pull request files call if it 404s.
func TestGithubClient_Retry404Files(t *testing.T) {
var numCalls = 0

testServer := httptest.NewTLSServer(
http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {

switch r.Method + " " + r.RequestURI {
case "GET /api/v3/repos/runatlantis/atlantis/pulls/1/files?per_page=300":
defer r.Body.Close() // nolint: errcheck
numCalls++
if numCalls < 3 {
w.WriteHeader(404)
} else {
w.WriteHeader(200)
}
return
default:
t.Errorf("got unexpected request at %q", r.RequestURI)
http.Error(w, "not found", http.StatusNotFound)
return
}
}))

testServerURL, err := url.Parse(testServer.URL)
Ok(t, err)
client, err := vcs.NewGithubClient(testServerURL.Host, &vcs.GithubUserCredentials{"user", "pass"}, logging.NewNoopLogger(t))
Ok(t, err)
defer disableSSLVerification()()
repo := models.Repo{
FullName: "runatlantis/atlantis",
Owner: "runatlantis",
Name: "atlantis",
CloneURL: "",
SanitizedCloneURL: "",
VCSHost: models.VCSHost{
Type: models.Github,
Hostname: "github.com",
},
}
pr := models.PullRequest{Num: 1}
_, err = client.GetModifiedFiles(repo, pr)
Ok(t, err)
Equals(t, 3, numCalls)
}

0 comments on commit f046361

Please sign in to comment.