Use modern error handling and enforce it via lint (#1327)

Co-authored-by: Anbraten <anton@ju60.de>

.golangci.yml

@@ -24,6 +24,7 @@ linters:
     - unused
     - whitespace
     - gofumpt
+    - errorlint
 
 run:
   timeout: 5m

.vscode/settings.json

@@ -14,5 +14,9 @@
       "./web"
   ],
   "prettier.configPath": "./web/.prettierrc.js",
-  "prettier.ignorePath": "./web/.prettierignore"
+  "prettier.ignorePath": "./web/.prettierignore",
+  "cSpell.words": [
+    "doublestar",
+    "multierr"
+  ]
 }

go.mod

@@ -41,6 +41,7 @@ require (
 	github.com/urfave/cli/v2 v2.20.2
 	github.com/xanzy/go-gitlab v0.73.1
 	github.com/xeipuuv/gojsonschema v1.2.0
+	go.uber.org/multierr v1.9.0
 	golang.org/x/crypto v0.1.0
 	golang.org/x/exp v0.0.0-20221031165847-c99f073a8326
 	golang.org/x/net v0.4.0
@@ -127,7 +128,6 @@ require (
 	github.com/xeipuuv/gojsonreference v0.0.0-20180127040603-bd5ef7bd5415 // indirect
 	github.com/xrash/smetrics v0.0.0-20201216005158-039620a65673 // indirect
 	go.uber.org/atomic v1.10.0 // indirect
-	go.uber.org/multierr v1.8.0 // indirect
 	go.uber.org/zap v1.23.0 // indirect
 	golang.org/x/mod v0.6.0 // indirect
 	golang.org/x/sys v0.3.0 // indirect

go.sum

@@ -739,8 +739,8 @@ go.uber.org/multierr v1.1.0/go.mod h1:wR5kodmAFQ0UK8QlbwjlSNy0Z68gJhDJUG5sjR94q/
 go.uber.org/multierr v1.3.0/go.mod h1:VgVr7evmIr6uPjLBxg28wmKNXyqE9akIJ5XnfpiKl+4=
 go.uber.org/multierr v1.5.0/go.mod h1:FeouvMocqHpRaaGuG9EjoKcStLC43Zu/fmqdUMPcKYU=
 go.uber.org/multierr v1.6.0/go.mod h1:cdWPpRnG4AhwMwsgIHip0KRBQjJy5kYEpYjJxpXp9iU=
-go.uber.org/multierr v1.8.0 h1:dg6GjLku4EH+249NNmoIciG9N/jURbDG+pFlTkhzIC8=
+go.uber.org/multierr v1.9.0 h1:7fIwc/ZtS0q++VgcfqFDxSBZVv/Xo49/SYnDFupUwlI=
-go.uber.org/multierr v1.8.0/go.mod h1:7EAYxJLBy9rStEaz58O2t4Uvip6FSURkq8/ppBp95ak=
+go.uber.org/multierr v1.9.0/go.mod h1:X2jQV1h+kxSjClGpnseKVIxpmcjrj7MNnI0bnlfKTVQ=
 go.uber.org/tools v0.0.0-20190618225709-2cfd321de3ee/go.mod h1:vJERXedbb3MVM5f9Ejo0C68/HhF8uaILCdgjnY+goOA=
 go.uber.org/zap v1.9.1/go.mod h1:vwi/ZaCAaUcBkycHslxD9B2zi4UTXhF60s6SWpuDF0Q=
 go.uber.org/zap v1.10.0/go.mod h1:vwi/ZaCAaUcBkycHslxD9B2zi4UTXhF60s6SWpuDF0Q=

pipeline/backend/local/local.go

@@ -16,6 +16,7 @@ package local
 
 import (
 	"context"
+	"errors"
 	"fmt"
 	"io"
 	"os"
@@ -133,11 +134,14 @@ func (e *local) Exec(ctx context.Context, step *types.Step) error {
 func (e *local) Wait(context.Context, *types.Step) (*types.State, error) {
 	err := e.cmd.Wait()
 	ExitCode := 0
-	if eerr, ok := err.(*exec.ExitError); ok {
+
-		ExitCode = eerr.ExitCode()
+	var execExitError *exec.ExitError
+	if errors.As(err, &execExitError) {
+		ExitCode = execExitError.ExitCode()
 		// Non-zero exit code is a pipeline failure, but not an agent error.
 		err = nil
 	}
+
 	return &types.State{
 		Exited:   true,
 		ExitCode: ExitCode,

pipeline/frontend/yaml/compiler/compiler_test.go

@@ -16,11 +16,11 @@ func TestSecretAvailable(t *testing.T) {
 	}
 	assert.True(t, secret.Available(&yaml.Container{
 		Image:    "golang",
-		Commands: types.Stringorslice(strslice.StrSlice{"echo 'this is not a plugin'"}),
+		Commands: types.StringOrSlice(strslice.StrSlice{"echo 'this is not a plugin'"}),
 	}))
 	assert.False(t, secret.Available(&yaml.Container{
 		Image:    "not-golang",
-		Commands: types.Stringorslice(strslice.StrSlice{"echo 'this is not a plugin'"}),
+		Commands: types.StringOrSlice(strslice.StrSlice{"echo 'this is not a plugin'"}),
 	}))
 	// secret only available for "golang" plugin
 	secret = Secret{
@@ -29,14 +29,14 @@ func TestSecretAvailable(t *testing.T) {
 	}
 	assert.True(t, secret.Available(&yaml.Container{
 		Image:    "golang",
-		Commands: types.Stringorslice(strslice.StrSlice{}),
+		Commands: types.StringOrSlice(strslice.StrSlice{}),
 	}))
 	assert.False(t, secret.Available(&yaml.Container{
 		Image:    "not-golang",
-		Commands: types.Stringorslice(strslice.StrSlice{}),
+		Commands: types.StringOrSlice(strslice.StrSlice{}),
 	}))
 	assert.False(t, secret.Available(&yaml.Container{
 		Image:    "not-golang",
-		Commands: types.Stringorslice(strslice.StrSlice{"echo 'this is not a plugin'"}),
+		Commands: types.StringOrSlice(strslice.StrSlice{"echo 'this is not a plugin'"}),
 	}))
 }

pipeline/frontend/yaml/config.go

@@ -11,7 +11,7 @@ type (
 	// Config defines a pipeline configuration.
 	Config struct {
 		When      constraint.When `yaml:"when,omitempty"`
-		Cache     types.Stringorslice
+		Cache     types.StringOrSlice
 		Platform  string
 		Workspace Workspace
 		Clone     Containers

pipeline/frontend/yaml/config_test.go

@@ -32,10 +32,10 @@ func TestParse(t *testing.T) {
 				g.Assert(out.Services.Containers[0].Image).Equal("mysql")
 				g.Assert(out.Pipeline.Containers[0].Name).Equal("test")
 				g.Assert(out.Pipeline.Containers[0].Image).Equal("golang")
-				g.Assert(out.Pipeline.Containers[0].Commands).Equal(types.Stringorslice{"go install", "go test"})
+				g.Assert(out.Pipeline.Containers[0].Commands).Equal(types.StringOrSlice{"go install", "go test"})
 				g.Assert(out.Pipeline.Containers[1].Name).Equal("build")
 				g.Assert(out.Pipeline.Containers[1].Image).Equal("golang")
-				g.Assert(out.Pipeline.Containers[1].Commands).Equal(types.Stringorslice{"go build"})
+				g.Assert(out.Pipeline.Containers[1].Commands).Equal(types.StringOrSlice{"go build"})
 				g.Assert(out.Pipeline.Containers[2].Name).Equal("notify")
 				g.Assert(out.Pipeline.Containers[2].Image).Equal("slack")
 				// g.Assert(out.Pipeline.Containers[2].NetworkMode).Equal("container:name")

pipeline/frontend/yaml/constraint/constraint.go

@@ -6,6 +6,7 @@ import (
 
 	"github.com/antonmedv/expr"
 	"github.com/bmatcuk/doublestar/v4"
+	"go.uber.org/multierr"
 	"gopkg.in/yaml.v3"
 
 	"github.com/woodpecker-ci/woodpecker/pipeline/frontend"
@@ -238,11 +239,11 @@ func (c *List) Excludes(v string) bool {
 // UnmarshalYAML unmarshals the constraint.
 func (c *List) UnmarshalYAML(value *yaml.Node) error {
 	out1 := struct {
-		Include types.Stringorslice
+		Include types.StringOrSlice
-		Exclude types.Stringorslice
+		Exclude types.StringOrSlice
 	}{}
 
-	var out2 types.Stringorslice
+	var out2 types.StringOrSlice
 
 	err1 := value.Decode(&out1)
 	err2 := value.Decode(&out2)
@@ -255,7 +256,7 @@ func (c *List) UnmarshalYAML(value *yaml.Node) error {
 
 	if err1 != nil && err2 != nil {
 		y, _ := yaml.Marshal(value)
-		return fmt.Errorf("Could not parse condition: %s", y)
+		return fmt.Errorf("Could not parse condition: %s: %w", y, multierr.Combine(err1, err2))
 	}
 
 	return nil
@@ -291,7 +292,7 @@ func (c *Map) Match(params map[string]string) bool {
 	return true
 }
 
-// UnmarshalYAML unmarshals the constraint map.
+// UnmarshalYAML unmarshal the constraint map.
 func (c *Map) UnmarshalYAML(unmarshal func(interface{}) error) error {
 	out1 := struct {
 		Include map[string]string
@@ -314,15 +315,15 @@ func (c *Map) UnmarshalYAML(unmarshal func(interface{}) error) error {
 	return nil
 }
 
-// UnmarshalYAML unmarshals the constraint.
+// UnmarshalYAML unmarshal the constraint.
 func (c *Path) UnmarshalYAML(value *yaml.Node) error {
 	out1 := struct {
-		Include       types.Stringorslice `yaml:"include,omitempty"`
+		Include       types.StringOrSlice `yaml:"include,omitempty"`
-		Exclude       types.Stringorslice `yaml:"exclude,omitempty"`
+		Exclude       types.StringOrSlice `yaml:"exclude,omitempty"`
 		IgnoreMessage string              `yaml:"ignore_message,omitempty"`
 	}{}
 
-	var out2 types.Stringorslice
+	var out2 types.StringOrSlice
 
 	err1 := value.Decode(&out1)
 	err2 := value.Decode(&out2)

pipeline/frontend/yaml/container.go

@@ -27,15 +27,15 @@ type (
 		AuthConfig    AuthConfig             `yaml:"auth_config,omitempty"`
 		CapAdd        []string               `yaml:"cap_add,omitempty"`
 		CapDrop       []string               `yaml:"cap_drop,omitempty"`
-		Commands      types.Stringorslice    `yaml:"commands,omitempty"`
+		Commands      types.StringOrSlice    `yaml:"commands,omitempty"`
 		CPUQuota      types.StringorInt      `yaml:"cpu_quota,omitempty"`
 		CPUSet        string                 `yaml:"cpuset,omitempty"`
 		CPUShares     types.StringorInt      `yaml:"cpu_shares,omitempty"`
 		Detached      bool                   `yaml:"detach,omitempty"`
 		Devices       []string               `yaml:"devices,omitempty"`
 		Tmpfs         []string               `yaml:"tmpfs,omitempty"`
-		DNS           types.Stringorslice    `yaml:"dns,omitempty"`
+		DNS           types.StringOrSlice    `yaml:"dns,omitempty"`
-		DNSSearch     types.Stringorslice    `yaml:"dns_search,omitempty"`
+		DNSSearch     types.StringOrSlice    `yaml:"dns_search,omitempty"`
 		Directory     string                 `yaml:"directory,omitempty"`
 		Environment   types.SliceorMap       `yaml:"environment,omitempty"`
 		ExtraHosts    []string               `yaml:"extra_hosts,omitempty"`

pipeline/frontend/yaml/container_test.go

@@ -74,15 +74,15 @@ func TestUnmarshalContainer(t *testing.T) {
 		},
 		CapAdd:        []string{"ALL"},
 		CapDrop:       []string{"NET_ADMIN", "SYS_ADMIN"},
-		Commands:      types.Stringorslice{"go build", "go test"},
+		Commands:      types.StringOrSlice{"go build", "go test"},
 		CPUQuota:      types.StringorInt(11),
 		CPUSet:        "1,2",
 		CPUShares:     types.StringorInt(99),
 		Detached:      true,
 		Devices:       []string{"/dev/ttyUSB0:/dev/ttyUSB0"},
 		Directory:     "example/",
-		DNS:           types.Stringorslice{"8.8.8.8"},
+		DNS:           types.StringOrSlice{"8.8.8.8"},
-		DNSSearch:     types.Stringorslice{"example.com"},
+		DNSSearch:     types.StringOrSlice{"example.com"},
 		Environment:   types.SliceorMap{"RACK_ENV": "development", "SHOW": "true"},
 		ExtraHosts:    []string{"somehost:162.242.195.82", "otherhost:50.31.209.229"},
 		Image:         "golang:latest",
@@ -102,7 +102,7 @@ func TestUnmarshalContainer(t *testing.T) {
 		Pull:        true,
 		Privileged:  true,
 		ShmSize:     types.MemStringorInt(1024),
-		Tmpfs:       types.Stringorslice{"/var/lib/test"},
+		Tmpfs:       types.StringOrSlice{"/var/lib/test"},
 		Volumes: types.Volumes{
 			Volumes: []*types.Volume{
 				{Source: "", Destination: "/var/lib/mysql"},
@@ -302,9 +302,9 @@ func stringsToInterface(val ...string) []interface{} {
 func TestIsPlugin(t *testing.T) {
 	assert.True(t, (&Container{}).IsPlugin())
 	assert.True(t, (&Container{
-		Commands: types.Stringorslice(strslice.StrSlice{}),
+		Commands: types.StringOrSlice(strslice.StrSlice{}),
 	}).IsPlugin())
 	assert.False(t, (&Container{
-		Commands: types.Stringorslice(strslice.StrSlice{"echo 'this is not a plugin'"}),
+		Commands: types.StringOrSlice(strslice.StrSlice{"echo 'this is not a plugin'"}),
 	}).IsPlugin())
 }

pipeline/frontend/yaml/error.go

@@ -14,6 +14,8 @@
 
 package yaml
 
+import "errors"
+
 // PipelineParseError is an error that occurs when the pipeline parsing fails.
 type PipelineParseError struct {
 	Err error
@@ -23,8 +25,8 @@ func (e PipelineParseError) Error() string {
 	return e.Err.Error()
 }
 
-func (e PipelineParseError) Is(target error) bool {
+func (e PipelineParseError) Is(err error) bool {
-	_, ok1 := target.(PipelineParseError)
+	target1 := PipelineParseError{}
-	_, ok2 := target.(*PipelineParseError)
+	target2 := &target1
-	return ok1 || ok2
+	return errors.As(err, &target1) || errors.As(err, &target2)
 }

pipeline/frontend/yaml/types/types_yaml.go

@@ -59,12 +59,12 @@ func (s *MemStringorInt) UnmarshalYAML(unmarshal func(interface{}) error) error
 	return errors.New("Failed to unmarshal MemStringorInt")
 }
 
-// Stringorslice represents
+// StringOrSlice represents
 // Using engine-api Strslice and augment it with YAML marshaling stuff. a string or an array of strings.
-type Stringorslice strslice.StrSlice
+type StringOrSlice strslice.StrSlice
 
 // UnmarshalYAML implements the Unmarshaler interface.
-func (s *Stringorslice) UnmarshalYAML(unmarshal func(interface{}) error) error {
+func (s *StringOrSlice) UnmarshalYAML(unmarshal func(interface{}) error) error {
 	var stringType string
 	if err := unmarshal(&stringType); err == nil {
 		*s = []string{stringType}
@@ -81,7 +81,7 @@ func (s *Stringorslice) UnmarshalYAML(unmarshal func(interface{}) error) error {
 		return nil
 	}
 
-	return errors.New("Failed to unmarshal Stringorslice")
+	return errors.New("Failed to unmarshal StringOrSlice")
 }
 
 // SliceorMap represents a slice or a map of strings.

pipeline/frontend/yaml/types/types_yaml_test.go

@@ -30,25 +30,25 @@ func TestStringorIntYaml(t *testing.T) {
 	}
 }
 
-type StructStringorslice struct {
+type StructStringOrSlice struct {
-	Foo Stringorslice
+	Foo StringOrSlice
 }
 
-func TestStringorsliceYaml(t *testing.T) {
+func TestStringOrSliceYaml(t *testing.T) {
 	str := `{foo: [bar, baz]}`
 
-	s := StructStringorslice{}
+	s := StructStringOrSlice{}
 	assert.NoError(t, yaml.Unmarshal([]byte(str), &s))
 
-	assert.Equal(t, Stringorslice{"bar", "baz"}, s.Foo)
+	assert.Equal(t, StringOrSlice{"bar", "baz"}, s.Foo)
 
 	d, err := yaml.Marshal(&s)
 	assert.Nil(t, err)
 
-	s2 := StructStringorslice{}
+	s2 := StructStringOrSlice{}
 	assert.NoError(t, yaml.Unmarshal(d, &s2))
 
-	assert.Equal(t, Stringorslice{"bar", "baz"}, s2.Foo)
+	assert.Equal(t, StringOrSlice{"bar", "baz"}, s2.Foo)
 }
 
 type StructSliceorMap struct {

server/api/helper.go

@@ -15,6 +15,7 @@
 package api
 
 import (
+	"errors"
 	"net/http"
 
 	"github.com/gin-gonic/gin"
@@ -28,12 +29,12 @@ import (
 )
 
 func handlePipelineErr(c *gin.Context, err error) {
-	if pipeline.IsErrNotFound(err) {
+	if errors.Is(err, &pipeline.ErrNotFound{}) {
-		c.String(http.StatusNotFound, "%v", err)
+		c.String(http.StatusNotFound, "%s", err)
-	} else if pipeline.IsErrBadRequest(err) {
+	} else if errors.Is(err, &pipeline.ErrBadRequest{}) {
-		c.String(http.StatusBadRequest, "%v", err)
+		c.String(http.StatusBadRequest, "%s", err)
-	} else if pipeline.IsErrFiltered(err) {
+	} else if errors.Is(err, &pipeline.ErrFiltered{}) {
-		c.String(http.StatusNoContent, "%v", err)
+		c.String(http.StatusNoContent, "%s", err)
 	} else {
 		_ = c.AbortWithError(http.StatusInternalServerError, err)
 	}

server/cron/cron.go

@@ -72,7 +72,7 @@ func CalcNewNext(schedule string, now time.Time) (time.Time, error) {
 
 	c, err := cron.Parse(schedule)
 	if err != nil {
-		return time.Time{}, fmt.Errorf("cron parse schedule: %v", err)
+		return time.Time{}, fmt.Errorf("cron parse schedule: %w", err)
 	}
 	return c.Next(now), nil
 }

server/forge/coding/internal/coding.go

@@ -72,13 +72,13 @@ func (c *Client) Do(method, u string, params url.Values) ([]byte, error) {
 		req, err = http.NewRequestWithContext(c.ctx, "GET", rawURL+"?"+params.Encode(), nil)
 	}
 	if err != nil {
-		return nil, fmt.Errorf("fail to create request for url %q: %v", rawURL, err)
+		return nil, fmt.Errorf("fail to create request for url %q: %w", rawURL, err)
 	}
 	req.Header.Set("User-Agent", c.agent)
 
 	resp, err := c.client.Do(req)
 	if err != nil {
-		return nil, fmt.Errorf("fail to request %s %s: %v", req.Method, req.URL, err)
+		return nil, fmt.Errorf("fail to request %s %s: %w", req.Method, req.URL, err)
 	}
 	if resp.StatusCode != http.StatusOK {
 		return nil, fmt.Errorf("%s %s respond %d", req.Method, req.URL, resp.StatusCode)
@@ -86,13 +86,13 @@ func (c *Client) Do(method, u string, params url.Values) ([]byte, error) {
 	defer resp.Body.Close()
 	body, err := io.ReadAll(resp.Body)
 	if err != nil {
-		return nil, fmt.Errorf("fail to read response from %s %s: %v", req.Method, req.URL.String(), err)
+		return nil, fmt.Errorf("fail to read response from %s %s: %w", req.Method, req.URL.String(), err)
 	}
 
 	apiResp := &GenericAPIResponse{}
 	err = json.Unmarshal(body, apiResp)
 	if err != nil {
-		return nil, fmt.Errorf("fail to parse response from %s %s: %v", req.Method, req.URL.String(), err)
+		return nil, fmt.Errorf("fail to parse response from %s %s: %w", req.Method, req.URL.String(), err)
 	}
 	if apiResp.Code != 0 {
 		return nil, fmt.Errorf("Coding OAuth API respond error: %s", string(body))

server/forge/configFetcher.go

@@ -75,7 +75,7 @@ func (cf *configFetcher) Fetch(ctx context.Context) (files []*types.FileMeta, er
 			newConfigs, useOld, err := cf.configExtension.FetchConfig(fetchCtx, cf.repo, cf.pipeline, files)
 			if err != nil {
 				log.Error().Msg("Got error " + err.Error())
-				return nil, fmt.Errorf("On Fetching config via http : %s", err)
+				return nil, fmt.Errorf("On Fetching config via http : %w", err)
 			}
 
 			if !useOld {
@@ -104,7 +104,7 @@ func (cf *configFetcher) fetch(c context.Context, timeout time.Duration, config
 			return fileMeta, err
 		}
 
-		return nil, fmt.Errorf("user defined config '%s' not found: %s", config, err)
+		return nil, fmt.Errorf("user defined config '%s' not found: %w", config, err)
 	}
 
 	log.Trace().Msgf("ConfigFetch[%s]: user did not defined own config, following default procedure", cf.repo.FullName)
@@ -118,7 +118,7 @@ func (cf *configFetcher) fetch(c context.Context, timeout time.Duration, config
 	case <-ctx.Done():
 		return nil, ctx.Err()
 	default:
-		return []*types.FileMeta{}, fmt.Errorf("ConfigFetcher: Fallback did not find config: %s", err)
+		return []*types.FileMeta{}, fmt.Errorf("ConfigFetcher: Fallback did not find config: %w", err)
 	}
 }
 

server/forge/gitea/gitea.go

@@ -314,7 +314,7 @@ func (c *Gitea) Dir(ctx context.Context, u *model.User, r *model.Repo, b *model.
 		if m, _ := filepath.Match(f, e.Path); m && e.Type == "blob" {
 			data, err := c.File(ctx, u, r, b, e.Path)
 			if err != nil {
-				return nil, fmt.Errorf("multi-pipeline cannot get %s: %s", e.Path, err)
+				return nil, fmt.Errorf("multi-pipeline cannot get %s: %w", e.Path, err)
 			}
 
 			configs = append(configs, &forge_types.FileMeta{

server/forge/gitlab/gitlab.go

@@ -119,7 +119,7 @@ func (g *GitLab) Login(ctx context.Context, res http.ResponseWriter, req *http.R
 
 	token, err := config.Exchange(oauth2Ctx, code)
 	if err != nil {
-		return nil, fmt.Errorf("Error exchanging token. %s", err)
+		return nil, fmt.Errorf("Error exchanging token. %w", err)
 	}
 
 	client, err := newClient(g.URL, token.AccessToken, g.SkipVerify)

server/forge/userSyncer.go

@@ -86,7 +86,7 @@ func (s *Syncer) Sync(ctx context.Context, user *model.User, flatPermissions boo
 			} else {
 				forgePerm, err := s.Forge.Perm(ctx, user, repo)
 				if err != nil {
-					return fmt.Errorf("could not fetch permission of repo '%s': %v", repo.FullName, err)
+					return fmt.Errorf("could not fetch permission of repo '%s': %w", repo.FullName, err)
 				}
 				repo.Perm.Pull = forgePerm.Pull
 				repo.Perm.Push = forgePerm.Push

server/model/cron.go

@@ -49,7 +49,7 @@ func (c *Cron) Validate() error {
 
 	_, err := cron.Parse(c.Schedule)
 	if err != nil {
-		return fmt.Errorf("can't parse schedule: %v", err)
+		return fmt.Errorf("can't parse schedule: %w", err)
 	}
 
 	return nil

server/model/secret.go

@@ -24,9 +24,10 @@ import (
 )
 
 var (
-	errSecretNameInvalid  = errors.New("Invalid Secret Name")
+	ErrSecretNameInvalid  = errors.New("Invalid Secret Name")
-	errSecretValueInvalid = errors.New("Invalid Secret Value")
+	ErrSecretImageInvalid = errors.New("Invalid Secret Image")
-	errSecretEventInvalid = errors.New("Invalid Secret Event")
+	ErrSecretValueInvalid = errors.New("Invalid Secret Value")
+	ErrSecretEventInvalid = errors.New("Invalid Secret Event")
 )
 
 // SecretService defines a service for managing secrets.
@@ -125,27 +126,27 @@ var validDockerImageString = regexp.MustCompile(
 func (s *Secret) Validate() error {
 	for _, event := range s.Events {
 		if !ValidateWebhookEvent(event) {
-			return fmt.Errorf("%s: '%s'", errSecretEventInvalid, event)
+			return fmt.Errorf("%w: '%s'", ErrSecretEventInvalid, event)
 		}
 	}
 	if len(s.Events) == 0 {
-		return fmt.Errorf("%s: no event specified", errSecretEventInvalid)
+		return fmt.Errorf("%w: no event specified", ErrSecretEventInvalid)
 	}
 
 	for _, image := range s.Images {
 		if len(image) == 0 {
-			return fmt.Errorf("empty image in images")
+			return fmt.Errorf("%w: empty image in images", ErrSecretImageInvalid)
 		}
 		if !validDockerImageString.MatchString(image) {
-			return fmt.Errorf("image '%s' do not match regexp '%s'", image, validDockerImageString.String())
+			return fmt.Errorf("%w: image '%s' do not match regexp '%s'", ErrSecretImageInvalid, image, validDockerImageString.String())
 		}
 	}
 
 	switch {
 	case len(s.Name) == 0:
-		return errSecretNameInvalid
+		return fmt.Errorf("%w: empty name", ErrSecretNameInvalid)
 	case len(s.Value) == 0:
-		return errSecretValueInvalid
+		return fmt.Errorf("%w: empty value", ErrSecretValueInvalid)
 	default:
 		return nil
 	}

server/model/user_test.go

@@ -14,7 +14,10 @@
 
 package model
 
-import "testing"
+import (
+	"errors"
+	"testing"
+)
 
 func TestUserValidate(t *testing.T) {
 	tests := []struct {
@@ -57,8 +60,8 @@ func TestUserValidate(t *testing.T) {
 
 	for _, test := range tests {
 		err := test.user.Validate()
-		if want, got := test.err, err; want != got {
+		if !errors.Is(err, test.err) {
-			t.Errorf("Want user validation error %s, got %s", want, got)
+			t.Errorf("Want user validation error %s, got %s", test.err, err)
 		}
 	}
 }

server/pipeline/approve.go

@@ -41,7 +41,7 @@ func Approve(ctx context.Context, store store.Store, currentPipeline *model.Pipe
 	}
 
 	if currentPipeline, err = UpdateToStatusPending(store, *currentPipeline, user.Login); err != nil {
-		return nil, fmt.Errorf("error updating pipeline. %s", err)
+		return nil, fmt.Errorf("error updating pipeline. %w", err)
 	}
 
 	var yamls []*forge_types.FileMeta

server/pipeline/cancel.go

@@ -29,12 +29,12 @@ import (
 // Cancel the pipeline and returns the status.
 func Cancel(ctx context.Context, store store.Store, repo *model.Repo, pipeline *model.Pipeline) error {
 	if pipeline.Status != model.StatusRunning && pipeline.Status != model.StatusPending && pipeline.Status != model.StatusBlocked {
-		return ErrBadRequest{Msg: "Cannot cancel a non-running or non-pending or non-blocked pipeline"}
+		return &ErrBadRequest{Msg: "Cannot cancel a non-running or non-pending or non-blocked pipeline"}
 	}
 
 	steps, err := store.StepList(pipeline)
 	if err != nil {
-		return ErrNotFound{Msg: err.Error()}
+		return &ErrNotFound{Msg: err.Error()}
 	}
 
 	// First cancel/evict steps in the queue in one go
@@ -92,7 +92,7 @@ func Cancel(ctx context.Context, store store.Store, repo *model.Repo, pipeline *
 
 	steps, err = store.StepList(killedBuild)
 	if err != nil {
-		return ErrNotFound{Msg: err.Error()}
+		return &ErrNotFound{Msg: err.Error()}
 	}
 	if killedBuild.Steps, err = model.Tree(steps); err != nil {
 		return err

server/pipeline/decline.go

@@ -31,7 +31,7 @@ func Decline(ctx context.Context, store store.Store, pipeline *model.Pipeline, u
 
 	_, err := UpdateToStatusDeclined(store, *pipeline, user.Login)
 	if err != nil {
-		return nil, fmt.Errorf("error updating pipeline. %s", err)
+		return nil, fmt.Errorf("error updating pipeline. %w", err)
 	}
 
 	if pipeline.Steps, err = store.StepList(pipeline); err != nil {

server/pipeline/errors.go

@@ -22,11 +22,6 @@ func (e ErrNotFound) Error() string {
 	return e.Msg
 }
 
-func IsErrNotFound(err error) bool {
-	_, ok := err.(ErrNotFound)
-	return ok
-}
-
 type ErrBadRequest struct {
 	Msg string
 }
@@ -35,11 +30,6 @@ func (e ErrBadRequest) Error() string {
 	return e.Msg
 }
 
-func IsErrBadRequest(err error) bool {
-	_, ok := err.(ErrBadRequest)
-	return ok
-}
-
 type ErrFiltered struct {
 	Msg string
 }
@@ -47,8 +37,3 @@ type ErrFiltered struct {
 func (e ErrFiltered) Error() string {
 	return "ignoring hook: " + e.Msg
 }
-
-func IsErrFiltered(err error) bool {
-	_, ok := err.(ErrFiltered)
-	return ok
-}

server/pipeline/restart.go

@@ -34,7 +34,7 @@ func Restart(ctx context.Context, store store.Store, lastBuild *model.Pipeline,
 	switch lastBuild.Status {
 	case model.StatusDeclined,
 		model.StatusBlocked:
-		return nil, ErrBadRequest{Msg: fmt.Sprintf("cannot restart a pipeline with status %s", lastBuild.Status)}
+		return nil, &ErrBadRequest{Msg: fmt.Sprintf("cannot restart a pipeline with status %s", lastBuild.Status)}
 	}
 
 	var pipelineFiles []*forge_types.FileMeta
@@ -44,7 +44,7 @@ func Restart(ctx context.Context, store store.Store, lastBuild *model.Pipeline,
 	if err != nil {
 		msg := fmt.Sprintf("failure to get pipeline config for %s. %s", repo.FullName, err)
 		log.Error().Msgf(msg)
-		return nil, ErrNotFound{Msg: msg}
+		return nil, &ErrNotFound{Msg: msg}
 	}
 
 	for _, y := range configs {
@@ -60,7 +60,7 @@ func Restart(ctx context.Context, store store.Store, lastBuild *model.Pipeline,
 
 		newConfig, useOld, err := server.Config.Services.ConfigService.FetchConfig(ctx, repo, lastBuild, currentFileMeta)
 		if err != nil {
-			return nil, ErrBadRequest{
+			return nil, &ErrBadRequest{
 				Msg: fmt.Sprintf("On fetching external pipeline config: %s", err),
 			}
 		}

server/plugins/encryption/aes_builder.go

@@ -15,6 +15,7 @@
 package encryption
 
 import (
+	"errors"
 	"fmt"
 
 	"github.com/urfave/cli/v2"
@@ -56,7 +57,7 @@ func (c aesConfiguration) Build() (model.EncryptionService, error) {
 	}
 
 	err = svc.validateKey()
-	if err == errEncryptionNotEnabled {
+	if errors.Is(err, errEncryptionNotEnabled) {
 		err = svc.enable()
 	}
 	if err != nil {

server/plugins/encryption/tink_builder.go

@@ -15,6 +15,7 @@
 package encryption
 
 import (
+	"errors"
 	"fmt"
 
 	"github.com/urfave/cli/v2"
@@ -48,20 +49,19 @@ func (c tinkConfiguration) Build() (model.EncryptionService, error) {
 		keysetFileWatcher: nil,
 		clients:           c.clients,
 	}
-	err := svc.initClients()
+
-	if err != nil {
+	if err := svc.initClients(); err != nil {
 		return nil, fmt.Errorf(errTemplateFailedInitializingClients, err)
 	}
 
-	err = svc.loadKeyset()
+	if err := svc.loadKeyset(); err != nil {
-	if err != nil {
 		return nil, fmt.Errorf(errTemplateTinkFailedLoadingKeyset, err)
 	}
 
-	err = svc.validateKeyset()
+	err := svc.validateKeyset()
-	if err == errEncryptionNotEnabled {
+	if errors.Is(err, errEncryptionNotEnabled) {
 		err = svc.enable()
-	} else if err == errEncryptionKeyRotated {
+	} else if errors.Is(err, errEncryptionKeyRotated) {
 		err = svc.rotate()
 	}
 
@@ -69,8 +69,7 @@ func (c tinkConfiguration) Build() (model.EncryptionService, error) {
 		return nil, fmt.Errorf(errTemplateTinkFailedValidatingKeyset, err)
 	}
 
-	err = svc.initFileWatcher()
+	if err := svc.initFileWatcher(); err != nil {
-	if err != nil {
 		return nil, fmt.Errorf(errTemplateTinkFailedInitializeFileWatcher, err)
 	}
 	return svc, nil

server/plugins/encryption/tink_state.go

@@ -15,33 +15,34 @@
 package encryption
 
 import (
+	"errors"
 	"fmt"
 
 	"github.com/rs/zerolog/log"
 )
 
 func (svc *tinkEncryptionService) enable() error {
-	err := svc.callbackOnEnable()
+	if err := svc.callbackOnEnable(); err != nil {
-	if err != nil {
 		return fmt.Errorf(errTemplateFailedEnablingEncryption, err)
 	}
-	err = svc.updateCiphertextSample()
+
-	if err != nil {
+	if err := svc.updateCiphertextSample(); err != nil {
 		return fmt.Errorf(errTemplateFailedEnablingEncryption, err)
 	}
+
 	log.Warn().Msg(logMessageEncryptionEnabled)
 	return nil
 }
 
 func (svc *tinkEncryptionService) disable() error {
-	err := svc.callbackOnDisable()
+	if err := svc.callbackOnDisable(); err != nil {
-	if err != nil {
 		return fmt.Errorf(errTemplateFailedDisablingEncryption, err)
 	}
-	err = svc.deleteCiphertextSample()
+
-	if err != nil {
+	if err := svc.deleteCiphertextSample(); err != nil {
 		return fmt.Errorf(errTemplateFailedDisablingEncryption, err)
 	}
+
 	log.Warn().Msg(logMessageEncryptionDisabled)
 	return nil
 }
@@ -55,26 +56,24 @@ func (svc *tinkEncryptionService) rotate() error {
 		keysetFileWatcher: nil,
 		clients:           svc.clients,
 	}
-	err := newSvc.loadKeyset()
+
-	if err != nil {
+	if err := newSvc.loadKeyset(); err != nil {
 		return fmt.Errorf(errTemplateFailedRotatingEncryption, err)
 	}
 
-	err = newSvc.validateKeyset()
+	err := newSvc.validateKeyset()
-	if err == errEncryptionKeyRotated {
+	if errors.Is(err, errEncryptionKeyRotated) {
 		err = newSvc.updateCiphertextSample()
 	}
 	if err != nil {
 		return fmt.Errorf(errTemplateFailedRotatingEncryption, err)
 	}
 
-	err = newSvc.callbackOnRotation()
+	if err := newSvc.callbackOnRotation(); err != nil {
-	if err != nil {
 		return fmt.Errorf(errTemplateFailedRotatingEncryption, err)
 	}
 
-	err = newSvc.initFileWatcher()
+	if err := newSvc.initFileWatcher(); err != nil {
-	if err != nil {
 		return fmt.Errorf(errTemplateFailedRotatingEncryption, err)
 	}
 	return nil
@@ -85,26 +84,25 @@ func (svc *tinkEncryptionService) updateCiphertextSample() error {
 	if err != nil {
 		return fmt.Errorf(errTemplateFailedUpdatingServerConfig, err)
 	}
-	err = svc.store.ServerConfigSet(ciphertextSampleConfigKey, ciphertext)
+
-	if err != nil {
+	if err := svc.store.ServerConfigSet(ciphertextSampleConfigKey, ciphertext); err != nil {
 		return fmt.Errorf(errTemplateFailedUpdatingServerConfig, err)
 	}
+
 	log.Info().Msg(logMessageEncryptionKeyRegistered)
 	return nil
 }
 
 func (svc *tinkEncryptionService) deleteCiphertextSample() error {
-	err := svc.store.ServerConfigDelete(ciphertextSampleConfigKey)
+	if err := svc.store.ServerConfigDelete(ciphertextSampleConfigKey); err != nil {
-	if err != nil {
+		return fmt.Errorf(errTemplateFailedUpdatingServerConfig, err)
-		err = fmt.Errorf(errTemplateFailedUpdatingServerConfig, err)
 	}
-	return err
+	return nil
 }
 
 func (svc *tinkEncryptionService) initClients() error {
 	for _, client := range svc.clients {
-		err := client.SetEncryptionService(svc)
+		if err := client.SetEncryptionService(svc); err != nil {
-		if err != nil {
 			return err
 		}
 	}
@@ -114,8 +112,7 @@ func (svc *tinkEncryptionService) initClients() error {
 
 func (svc *tinkEncryptionService) callbackOnEnable() error {
 	for _, client := range svc.clients {
-		err := client.EnableEncryption()
+		if err := client.EnableEncryption(); err != nil {
-		if err != nil {
 			return err
 		}
 	}
@@ -125,8 +122,7 @@ func (svc *tinkEncryptionService) callbackOnEnable() error {
 
 func (svc *tinkEncryptionService) callbackOnRotation() error {
 	for _, client := range svc.clients {
-		err := client.MigrateEncryption(svc)
+		if err := client.MigrateEncryption(svc); err != nil {
-		if err != nil {
 			return err
 		}
 	}
@@ -136,8 +132,7 @@ func (svc *tinkEncryptionService) callbackOnRotation() error {
 
 func (svc *tinkEncryptionService) callbackOnDisable() error {
 	for _, client := range svc.clients {
-		err := client.MigrateEncryption(&noEncryption{})
+		if err := client.MigrateEncryption(&noEncryption{}); err != nil {
-		if err != nil {
 			return err
 		}
 	}

server/pubsub/pub_test.go

@@ -2,6 +2,7 @@ package pubsub
 
 import (
 	"context"
+	"errors"
 	"sync"
 	"testing"
 	"time"
@@ -56,7 +57,7 @@ func TestPublishNotFound(t *testing.T) {
 	)
 	broker := New()
 	err := broker.Publish(context.Background(), testTopic, testMessage)
-	if err != ErrNotFound {
+	if !errors.Is(err, ErrNotFound) {
 		t.Errorf("Expect Not Found error when topic does not exist")
 	}
 }
@@ -68,7 +69,7 @@ func TestSubscribeNotFound(t *testing.T) {
 	)
 	broker := New()
 	err := broker.Subscribe(context.Background(), testTopic, testCallback)
-	if err != ErrNotFound {
+	if !errors.Is(err, ErrNotFound) {
 		t.Errorf("Expect Not Found error when topic does not exist")
 	}
 }

server/queue/fifo_test.go

@@ -2,6 +2,7 @@ package queue
 
 import (
 	"context"
+	"errors"
 	"fmt"
 	"sync"
 	"testing"
@@ -116,7 +117,7 @@ func TestFifoEvict(t *testing.T) {
 	if len(info.Pending) != 0 {
 		t.Errorf("expect pending queue has zero items")
 	}
-	if err := q.Evict(noContext, t1.ID); err != ErrNotFound {
+	if err := q.Evict(noContext, t1.ID); !errors.Is(err, ErrNotFound) {
 		t.Errorf("expect not found error when evicting item not in queue, got %s", err)
 	}
 }

server/store/datastore/migration/000_legacy_to_xorm.go

@@ -76,7 +76,7 @@ var legacy2Xorm = task{
 		} {
 			exist, err := sess.Exist(&migrations{mig})
 			if err != nil {
-				return fmt.Errorf("test migration existence: %v", err)
+				return fmt.Errorf("test migration existence: %w", err)
 			}
 			if !exist {
 				log.Error().Msgf("migration step '%s' missing, please upgrade to last stable v0.14.x version first", mig)
@@ -96,10 +96,10 @@ var legacy2Xorm = task{
 				return err
 			}
 			if _, err := sess.Exec("INSERT INTO build_config (config_id, build_id) SELECT config_id,build_id FROM old_build_config;"); err != nil {
-				return fmt.Errorf("unable to set copy data into temp table %s. Error: %v", "old_build_config", err)
+				return fmt.Errorf("unable to set copy data into temp table %s. Error: %w", "old_build_config", err)
 			}
 			if err := sess.DropTable("old_build_config"); err != nil {
-				return fmt.Errorf("could not drop table '%s': %v", "old_build_config", err)
+				return fmt.Errorf("could not drop table '%s': %w", "old_build_config", err)
 			}
 		}
 
@@ -120,7 +120,7 @@ var legacy2Xorm = task{
 				"DROP INDEX IF EXISTS ix_perms_user ON perms;",
 			} {
 				if _, err := sess.Exec(exec); err != nil {
-					return fmt.Errorf("exec: '%s' failed: %v", exec, err)
+					return fmt.Errorf("exec: '%s' failed: %w", exec, err)
 				}
 			}
 		case schemas.SQLITE, schemas.POSTGRES:
@@ -138,7 +138,7 @@ var legacy2Xorm = task{
 				"DROP INDEX IF EXISTS ix_perms_user;",
 			} {
 				if _, err := sess.Exec(exec); err != nil {
-					return fmt.Errorf("exec: '%s' failed: %v", exec, err)
+					return fmt.Errorf("exec: '%s' failed: %w", exec, err)
 				}
 			}
 		default:

server/store/datastore/migration/common.go

@@ -152,7 +152,7 @@ func dropTableColumns(sess *xorm.Session, tableName string, columnNames ...strin
 			cols += "DROP COLUMN `" + col + "` CASCADE"
 		}
 		if _, err := sess.Exec(fmt.Sprintf("ALTER TABLE `%s` %s", tableName, cols)); err != nil {
-			return fmt.Errorf("drop table `%s` columns %v: %v", tableName, columnNames, err)
+			return fmt.Errorf("drop table `%s` columns %v: %w", tableName, columnNames, err)
 		}
 	case schemas.MYSQL:
 		// Drop indexes on columns first
@@ -180,7 +180,7 @@ func dropTableColumns(sess *xorm.Session, tableName string, columnNames ...strin
 			cols += "DROP COLUMN `" + col + "`"
 		}
 		if _, err := sess.Exec(fmt.Sprintf("ALTER TABLE `%s` %s", tableName, cols)); err != nil {
-			return fmt.Errorf("drop table `%s` columns %v: %v", tableName, columnNames, err)
+			return fmt.Errorf("drop table `%s` columns %v: %w", tableName, columnNames, err)
 		}
 	case schemas.MSSQL:
 		cols := ""
@@ -194,27 +194,27 @@ func dropTableColumns(sess *xorm.Session, tableName string, columnNames ...strin
 			tableName, strings.ReplaceAll(cols, "`", "'"))
 		constraints := make([]string, 0)
 		if err := sess.SQL(sql).Find(&constraints); err != nil {
-			return fmt.Errorf("find constraints: %v", err)
+			return fmt.Errorf("find constraints: %w", err)
 		}
 		for _, constraint := range constraints {
 			if _, err := sess.Exec(fmt.Sprintf("ALTER TABLE `%s` DROP CONSTRAINT `%s`", tableName, constraint)); err != nil {
-				return fmt.Errorf("drop table `%s` default constraint `%s`: %v", tableName, constraint, err)
+				return fmt.Errorf("drop table `%s` default constraint `%s`: %w", tableName, constraint, err)
 			}
 		}
 		sql = fmt.Sprintf("SELECT DISTINCT Name FROM sys.indexes INNER JOIN sys.index_columns ON indexes.index_id = index_columns.index_id AND indexes.object_id = index_columns.object_id WHERE indexes.object_id = OBJECT_ID('%[1]s') AND index_columns.column_id IN (SELECT column_id FROM sys.columns WHERE LOWER(name) IN (%[2]s) AND object_id = OBJECT_ID('%[1]s'))",
 			tableName, strings.ReplaceAll(cols, "`", "'"))
 		constraints = make([]string, 0)
 		if err := sess.SQL(sql).Find(&constraints); err != nil {
-			return fmt.Errorf("find constraints: %v", err)
+			return fmt.Errorf("find constraints: %w", err)
 		}
 		for _, constraint := range constraints {
-			if _, err := sess.Exec(fmt.Sprintf("DROP INDEX `%[2]s` ON `%[1]s`", tableName, constraint)); err != nil {
+			if _, err := sess.Exec(fmt.Sprintf("DROP INDEX `%s` ON `%s`", constraint, tableName)); err != nil {
-				return fmt.Errorf("drop index `%[2]s` on `%[1]s`: %v", tableName, constraint, err)
+				return fmt.Errorf("drop index `%s` on `%s`: %w", constraint, tableName, err)
 			}
 		}
 
 		if _, err := sess.Exec(fmt.Sprintf("ALTER TABLE `%s` DROP COLUMN %s", tableName, cols)); err != nil {
-			return fmt.Errorf("drop table `%s` columns %v: %v", tableName, columnNames, err)
+			return fmt.Errorf("drop table `%s` columns %v: %w", tableName, columnNames, err)
 		}
 	default:
 		return fmt.Errorf("dialect '%s' not supported", dialect)

server/store/datastore/migration/migration.go

@@ -175,7 +175,7 @@ type syncEngine interface {
 func syncAll(sess syncEngine) error {
 	for _, bean := range allBeans {
 		if err := sess.Sync2(bean); err != nil {
-			return fmt.Errorf("sync2 error '%s': %v", reflect.TypeOf(bean), err)
+			return fmt.Errorf("sync2 error '%s': %w", reflect.TypeOf(bean), err)
 		}
 	}
 	return nil