Drop error only on purpose or else report back or log (#514)

- Remove Deadcode
- Simplify Code
- Drop error only on purpose

.golangci.yml

@@ -8,7 +8,17 @@ linters:
     - gofmt
     - goimports
     - govet
+    - deadcode
+    - gosimple
+    - typecheck
+    - errcheck
     - bidichk
 
 run:
   timeout: 5m
+
+issues:
+  exclude-rules:
+    - path: woodpecker-go/woodpecker/client.go|server/swagger/swagger.go
+      linters:
+        - deadcode

agent/runner.go

@@ -122,7 +122,9 @@ func (r *Runner) Run(ctx context.Context) error {
 			case <-time.After(time.Minute):
 				logger.Debug().Msg("pipeline lease renewed")
 
-				r.client.Extend(ctx, work.ID)
+				if err := r.client.Extend(ctx, work.ID); err != nil {
+					log.Error().Err(err).Msg("extending pipeline deadline failed")
+				}
 			}
 		}
 	}()
@@ -159,12 +161,14 @@ func (r *Runner) Run(ctx context.Context) error {
 		loglogger.Debug().Msg("log stream opened")
 
 		limitedPart := io.LimitReader(part, maxLogsUpload)
-		logstream := rpc.NewLineWriter(r.client, work.ID, proc.Alias, secrets...)
-		io.Copy(logstream, limitedPart)
+		logStream := rpc.NewLineWriter(r.client, work.ID, proc.Alias, secrets...)
+		if _, err := io.Copy(logStream, limitedPart); err != nil {
+			log.Error().Err(err).Msg("copy limited logStream part")
+		}
 
 		loglogger.Debug().Msg("log stream copied")
 
-		data, err := json.Marshal(logstream.Lines())
+		data, err := json.Marshal(logStream.Lines())
 		if err != nil {
 			loglogger.Err(err).Msg("could not marshal logstream")
 		}

cli/build/build_list.go

@@ -78,7 +78,9 @@ func buildList(c *cli.Context) error {
 		if status != "" && build.Status != status {
 			continue
 		}
-		tmpl.Execute(os.Stdout, build)
+		if err := tmpl.Execute(os.Stdout, build); err != nil {
+			return err
+		}
 		count++
 	}
 	return nil

cli/build/build_queue.go

@@ -44,7 +44,9 @@ func buildQueue(c *cli.Context) error {
 	}
 
 	for _, build := range builds {
-		tmpl.Execute(os.Stdout, build)
+		if err := tmpl.Execute(os.Stdout, build); err != nil {
+			return err
+		}
 	}
 	return nil
 }

cli/exec/exec.go

@@ -260,8 +260,7 @@ var defaultLogger = pipeline.LogFunc(func(proc *backend.Step, rc multipart.Reade
 		return err
 	}
 
-	logstream := NewLineWriter(proc.Alias)
-	io.Copy(logstream, part)
-
-	return nil
+	logStream := NewLineWriter(proc.Alias)
+	_, err = io.Copy(logStream, part)
+	return err
 })

cli/lint/lint.go

@@ -43,7 +43,7 @@ func lint(c *cli.Context) error {
 		// check if it is a regular file (not dir)
 		if info.Mode().IsRegular() && strings.HasSuffix(info.Name(), ".yml") {
 			fmt.Println("#", info.Name())
-			lintFile(path)
+			_ = lintFile(path) // TODO: should we drop errors or store them and report back?
 			fmt.Println("")
 			return nil
 		}

cli/registry/registry_list.go

@@ -49,7 +49,9 @@ func registryList(c *cli.Context) error {
 		return err
 	}
 	for _, registry := range list {
-		tmpl.Execute(os.Stdout, registry)
+		if err := tmpl.Execute(os.Stdout, registry); err != nil {
+			return err
+		}
 	}
 	return nil
 }

cli/repo/repo_list.go

@@ -45,7 +45,9 @@ func repoList(c *cli.Context) error {
 		if org != "" && org != repo.Owner {
 			continue
 		}
-		tmpl.Execute(os.Stdout, repo)
+		if err := tmpl.Execute(os.Stdout, repo); err != nil {
+			return err
+		}
 	}
 	return nil
 }

