Skip to content

Commit

Permalink
fix: only fail announcing phase in the end (#3666)
Browse files Browse the repository at this point in the history
This prevents one announce failure to skip all other announcers.

The release will still report a failure in the end, but will not fail in
the first failure.

Also improved errors messages a little bit.

closes #3663

Signed-off-by: Carlos A Becker <caarlos0@users.noreply.github.com>
  • Loading branch information
caarlos0 committed Dec 28, 2022
1 parent 1d2842c commit 47ce9a9
Show file tree
Hide file tree
Showing 27 changed files with 158 additions and 88 deletions.
2 changes: 2 additions & 0 deletions go.mod
Expand Up @@ -22,6 +22,7 @@ require (
github.com/google/uuid v1.3.0
github.com/goreleaser/fileglob v1.3.0
github.com/goreleaser/nfpm/v2 v2.22.2
github.com/hashicorp/go-multierror v1.1.1
github.com/imdario/mergo v0.3.13
github.com/invopop/jsonschema v0.7.0
github.com/jarcoal/httpmock v1.2.0
Expand Down Expand Up @@ -132,6 +133,7 @@ require (
github.com/googleapis/gax-go/v2 v2.4.0 // indirect
github.com/goreleaser/chglog v0.2.2 // indirect
github.com/gorilla/websocket v1.5.0 // indirect
github.com/hashicorp/errwrap v1.1.0 // indirect
github.com/hashicorp/go-cleanhttp v0.5.2 // indirect
github.com/hashicorp/go-retryablehttp v0.7.1 // indirect
github.com/hashicorp/go-version v1.2.1 // indirect
Expand Down
2 changes: 2 additions & 0 deletions go.sum
Expand Up @@ -935,6 +935,7 @@ github.com/hashicorp/consul/sdk v0.8.0/go.mod h1:GBvyrGALthsZObzUGsfgHZQDXjg4lOj
github.com/hashicorp/cronexpr v1.1.1/go.mod h1:P4wA0KBl9C5q2hABiMO7cp6jcIg96CDh1Efb3g1PWA4=
github.com/hashicorp/errwrap v0.0.0-20141028054710-7554cd9344ce/go.mod h1:YH+1FKiLXxHSkmPseP+kNlulaMuP3n2brvKWEqk/Jc4=
github.com/hashicorp/errwrap v1.0.0/go.mod h1:YH+1FKiLXxHSkmPseP+kNlulaMuP3n2brvKWEqk/Jc4=
github.com/hashicorp/errwrap v1.1.0 h1:OxrOeh75EUXMY8TBjag2fzXGZ40LB6IKw45YeGUDY2I=
github.com/hashicorp/errwrap v1.1.0/go.mod h1:YH+1FKiLXxHSkmPseP+kNlulaMuP3n2brvKWEqk/Jc4=
github.com/hashicorp/go-cleanhttp v0.5.0/go.mod h1:JpRdi6/HCYpAwUzNwuwqhbovhLtngrth3wmdIIUrZ80=
github.com/hashicorp/go-cleanhttp v0.5.1/go.mod h1:JpRdi6/HCYpAwUzNwuwqhbovhLtngrth3wmdIIUrZ80=
Expand All @@ -950,6 +951,7 @@ github.com/hashicorp/go-msgpack v0.5.3/go.mod h1:ahLV/dePpqEmjfWmKiqvPkv/twdG7iP
github.com/hashicorp/go-multierror v0.0.0-20161216184304-ed905158d874/go.mod h1:JMRHfdO9jKNzS/+BTlxCjKNQHg/jZAft8U7LloJvN7I=
github.com/hashicorp/go-multierror v1.0.0/go.mod h1:dHtQlpGsu+cZNNAkkCN/P3hoUDHhCYQXV3UM06sGGrk=
github.com/hashicorp/go-multierror v1.1.0/go.mod h1:spPvp8C1qA32ftKqdAHm4hHTbPw+vmowP0z+KUhOZdA=
github.com/hashicorp/go-multierror v1.1.1 h1:H5DkEtf6CXdFp0N0Em5UCwQpXMWke8IA0+lD48awMYo=
github.com/hashicorp/go-multierror v1.1.1/go.mod h1:iw975J/qwKPdAO1clOe2L8331t/9/fmwbPZ6JB6eMoM=
github.com/hashicorp/go-retryablehttp v0.5.3/go.mod h1:9B5zBasrRhHXnJnui7y6sL7es7NDiJgTc6Er0maI1Xs=
github.com/hashicorp/go-retryablehttp v0.7.1 h1:sUiuQAnLlbvmExtFQs72iFW/HXeUn8Z1aJLQ4LJJbTQ=
Expand Down
29 changes: 29 additions & 0 deletions internal/middleware/errhandler/error.go
Expand Up @@ -5,6 +5,7 @@ import (
"github.com/goreleaser/goreleaser/internal/middleware"
"github.com/goreleaser/goreleaser/internal/pipe"
"github.com/goreleaser/goreleaser/pkg/context"
"github.com/hashicorp/go-multierror"
)

// Handle handles an action error, ignoring and logging pipe skipped
Expand All @@ -22,3 +23,31 @@ func Handle(action middleware.Action) middleware.Action {
return err
}
}

// Memo is a handler that memorizes errors, so you can grab them all in the end
// instead of returning each of them.
type Memo struct {
err error
}

// Error returns the underlying error.
func (m *Memo) Error() error {
return m.err
}

// Wrap the given action, memorizing its errors.
// The resulting action will always return a nil error.
func (m *Memo) Wrap(action middleware.Action) middleware.Action {
return func(ctx *context.Context) error {
err := action(ctx)
if err == nil {
return nil
}
if pipe.IsSkip(err) {
log.WithField("reason", err.Error()).Warn("pipe skipped")
return nil
}
m.err = multierror.Append(m.err, err)
return nil
}
}
28 changes: 28 additions & 0 deletions internal/middleware/errhandler/error_test.go
@@ -1,11 +1,13 @@
package errhandler

import (
"errors"
"fmt"
"testing"

"github.com/goreleaser/goreleaser/internal/pipe"
"github.com/goreleaser/goreleaser/pkg/context"
"github.com/hashicorp/go-multierror"
"github.com/stretchr/testify/require"
)

Expand All @@ -28,3 +30,29 @@ func TestError(t *testing.T) {
})(nil))
})
}

