Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: do not retry upload if file already exists #1390

Merged
merged 5 commits into from Mar 22, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
15 changes: 15 additions & 0 deletions internal/client/client.go
Expand Up @@ -4,6 +4,7 @@ package client
import (
"os"

"github.com/apex/log"
"github.com/goreleaser/goreleaser/internal/artifact"
"github.com/goreleaser/goreleaser/pkg/config"
"github.com/goreleaser/goreleaser/pkg/context"
Expand All @@ -25,6 +26,7 @@ type Client interface {

// New creates a new client depending on the token type
func New(ctx *context.Context) (Client, error) {
log.WithField("type", ctx.TokenType).Info("token type")
if ctx.TokenType == context.TokenTypeGitHub {
return NewGitHub(ctx)
}
Expand All @@ -36,3 +38,16 @@ func New(ctx *context.Context) (Client, error) {
}
return nil, nil
}

// RetriableError is an error that will cause the action to be retried.
type RetriableError struct {
Err error
}

func (e RetriableError) Error() string {
return e.Err.Error()
}

func (e RetriableError) Retriable() bool {
return true
}
5 changes: 4 additions & 1 deletion internal/client/gitea.go
Expand Up @@ -184,5 +184,8 @@ func (c *giteaClient) Upload(
repoName := releaseConfig.Gitea.Name

_, err = c.client.CreateReleaseAttachment(owner, repoName, giteaReleaseID, file, artifact.Name)
return err
if err != nil {
return RetriableError{err}
}
return nil
}
7 changes: 5 additions & 2 deletions internal/client/github.go
Expand Up @@ -158,7 +158,7 @@ func (c *githubClient) Upload(
if err != nil {
return err
}
_, _, err = c.client.Repositories.UploadReleaseAsset(
_, resp, err := c.client.Repositories.UploadReleaseAsset(
ctx,
ctx.Config.Release.GitHub.Owner,
ctx.Config.Release.GitHub.Name,
Expand All @@ -168,5 +168,8 @@ func (c *githubClient) Upload(
},
file,
)
return err
if resp.StatusCode == 422 {
return err
}
return RetriableError{err}
}
4 changes: 2 additions & 2 deletions internal/client/gitlab.go
Expand Up @@ -263,7 +263,7 @@ func (c *gitlabClient) Upload(
})

if err != nil {
return err
return RetriableError{err}
}

log.WithFields(log.Fields{
Expand All @@ -284,7 +284,7 @@ func (c *gitlabClient) Upload(
// in following publish pipes like brew, scoop
artifact.Extra["ArtifactUploadHash"] = fileUploadHash

return err
return nil
}

// extractProjectFileHashFrom extracts the hash from the
Expand Down
11 changes: 6 additions & 5 deletions internal/pipe/release/release.go
Expand Up @@ -176,20 +176,21 @@ func upload(ctx *context.Context, client client.Client, releaseID string, artifa
defer file.Close() // nolint: errcheck
log.WithField("file", file.Name()).WithField("name", artifact.Name).Info("uploading to release")
if err := client.Upload(ctx, releaseID, artifact, file); err != nil {
log.WithFields(log.Fields{
"try": try,
"artifact": artifact.Name,
}).Warnf("failed to upload artifact, will retry")
log.WithField("try", try).
WithField("artifact", artifact.Name).
WithError(err).
Warnf("failed to upload artifact, will retry")
return err
}
return nil
}
how := []func(uint, error) bool{
strategy.Limit(10),
strategy.Backoff(backoff.Linear(50 * time.Millisecond)),
strategy.CheckError(false),
}
if err := retry.Try(ctx, what, how...); err != nil {
return errors.Wrapf(err, "failed to upload %s after %d retries", artifact.Name, repeats)
return errors.Wrapf(err, "failed to upload %s after %d tries", artifact.Name, repeats)
}
return nil
}
Expand Down
35 changes: 18 additions & 17 deletions internal/pipe/release/release_test.go
Expand Up @@ -9,6 +9,7 @@ import (
"testing"

"github.com/goreleaser/goreleaser/internal/artifact"
"github.com/goreleaser/goreleaser/internal/client"
"github.com/goreleaser/goreleaser/internal/testlib"
"github.com/goreleaser/goreleaser/pkg/config"
"github.com/goreleaser/goreleaser/pkg/context"
Expand Down Expand Up @@ -221,7 +222,7 @@ func TestRunPipeUploadFailure(t *testing.T) {
client := &DummyClient{
FailToUpload: true,
}
assert.EqualError(t, doPublish(ctx, client), "failed to upload bin.tar.gz after 10 retries: upload failed")
assert.EqualError(t, doPublish(ctx, client), "failed to upload bin.tar.gz after 1 tries: upload failed")
assert.True(t, client.CreatedRelease)
assert.False(t, client.UploadedFile)
}
Expand Down Expand Up @@ -526,38 +527,38 @@ type DummyClient struct {
Lock sync.Mutex
}

func (client *DummyClient) CreateRelease(ctx *context.Context, body string) (releaseID string, err error) {
if client.FailToCreateRelease {
func (c *DummyClient) CreateRelease(ctx *context.Context, body string) (releaseID string, err error) {
if c.FailToCreateRelease {
return "", errors.New("release failed")
}
client.CreatedRelease = true
c.CreatedRelease = true
return
}

func (client *DummyClient) CreateFile(ctx *context.Context, commitAuthor config.CommitAuthor, repo config.Repo, content []byte, path, msg string) (err error) {
func (c *DummyClient) CreateFile(ctx *context.Context, commitAuthor config.CommitAuthor, repo config.Repo, content []byte, path, msg string) (err error) {
return
}

func (client *DummyClient) Upload(ctx *context.Context, releaseID string, artifact *artifact.Artifact, file *os.File) error {
client.Lock.Lock()
defer client.Lock.Unlock()
if client.UploadedFilePaths == nil {
client.UploadedFilePaths = map[string]string{}
func (c *DummyClient) Upload(ctx *context.Context, releaseID string, artifact *artifact.Artifact, file *os.File) error {
c.Lock.Lock()
defer c.Lock.Unlock()
if c.UploadedFilePaths == nil {
c.UploadedFilePaths = map[string]string{}
}
// ensure file is read to better mimic real behavior
_, err := ioutil.ReadAll(file)
if err != nil {
return errors.Wrapf(err, "unexpected error")
}
if client.FailToUpload {
if c.FailToUpload {
return errors.New("upload failed")
}
if client.FailFirstUpload {
client.FailFirstUpload = false
return errors.New("upload failed, should retry")
if c.FailFirstUpload {
c.FailFirstUpload = false
return client.RetriableError{errors.New("upload failed, should retry")}
}
client.UploadedFile = true
client.UploadedFileNames = append(client.UploadedFileNames, artifact.Name)
client.UploadedFilePaths[artifact.Name] = artifact.Path
c.UploadedFile = true
c.UploadedFileNames = append(c.UploadedFileNames, artifact.Name)
c.UploadedFilePaths[artifact.Name] = artifact.Path
return nil
}