cli/repo/repo_sync.go

@@ -41,7 +41,9 @@ func repoSync(c *cli.Context) error {
 		if org != "" && org != repo.Owner {
 			continue
 		}
-		tmpl.Execute(os.Stdout, repo)
+		if err := tmpl.Execute(os.Stdout, repo); err != nil {
+			return err
+		}
 	}
 	return nil
 }

cli/secret/secret_list.go

@@ -50,7 +50,9 @@ func secretList(c *cli.Context) error {
 		return err
 	}
 	for _, registry := range list {
-		tmpl.Execute(os.Stdout, registry)
+		if err := tmpl.Execute(os.Stdout, registry); err != nil {
+			return err
+		}
 	}
 	return nil
 }

cli/user/user_list.go

@@ -36,7 +36,9 @@ func userList(c *cli.Context) error {
 		return err
 	}
 	for _, user := range users {
-		tmpl.Execute(os.Stdout, user)
+		if err := tmpl.Execute(os.Stdout, user); err != nil {
+			return err
+		}
 	}
 	return nil
 }

cmd/agent/health.go

@@ -19,6 +19,7 @@ import (
 	"fmt"
 	"net/http"
 
+	"github.com/rs/zerolog/log"
 	"github.com/urfave/cli/v2"
 
 	"github.com/woodpecker-ci/woodpecker/agent"
@@ -46,7 +47,7 @@ func handleHeartbeat(w http.ResponseWriter, r *http.Request) {
 func handleVersion(w http.ResponseWriter, r *http.Request) {
 	w.WriteHeader(200)
 	w.Header().Add("Content-Type", "text/json")
-	json.NewEncoder(w).Encode(versionResp{
+	_ = json.NewEncoder(w).Encode(versionResp{
 		Source:  "https://github.com/woodpecker-ci/woodpecker",
 		Version: version.String(),
 	})
@@ -59,7 +60,9 @@ func handleStats(w http.ResponseWriter, r *http.Request) {
 		w.WriteHeader(500)
 	}
 	w.Header().Add("Content-Type", "text/json")
-	counter.WriteTo(w)
+	if _, err := counter.WriteTo(w); err != nil {
+		log.Error().Err(err).Msg("handleStats")
+	}
 }
 
 type versionResp struct {

cmd/agent/signal.go

@@ -12,14 +12,6 @@ import (
 	"syscall"
 )
 
-// WithContext returns a copy of parent context whose Done channel is closed
-// when an os interrupt signal is received.
-func WithContext(ctx context.Context) context.Context {
-	return WithContextFunc(ctx, func() {
-		println("interrupt received, terminating process")
-	})
-}
-
 // WithContextFunc returns a copy of parent context that is cancelled when
 // an os interrupt signal is received. The callback function f is invoked
 // before cancellation.

cmd/server/main.go

@@ -19,19 +19,21 @@ import (
 	"os"
 
 	"github.com/joho/godotenv"
-	_ "github.com/joho/godotenv/autoload"
+	"github.com/rs/zerolog/log"
 	"github.com/urfave/cli/v2"
 
 	"github.com/woodpecker-ci/woodpecker/version"
 )
 
 func main() {
-	godotenv.Load(".env")
+	if err := godotenv.Load(".env"); err != nil {
+		log.Error().Err(err).Msg("load godotenv failed")
+	}
 	app := cli.NewApp()
 	app.Name = "woodpecker-server"
 	app.Version = version.String()
 	app.Usage = "woodpecker server"
-	app.Action = loop
+	app.Action = run
 	app.Flags = flags
 	app.Before = before
 

cmd/server/server.go

@@ -49,7 +49,7 @@ import (
 	"github.com/woodpecker-ci/woodpecker/server/store"
 )
 
-func loop(c *cli.Context) error {
+func run(c *cli.Context) error {
 
 	if c.Bool("pretty") {
 		log.Logger = log.Output(
@@ -224,7 +224,9 @@ func loop(c *cli.Context) error {
 	}
 
 	dir := cacheDir()
-	os.MkdirAll(dir, 0700)
+	if err := os.MkdirAll(dir, 0700); err != nil {
+		return err
+	}
 
 	manager := &autocert.Manager{
 		Prompt:     autocert.AcceptTOS,
@@ -259,7 +261,9 @@ func setupEvilGlobals(c *cli.Context, v store.Store, r remote.Remote) {
 	server.Config.Services.Queue = setupQueue(c, v)
 	server.Config.Services.Logs = logging.New()
 	server.Config.Services.Pubsub = pubsub.New()
-	server.Config.Services.Pubsub.Create(context.Background(), "topic/events")
+	if err := server.Config.Services.Pubsub.Create(context.Background(), "topic/events"); err != nil {
+		log.Error().Err(err).Msg("could not create pubsub service")
+	}
 	server.Config.Services.Registries = setupRegistryService(c, v)
 	server.Config.Services.Secrets = setupSecretService(c, v)
 	server.Config.Services.Senders = sender.New(v, v)

go.mod

@@ -35,6 +35,7 @@ require (
 	github.com/moby/term v0.0.0-20210619224110-3f7ff695adc6
 	github.com/morikuni/aec v1.0.0 // indirect
 	github.com/mrjones/oauth v0.0.0-20190623134757-126b35219450
+	github.com/pkg/errors v0.9.1
 	github.com/prometheus/client_golang v1.11.0
 	github.com/rs/zerolog v1.25.0
 	github.com/stretchr/objx v0.3.0 // indirect

pipeline/backend/docker/convert.go

@@ -79,7 +79,7 @@ func toHostConfig(proc *backend.Step) *container.HostConfig {
 	}
 	config.Tmpfs = map[string]string{}
 	for _, path := range proc.Tmpfs {
-		if strings.Index(path, ":") == -1 {
+		if !strings.Contains(path, ":") {
 			config.Tmpfs[path] = ""
 			continue
 		}
@@ -89,9 +89,6 @@ func toHostConfig(proc *backend.Step) *container.HostConfig {
 		}
 		config.Tmpfs[parts[0]] = parts[1]
 	}
-	// if proc.OomKillDisable {
-	// 	config.OomKillDisable = &proc.OomKillDisable
-	// }
 
 	return config
 }

pipeline/backend/docker/docker.go

@@ -12,6 +12,7 @@ import (
 	"github.com/moby/moby/pkg/jsonmessage"
 	"github.com/moby/moby/pkg/stdcopy"
 	"github.com/moby/term"
+	"github.com/rs/zerolog/log"
 
 	"github.com/woodpecker-ci/woodpecker/pipeline/backend"
 )
@@ -80,7 +81,9 @@ func (e *engine) Exec(ctx context.Context, proc *backend.Step) error {
 			defer responseBody.Close()
 
 			fd, isTerminal := term.GetFdInfo(os.Stdout)
-			jsonmessage.DisplayJSONMessagesStream(responseBody, os.Stdout, fd, isTerminal, nil)
+			if err := jsonmessage.DisplayJSONMessagesStream(responseBody, os.Stdout, fd, isTerminal, nil); err != nil {
+				log.Error().Err(err).Msg("DisplayJSONMessagesStream")
+			}
 		}
 		// fix for drone/drone#1917
 		if perr != nil && proc.AuthConfig.Password != "" {
@@ -98,7 +101,9 @@ func (e *engine) Exec(ctx context.Context, proc *backend.Step) error {
 		}
 		defer responseBody.Close()
 		fd, isTerminal := term.GetFdInfo(os.Stdout)
-		jsonmessage.DisplayJSONMessagesStream(responseBody, os.Stdout, fd, isTerminal, nil)
+		if err := jsonmessage.DisplayJSONMessagesStream(responseBody, os.Stdout, fd, isTerminal, nil); err != nil {
+			log.Error().Err(err).Msg("DisplayJSONMessagesStream")
+		}
 
 		_, err = e.client.ContainerCreate(ctx, config, hostConfig, nil, nil, proc.Name)
 	}
@@ -162,27 +167,36 @@ func (e *engine) Tail(ctx context.Context, proc *backend.Step) (io.ReadCloser, e
 	}
 	rc, wc := io.Pipe()
 
+	// de multiplex 'logs' who contains two streams, previously multiplexed together using StdWriter
 	go func() {
-		stdcopy.StdCopy(wc, wc, logs)
-		logs.Close()
-		wc.Close()
-		rc.Close()
+		_, _ = stdcopy.StdCopy(wc, wc, logs)
+		_ = logs.Close()
+		_ = wc.Close()
+		_ = rc.Close()
 	}()
 	return rc, nil
 }
 
 func (e *engine) Destroy(_ context.Context, conf *backend.Config) error {
 	for _, stage := range conf.Stages {
 		for _, step := range stage.Steps {
-			e.client.ContainerKill(noContext, step.Name, "9")
-			e.client.ContainerRemove(noContext, step.Name, removeOpts)
+			if err := e.client.ContainerKill(noContext, step.Name, "9"); err != nil {
+				log.Error().Err(err).Msgf("could not kill container '%s'", stage.Name)
+			}
+			if err := e.client.ContainerRemove(noContext, step.Name, removeOpts); err != nil {
+				log.Error().Err(err).Msgf("could not remove container '%s'", stage.Name)
+			}
 		}
 	}
 	for _, v := range conf.Volumes {
-		e.client.VolumeRemove(noContext, v.Name, true)
+		if err := e.client.VolumeRemove(noContext, v.Name, true); err != nil {
+			log.Error().Err(err).Msgf("could not remove volume '%s'", v.Name)
+		}
 	}
 	for _, n := range conf.Networks {
-		e.client.NetworkRemove(noContext, n.Name)
+		if err := e.client.NetworkRemove(noContext, n.Name); err != nil {
+			log.Error().Err(err).Msgf("could not remove network '%s'", n.Name)
+		}
 	}
 	return nil
 }

pipeline/frontend/metadata.go

@@ -233,7 +233,7 @@ func (m *Metadata) EnvironDrone() map[string]string {
 	return params
 }
 
-var pullRegexp = regexp.MustCompile("\\d+")
+var pullRegexp = regexp.MustCompile(`\d+`)
 
 func (m *Metadata) SetPlatform(platform string) {
 	if platform == "" {

pipeline/frontend/yaml/compiler/convert.go

@@ -5,6 +5,8 @@ import (
 	"path"
 	"strings"
 
+	"github.com/rs/zerolog/log"
+
 	"github.com/woodpecker-ci/woodpecker/pipeline/backend"
 	"github.com/woodpecker-ci/woodpecker/pipeline/frontend/yaml"
 )
@@ -67,12 +69,14 @@ func (c *Compiler) createProcess(name string, container *yaml.Container, section
 		detached = true
 	}
 
-	if detached == false || len(container.Commands) != 0 {
+	if !detached || len(container.Commands) != 0 {
 		workingdir = path.Join(c.base, c.path)
 	}
 
-	if detached == false {
-		paramsToEnv(container.Vargs, environment)
+	if !detached {
+		if err := paramsToEnv(container.Vargs, environment); err != nil {
+			log.Error().Err(err).Msg("paramsToEnv")
+		}
 	}
 
 	if len(container.Commands) != 0 {

pipeline/frontend/yaml/compiler/params_test.go

@@ -5,6 +5,7 @@ import (
 	"testing"
 
 	"github.com/kr/pretty"
+	"github.com/stretchr/testify/assert"
 )
 
 func TestParamsToEnv(t *testing.T) {
@@ -28,7 +29,7 @@ func TestParamsToEnv(t *testing.T) {
 		"PLUGIN_COMPLEX": `[{"name":"Jack"},{"name":"Jill"}]`,
 	}
 	got := map[string]string{}
-	paramsToEnv(from, got)
+	assert.NoError(t, paramsToEnv(from, got))
 
 	if !reflect.DeepEqual(want, got) {
 		t.Errorf("Problem converting plugin parameters to environment variables")