Skip to content

Commit

Permalink
Retry get pull request calls to github
Browse files Browse the repository at this point in the history
These calls have been 404'ing lately, likely due to eventual consistency
issues on GitHub's side where they send the PR created webhook but it's
not yet available on their API.

To work around this, we will retry up to 3 times with a 1s delay.
  • Loading branch information
lkysow committed Jul 26, 2020
1 parent e04c454 commit 67b2217
Show file tree
Hide file tree
Showing 2 changed files with 65 additions and 1 deletion.
21 changes: 20 additions & 1 deletion server/events/vcs/github_client.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import (
"context"
"fmt"
"strings"
"time"

"github.com/runatlantis/atlantis/server/events/models"
"github.com/runatlantis/atlantis/server/events/vcs/common"
Expand Down Expand Up @@ -273,7 +274,25 @@ func (g *GithubClient) PullIsMergeable(repo models.Repo, pull models.PullRequest

// GetPullRequest returns the pull request.
func (g *GithubClient) GetPullRequest(repo models.Repo, num int) (*github.PullRequest, error) {
pull, _, err := g.client.PullRequests.Get(g.ctx, repo.Owner, repo.Name, num)
var err error
var pull *github.PullRequest

// GitHub has started to return 404's here (#1019) even after they send the webhook.
// They've got some eventual consistency issues going on so we're just going
// to retry up to 3 times with a 1s sleep.
numRetries := 3
retryDelay := 1 * time.Second
for i := 0; i < numRetries; i++ {
pull, _, err = g.client.PullRequests.Get(g.ctx, repo.Owner, repo.Name, num)
if err == nil {
return pull, nil
}
ghErr, ok := err.(*github.ErrorResponse)
if !ok || ghErr.Response.StatusCode != 404 {
return pull, err
}
time.Sleep(retryDelay)
}
return pull, err
}

Expand Down
45 changes: 45 additions & 0 deletions server/events/vcs/github_client_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -852,3 +852,48 @@ func TestGithubClient_SplitComments(t *testing.T) {
Assert(t, strings.Contains(firstSplit, models.PlanCommand.String()), fmt.Sprintf("comment should contain the command name but was %q", firstSplit))
Assert(t, strings.Contains(secondSplit, "continued from previous comment"), fmt.Sprintf("comment should contain no reference to the command name but was %q", secondSplit))
}

// Test that we retry the get pull request call if it 404s.
func TestGithubClient_Retry404(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":
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"}, nil)
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",
},
}
_, err = client.GetPullRequest(repo, 1)
Ok(t, err)
Equals(t, 3, numCalls)
}

0 comments on commit 67b2217

Please sign in to comment.