Skip to content

Commit

Permalink
fix(github): do not fail branch creation if it already exists (#4471)
Browse files Browse the repository at this point in the history
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 <caarlos0@users.noreply.github.com>
  • Loading branch information
caarlos0 committed Dec 12, 2023
1 parent 3f54b5e commit aa9986e
Show file tree
Hide file tree
Showing 2 changed files with 72 additions and 6 deletions.
16 changes: 10 additions & 6 deletions internal/client/github.go
Expand Up @@ -2,6 +2,7 @@ package client

import (
"crypto/tls"
"errors"
"fmt"
"net/http"
"net/url"
Expand Down Expand Up @@ -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
}
Expand Down Expand Up @@ -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)
Expand All @@ -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)
}
}
}
}
Expand All @@ -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)
}

Expand Down Expand Up @@ -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}
Expand Down
62 changes: 62 additions & 0 deletions internal/client/github_test.go
Expand Up @@ -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()
Expand Down

0 comments on commit aa9986e

Please sign in to comment.