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

* fix: do not retry upload if file already exists

Signed-off-by: Carlos Alexandro Becker <caarlos0@gmail.com>

* fix: logs

Signed-off-by: Carlos Alexandro Becker <caarlos0@gmail.com>

* fix: gitea client

Signed-off-by: Carlos Alexandro Becker <caarlos0@gmail.com>

* fix: godocs

Signed-off-by: Carlos Alexandro Becker <caarlos0@gmail.com>

* fix: tests

Signed-off-by: Carlos Alexandro Becker <caarlos0@gmail.com>

internal/client/client.go

@@ -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"
@@ -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)
 	}
@@ -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
+}

internal/client/gitea.go

@@ -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
 }

internal/client/github.go

@@ -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,
@@ -168,5 +168,8 @@ func (c *githubClient) Upload(
 		},
 		file,
 	)
+	if resp.StatusCode == 422 {
 		return err
+	}
+	return RetriableError{err}
 }

internal/client/gitlab.go

@@ -263,7 +263,7 @@ func (c *gitlabClient) Upload(
 		})
 
 	if err != nil {
-		return err
+		return RetriableError{err}
 	}
 
 	log.WithFields(log.Fields{
@@ -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

internal/pipe/release/release.go

@@ -176,10 +176,10 @@ 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{
+			log.WithField("try", try).
-				"try":      try,
+				WithField("artifact", artifact.Name).
-				"artifact": artifact.Name,
+				WithError(err).
-			}).Warnf("failed to upload artifact, will retry")
+				Warnf("failed to upload artifact, will retry")
 			return err
 		}
 		return nil
@@ -187,9 +187,10 @@ func upload(ctx *context.Context, client client.Client, releaseID string, artifa
 	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
 }

internal/pipe/release/release_test.go

@@ -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"
@@ -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)
 }
@@ -526,38 +527,38 @@ type DummyClient struct {
 	Lock                sync.Mutex
 }
 
-func (client *DummyClient) CreateRelease(ctx *context.Context, body string) (releaseID string, err error) {
+func (c *DummyClient) CreateRelease(ctx *context.Context, body string) (releaseID string, err error) {
-	if client.FailToCreateRelease {
+	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 {
+func (c *DummyClient) Upload(ctx *context.Context, releaseID string, artifact *artifact.Artifact, file *os.File) error {
-	client.Lock.Lock()
+	c.Lock.Lock()
-	defer client.Lock.Unlock()
+	defer c.Lock.Unlock()
-	if client.UploadedFilePaths == nil {
+	if c.UploadedFilePaths == nil {
-		client.UploadedFilePaths = map[string]string{}
+		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 {
+	if c.FailFirstUpload {
-		client.FailFirstUpload = false
+		c.FailFirstUpload = false
-		return errors.New("upload failed, should retry")
+		return client.RetriableError{errors.New("upload failed, should retry")}
 	}
-	client.UploadedFile = true
+	c.UploadedFile = true
-	client.UploadedFileNames = append(client.UploadedFileNames, artifact.Name)
+	c.UploadedFileNames = append(c.UploadedFileNames, artifact.Name)
-	client.UploadedFilePaths[artifact.Name] = artifact.Path
+	c.UploadedFilePaths[artifact.Name] = artifact.Path
 	return nil
 }