From e8f109d8eb1ce90271d1a4b5c53b25faffc8d555 Mon Sep 17 00:00:00 2001
From: Iain Lane <iain@orangesquash.org.uk>
Date: Mon, 17 Jan 2022 11:08:27 +0000
Subject: [PATCH] Retry /files/ requests to github

Similar to #1131, we see this for /files/ too, resulting in a plan
error.

Closes #1905
---
 server/events/vcs/github_client.go      | 49 +++++++++++++++++--------
 server/events/vcs/github_client_test.go | 46 +++++++++++++++++++++++
 2 files changed, 80 insertions(+), 15 deletions(-)

diff --git a/server/events/vcs/github_client.go b/server/events/vcs/github_client.go
index fc6e80207c..47eb64565e 100644
--- a/server/events/vcs/github_client.go
+++ b/server/events/vcs/github_client.go
@@ -112,6 +112,8 @@ 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,
@@ -119,24 +121,41 @@ func (g *GithubClient) GetModifiedFiles(repo models.Repo, pull models.PullReques
 		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 {
+				break listloop
+			}
+			nextPage = resp.NextPage
 		}
-		if resp.NextPage == 0 {
-			break
-		}
-		nextPage = resp.NextPage
 	}
 	return files, nil
 }
diff --git a/server/events/vcs/github_client_test.go b/server/events/vcs/github_client_test.go
index bb4f55703e..88f0888d3f 100644
--- a/server/events/vcs/github_client_test.go
+++ b/server/events/vcs/github_client_test.go
@@ -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)
+}