func TestErrorMemo(t *testing.T) {
memo := Memo{}
t.Run("no errors", func(t *testing.T) {
require.NoError(t, memo.Wrap(func(ctx *context.Context) error {
return nil
})(nil))
})

t.Run("pipe skipped", func(t *testing.T) {
require.NoError(t, memo.Wrap(func(ctx *context.Context) error {
return pipe.ErrSkipValidateEnabled
})(nil))
})

t.Run("some err", func(t *testing.T) {
require.NoError(t, memo.Wrap(func(ctx *context.Context) error {
return fmt.Errorf("pipe errored")
})(nil))
})

err := memo.Error()
merr := &multierror.Error{}
require.True(t, errors.As(err, &merr), "must be a multierror")
require.Len(t, merr.Errors, 1)
}
15 changes: 7 additions & 8 deletions internal/pipe/announce/announce.go
Expand Up @@ -68,16 +68,15 @@ func (Pipe) Skip(ctx *context.Context) bool {

// Run the pipe.
func (Pipe) Run(ctx *context.Context) error {
memo := errhandler.Memo{}
for _, announcer := range announcers {
if err := skip.Maybe(
_ = skip.Maybe(
announcer,
logging.PadLog(
announcer.String(),
errhandler.Handle(announcer.Announce),
),
)(ctx); err != nil {
return fmt.Errorf("%s: failed to announce release: %w", announcer.String(), err)
}
logging.PadLog(announcer.String(), memo.Wrap(announcer.Announce)),
)(ctx)
}
if memo.Error() != nil {
return fmt.Errorf("failed to announce release: %w", memo.Error())
}
return nil
}
12 changes: 11 additions & 1 deletion internal/pipe/announce/announce_test.go
@@ -1,10 +1,12 @@
package announce

import (
"errors"
"testing"

"github.com/goreleaser/goreleaser/pkg/config"
"github.com/goreleaser/goreleaser/pkg/context"
"github.com/hashicorp/go-multierror"
"github.com/stretchr/testify/require"
)

Expand All @@ -18,9 +20,17 @@ func TestAnnounce(t *testing.T) {
Twitter: config.Twitter{
Enabled: true,
},
Mastodon: config.Mastodon{
Enabled: true,
Server: "https://localhost:1234/",
},
},
})
require.Error(t, Pipe{}.Run(ctx))
err := Pipe{}.Run(ctx)
require.Error(t, err)
merr := &multierror.Error{}
require.True(t, errors.As(err, &merr), "must be a multierror")
require.Len(t, merr.Errors, 2)
}

