From aa9986e8268daed6b4adaa5d11a81f98dc20c11b Mon Sep 17 00:00:00 2001 From: Carlos Alexandro Becker Date: Tue, 12 Dec 2023 16:12:07 -0300 Subject: [PATCH] fix(github): do not fail branch creation if it already exists (#4471) github api is eventually consistent... so, we might ask for the branch, it might say it does't exist, and when we try and create it, it might error because it already exists. this should avoid breaking it --------- Signed-off-by: Carlos Alexandro Becker --- internal/client/github.go | 16 +++++---- internal/client/github_test.go | 62 ++++++++++++++++++++++++++++++++++ 2 files changed, 72 insertions(+), 6 deletions(-) diff --git a/internal/client/github.go b/internal/client/github.go index a320d6545fd..27f5caaf660 100644 --- a/internal/client/github.go +++ b/internal/client/github.go @@ -2,6 +2,7 @@ package client import ( "crypto/tls" + "errors" "fmt" "net/http" "net/url" @@ -231,7 +232,7 @@ func (c *githubClient) OpenPullRequest( }, ) if err != nil { - if res.StatusCode == 422 { + if res.StatusCode == http.StatusUnprocessableEntity { log.WithError(err).Warn("pull request validation failed") return nil } @@ -283,11 +284,11 @@ func (c *githubClient) CreateFile( if defBranch != branch && branch != "" { _, res, err := c.client.Repositories.GetBranch(ctx, repo.Owner, repo.Name, branch, 100) - if err != nil && (res == nil || res.StatusCode != 404) { + if err != nil && (res == nil || res.StatusCode != http.StatusNotFound) { return fmt.Errorf("could not get branch %q: %w", branch, err) } - if res.StatusCode == 404 { + if res.StatusCode == http.StatusNotFound { defRef, _, err := c.client.Git.GetRef(ctx, repo.Owner, repo.Name, "refs/heads/"+defBranch) if err != nil { return fmt.Errorf("could not get ref %q: %w", "refs/heads/"+defBranch, err) @@ -299,7 +300,10 @@ func (c *githubClient) CreateFile( SHA: defRef.Object.SHA, }, }); err != nil { - return fmt.Errorf("could not create ref %q from %q: %w", "refs/heads/"+branch, defRef.Object.GetSHA(), err) + rerr := new(github.ErrorResponse) + if !errors.As(err, &rerr) || rerr.Message != "Reference already exists" { + return fmt.Errorf("could not create ref %q from %q: %w", "refs/heads/"+branch, defRef.Object.GetSHA(), err) + } } } } @@ -313,7 +317,7 @@ func (c *githubClient) CreateFile( Ref: branch, }, ) - if err != nil && (res == nil || res.StatusCode != 404) { + if err != nil && (res == nil || res.StatusCode != http.StatusNotFound) { return fmt.Errorf("could not get %q: %w", path, err) } @@ -475,7 +479,7 @@ func (c *githubClient) Upload( if err == nil { return nil } - if resp != nil && resp.StatusCode == 422 { + if resp != nil && resp.StatusCode == http.StatusUnprocessableEntity { return err } return RetriableError{err} diff --git a/internal/client/github_test.go b/internal/client/github_test.go index 5f91f6763ad..ab4f847973f 100644 --- a/internal/client/github_test.go +++ b/internal/client/github_test.go @@ -808,6 +808,68 @@ func TestGitHubCreateFileHappyPathUpdate(t *testing.T) { require.NoError(t, client.CreateFile(ctx, config.CommitAuthor{}, repo, []byte("content"), "file.txt", "message")) } +func TestGitHubCreateFileFeatureBranchAlreadyExists(t *testing.T) { + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + defer r.Body.Close() + + if r.URL.Path == "/repos/someone/something/branches/feature" && r.Method == http.MethodGet { + w.WriteHeader(http.StatusNotFound) + return + } + + if r.URL.Path == "/repos/someone/something/git/ref/heads/main" { + fmt.Fprint(w, `{"object": {"sha": "fake-sha"}}`) + return + } + + if r.URL.Path == "/repos/someone/something/git/refs" && r.Method == http.MethodPost { + w.WriteHeader(http.StatusUnprocessableEntity) + fmt.Fprintf(w, `{"message": "Reference already exists"}`) + return + } + + if r.URL.Path == "/repos/someone/something" { + w.WriteHeader(http.StatusOK) + fmt.Fprint(w, `{"default_branch": "main"}`) + return + } + + if r.URL.Path == "/repos/someone/something/contents/file.txt" && r.Method == http.MethodGet { + w.WriteHeader(http.StatusNotFound) + return + } + + if r.URL.Path == "/repos/someone/something/contents/file.txt" && r.Method == http.MethodPut { + w.WriteHeader(http.StatusOK) + return + } + + if r.URL.Path == "/rate_limit" { + w.WriteHeader(http.StatusOK) + fmt.Fprint(w, `{"resources":{"core":{"remaining":120}}}`) + return + } + + t.Error("unhandled request: " + r.Method + " " + r.URL.Path) + })) + defer srv.Close() + + ctx := testctx.NewWithCfg(config.Project{ + GitHubURLs: config.GitHubURLs{ + API: srv.URL + "/", + }, + }) + client, err := newGitHub(ctx, "test-token") + require.NoError(t, err) + repo := Repo{ + Owner: "someone", + Name: "something", + Branch: "feature", + } + + require.NoError(t, client.CreateFile(ctx, config.CommitAuthor{}, repo, []byte("content"), "file.txt", "message")) +} + func TestGitHubCreateFileFeatureBranchDoesNotExist(t *testing.T) { srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { defer r.Body.Close()