func TestAnnounceAllDisabled(t *testing.T) {
Expand Down
10 changes: 5 additions & 5 deletions internal/pipe/discord/discord.go
Expand Up @@ -49,24 +49,24 @@ func (p Pipe) Default(ctx *context.Context) error {
func (p Pipe) Announce(ctx *context.Context) error {
msg, err := tmpl.New(ctx).Apply(ctx.Config.Announce.Discord.MessageTemplate)
if err != nil {
return fmt.Errorf("announce: failed to announce to discord: %w", err)
return fmt.Errorf("discord: %w", err)
}

var cfg Config
if err = env.Parse(&cfg); err != nil {
return fmt.Errorf("announce: failed to announce to discord: %w", err)
return fmt.Errorf("discord: %w", err)
}

log.Infof("posting: '%s'", msg)

webhookID, err := snowflake.Parse(cfg.WebhookID)
if err != nil {
return fmt.Errorf("announce: failed to announce to discord: %w", err)
return fmt.Errorf("discord: %w", err)
}

color, err := strconv.Atoi(ctx.Config.Announce.Discord.Color)
if err != nil {
return fmt.Errorf("announce: failed to announce to discord: %w", err)
return fmt.Errorf("discord: %w", err)
}
if _, err = webhook.New(webhookID, cfg.WebhookToken).CreateMessage(discord.WebhookMessageCreate{
Embeds: []discord.Embed{
Expand All @@ -80,7 +80,7 @@ func (p Pipe) Announce(ctx *context.Context) error {
},
},
}); err != nil {
return fmt.Errorf("announce: failed to announce to discord: %w", err)
return fmt.Errorf("discord: %w", err)
}
return nil
}
4 changes: 2 additions & 2 deletions internal/pipe/discord/discord_test.go
Expand Up @@ -26,7 +26,7 @@ func TestAnnounceInvalidTemplate(t *testing.T) {
},
},
})
require.EqualError(t, Pipe{}.Announce(ctx), `announce: failed to announce to discord: template: tmpl:1: unexpected "}" in operand`)
require.EqualError(t, Pipe{}.Announce(ctx), `discord: template: tmpl:1: unexpected "}" in operand`)
}

func TestAnnounceMissingEnv(t *testing.T) {
Expand All @@ -36,7 +36,7 @@ func TestAnnounceMissingEnv(t *testing.T) {
},
})
require.NoError(t, Pipe{}.Default(ctx))
require.EqualError(t, Pipe{}.Announce(ctx), `announce: failed to announce to discord: env: environment variable "DISCORD_WEBHOOK_ID" should not be empty; environment variable "DISCORD_WEBHOOK_TOKEN" should not be empty`)
require.EqualError(t, Pipe{}.Announce(ctx), `discord: env: environment variable "DISCORD_WEBHOOK_ID" should not be empty; environment variable "DISCORD_WEBHOOK_TOKEN" should not be empty`)
}

func TestSkip(t *testing.T) {
Expand Down
8 changes: 4 additions & 4 deletions internal/pipe/linkedin/linkedin.go
Expand Up @@ -31,25 +31,25 @@ func (Pipe) Default(ctx *context.Context) error {
func (Pipe) Announce(ctx *context.Context) error {
message, err := tmpl.New(ctx).Apply(ctx.Config.Announce.LinkedIn.MessageTemplate)
if err != nil {
return fmt.Errorf("failed to announce to linkedin: %w", err)
return fmt.Errorf("linkedin: %w", err)
}

var cfg Config
if err := env.Parse(&cfg); err != nil {
return fmt.Errorf("failed to announce to linkedin: %w", err)
return fmt.Errorf("linkedin: %w", err)
}

c, err := createLinkedInClient(oauthClientConfig{
Context: ctx,
AccessToken: cfg.AccessToken,
})
if err != nil {
return fmt.Errorf("failed to announce to linkedin: %w", err)
return fmt.Errorf("linkedin: %w", err)
}

url, err := c.Share(message)
if err != nil {
return fmt.Errorf("failed to announce to linkedin: %w", err)
return fmt.Errorf("linkedin: %w", err)
}

log.Infof("The text post is available at: %s\n", url)
Expand Down
6 changes: 3 additions & 3 deletions internal/pipe/linkedin/linkedin_test.go
Expand Up @@ -21,7 +21,7 @@ func TestDefault(t *testing.T) {
func TestAnnounceDisabled(t *testing.T) {
ctx := context.New(config.Project{})
require.NoError(t, Pipe{}.Default(ctx))
require.EqualError(t, Pipe{}.Announce(ctx), `failed to announce to linkedin: env: environment variable "LINKEDIN_ACCESS_TOKEN" should not be empty`)
require.EqualError(t, Pipe{}.Announce(ctx), `linkedin: env: environment variable "LINKEDIN_ACCESS_TOKEN" should not be empty`)
}

func TestAnnounceInvalidTemplate(t *testing.T) {
Expand All @@ -33,7 +33,7 @@ func TestAnnounceInvalidTemplate(t *testing.T) {
},
},
})
require.EqualError(t, Pipe{}.Announce(ctx), `failed to announce to linkedin: template: tmpl:1: unexpected "}" in operand`)
require.EqualError(t, Pipe{}.Announce(ctx), `linkedin: template: tmpl:1: unexpected "}" in operand`)
}

func TestAnnounceMissingEnv(t *testing.T) {
Expand All @@ -45,7 +45,7 @@ func TestAnnounceMissingEnv(t *testing.T) {
},
})
require.NoError(t, Pipe{}.Default(ctx))
require.EqualError(t, Pipe{}.Announce(ctx), `failed to announce to linkedin: env: environment variable "LINKEDIN_ACCESS_TOKEN" should not be empty`)
require.EqualError(t, Pipe{}.Announce(ctx), `linkedin: env: environment variable "LINKEDIN_ACCESS_TOKEN" should not be empty`)
}

func TestSkip(t *testing.T) {
Expand Down
6 changes: 3 additions & 3 deletions internal/pipe/mastodon/mastodon.go
Expand Up @@ -36,12 +36,12 @@ func (Pipe) Default(ctx *context.Context) error {
func (Pipe) Announce(ctx *context.Context) error {
msg, err := tmpl.New(ctx).Apply(ctx.Config.Announce.Mastodon.MessageTemplate)
if err != nil {
return fmt.Errorf("announce: failed to announce to mastodon: %w", err)
return fmt.Errorf("mastodon: %w", err)
}

var cfg Config
if err := env.Parse(&cfg); err != nil {
return fmt.Errorf("announce: failed to announce to mastodon: %w", err)
return fmt.Errorf("mastodon: %w", err)
}

client := mastodon.NewClient(&mastodon.Config{
Expand All @@ -55,7 +55,7 @@ func (Pipe) Announce(ctx *context.Context) error {
if _, err := client.PostStatus(ctx, &mastodon.Toot{
Status: msg,
}); err != nil {
return fmt.Errorf("announce: failed to announce to mastodon: %w", err)
return fmt.Errorf("mastodon: %w", err)
}
return nil
}
4 changes: 2 additions & 2 deletions internal/pipe/mastodon/mastodon_test.go
Expand Up @@ -26,7 +26,7 @@ func TestAnnounceInvalidTemplate(t *testing.T) {
},
},
})
require.EqualError(t, Pipe{}.Announce(ctx), `announce: failed to announce to mastodon: template: tmpl:1: unexpected "}" in operand`)
require.EqualError(t, Pipe{}.Announce(ctx), `mastodon: template: tmpl:1: unexpected "}" in operand`)
}

func TestAnnounceMissingEnv(t *testing.T) {
Expand All @@ -36,7 +36,7 @@ func TestAnnounceMissingEnv(t *testing.T) {
},
})
require.NoError(t, Pipe{}.Default(ctx))
require.EqualError(t, Pipe{}.Announce(ctx), `announce: failed to announce to mastodon: env: environment variable "MASTODON_CLIENT_ID" should not be empty; environment variable "MASTODON_CLIENT_SECRET" should not be empty; environment variable "MASTODON_ACCESS_TOKEN" should not be empty`)
require.EqualError(t, Pipe{}.Announce(ctx), `mastodon: env: environment variable "MASTODON_CLIENT_ID" should not be empty; environment variable "MASTODON_CLIENT_SECRET" should not be empty; environment variable "MASTODON_ACCESS_TOKEN" should not be empty`)
}

func TestSkip(t *testing.T) {
Expand Down
10 changes: 5 additions & 5 deletions internal/pipe/mattermost/mattermost.go
Expand Up @@ -51,17 +51,17 @@ func (Pipe) Default(ctx *context.Context) error {
func (Pipe) Announce(ctx *context.Context) error {
msg, err := tmpl.New(ctx).Apply(ctx.Config.Announce.Mattermost.MessageTemplate)
if err != nil {
return fmt.Errorf("announce: failed to announce to mattermost: %w", err)
return fmt.Errorf("mattermost: %w", err)
}

title, err := tmpl.New(ctx).Apply(ctx.Config.Announce.Mattermost.TitleTemplate)
if err != nil {
return fmt.Errorf("announce: failed to announce to teams: %w", err)
return fmt.Errorf("teams: %w", err)
}

var cfg Config
if err := env.Parse(&cfg); err != nil {
return fmt.Errorf("announce: failed to announce to mattermost: %w", err)
return fmt.Errorf("mattermost: %w", err)
}

log.Infof("posting: %q", msg)
Expand All @@ -82,7 +82,7 @@ func (Pipe) Announce(ctx *context.Context) error {

err = postWebhook(ctx, cfg.Webhook, wm)
if err != nil {
return fmt.Errorf("announce: failed to announce to mattermost: %w", err)
return fmt.Errorf("mattermost: %w", err)
}

return nil
Expand All @@ -102,7 +102,7 @@ func postWebhook(ctx *context.Context, url string, msg *incomingWebhookRequest)

r, err := http.DefaultClient.Do(req)
if err != nil {
return fmt.Errorf("announce: failed to announce to mattermost: %w", err)
return fmt.Errorf("mattermost: %w", err)
}
closeBody(r)

Expand Down
4 changes: 2 additions & 2 deletions internal/pipe/mattermost/mattermost_test.go
Expand Up @@ -31,7 +31,7 @@ func TestAnnounceInvalidTemplate(t *testing.T) {
},
},
})
require.EqualError(t, Pipe{}.Announce(ctx), `announce: failed to announce to mattermost: template: tmpl:1: unexpected "}" in operand`)
require.EqualError(t, Pipe{}.Announce(ctx), `mattermost: template: tmpl:1: unexpected "}" in operand`)
}

func TestAnnounceMissingEnv(t *testing.T) {
Expand All @@ -41,7 +41,7 @@ func TestAnnounceMissingEnv(t *testing.T) {
},
})
require.NoError(t, Pipe{}.Default(ctx))
require.EqualError(t, Pipe{}.Announce(ctx), `announce: failed to announce to mattermost: env: environment variable "MATTERMOST_WEBHOOK" should not be empty`)
require.EqualError(t, Pipe{}.Announce(ctx), `mattermost: env: environment variable "MATTERMOST_WEBHOOK" should not be empty`)
}

func TestSkip(t *testing.T) {
Expand Down

0 comments on commit 47ce9a9

Please sign in to comment.