Maintenance: improve error wrapping

.golangci.yml

@@ -10,6 +10,7 @@ issues:
       linters:
         - dupl
         - maligned
+        - goerr113
     - path: internal/server/
       linters:
         - dupl
@@ -40,7 +41,7 @@ linters:
     - gomnd
     - goprintffuncname
     - gosec
-    # - goerr113
+    - goerr113
     - gosimple
     - govet
     - importas

cmd/gluetun/main.go

@@ -106,6 +106,10 @@ func main() {
 	nativeos.Exit(1)
 }
 
+var (
+	errCommandUnknown = errors.New("command is unknown")
+)
+
 //nolint:gocognit,gocyclo
 func _main(ctx context.Context, buildInfo models.BuildInformation,
 	args []string, logger logging.ParentLogger, os os.OS,
@@ -121,7 +125,7 @@ func _main(ctx context.Context, buildInfo models.BuildInformation,
 		case "update":
 			return cli.Update(ctx, args[2:], os, logger)
 		default:
-			return fmt.Errorf("command %q is unknown", args[1])
+			return fmt.Errorf("%w: %s", errCommandUnknown, args[1])
 		}
 	}
 
@@ -429,7 +433,7 @@ func routeReadyEvents(ctx context.Context, done chan<- struct{}, buildInfo model
 				first = false
 				message, err := versionpkg.GetMessage(ctx, buildInfo, httpClient)
 				if err != nil {
-					logger.Error(err)
+					logger.Error("cannot get version information: " + err.Error())
 				} else {
 					logger.Info(message)
 				}

internal/cli/update.go

@@ -2,6 +2,7 @@ package cli
 
 import (
 	"context"
+	"errors"
 	"flag"
 	"fmt"
 	"net/http"
@@ -15,6 +16,13 @@ import (
 	"github.com/qdm12/golibs/os"
 )
 
+var (
+	ErrNoFileOrStdoutFlag      = errors.New("at least one of -file or -stdout must be specified")
+	ErrSyncServers             = errors.New("cannot sync hardcoded and persisted servers")
+	ErrUpdateServerInformation = errors.New("cannot update server information")
+	ErrWriteToFile             = errors.New("cannot write updated information to file")
+)
+
 func (c *cli) Update(ctx context.Context, args []string, os os.OS, logger logging.Logger) error {
 	options := configuration.Updater{CLI: true}
 	var flushToFile bool
@@ -40,7 +48,7 @@ func (c *cli) Update(ctx context.Context, args []string, os os.OS, logger loggin
 		return err
 	}
 	if !flushToFile && !options.Stdout {
-		return fmt.Errorf("at least one of -file or -stdout must be specified")
+		return ErrNoFileOrStdoutFlag
 	}
 
 	const clientTimeout = 10 * time.Second
@@ -48,16 +56,16 @@ func (c *cli) Update(ctx context.Context, args []string, os os.OS, logger loggin
 	storage := storage.New(logger, os, constants.ServersData)
 	currentServers, err := storage.SyncServers(constants.GetAllServers())
 	if err != nil {
-		return fmt.Errorf("cannot update servers: %w", err)
+		return fmt.Errorf("%w: %s", ErrSyncServers, err)
 	}
 	updater := updater.New(options, httpClient, currentServers, logger)
 	allServers, err := updater.UpdateServers(ctx)
 	if err != nil {
-		return err
+		return fmt.Errorf("%w: %s", ErrUpdateServerInformation, err)
 	}
 	if flushToFile {
 		if err := storage.FlushToFile(allServers); err != nil {
-			return fmt.Errorf("cannot update servers: %w", err)
+			return fmt.Errorf("%w: %s", ErrWriteToFile, err)
 		}
 	}
 

internal/configuration/cyberghost.go

@@ -2,7 +2,7 @@ package configuration
 
 import (
 	"encoding/pem"
-	"fmt"
+	"errors"
 	"strings"
 
 	"github.com/qdm12/gluetun/internal/constants"
@@ -81,10 +81,12 @@ func readCyberghostClientKey(r reader) (clientKey string, err error) {
 	return extractClientKey(b)
 }
 
+var errDecodePEMBlockClientKey = errors.New("cannot decode PEM block from client key")
+
 func extractClientKey(b []byte) (key string, err error) {
 	pemBlock, _ := pem.Decode(b)
 	if pemBlock == nil {
-		return "", fmt.Errorf("cannot decode PEM block from client key")
+		return "", errDecodePEMBlockClientKey
 	}
 	parsedBytes := pem.EncodeToMemory(pemBlock)
 	s := string(parsedBytes)
@@ -102,10 +104,12 @@ func readCyberghostClientCertificate(r reader) (clientCertificate string, err er
 	return extractClientCertificate(b)
 }
 
+var errDecodePEMBlockClientCert = errors.New("cannot decode PEM block from client certificate")
+
 func extractClientCertificate(b []byte) (certificate string, err error) {
 	pemBlock, _ := pem.Decode(b)
 	if pemBlock == nil {
-		return "", fmt.Errorf("cannot decode PEM block from client certificate")
+		return "", errDecodePEMBlockClientCert
 	}
 	parsedBytes := pem.EncodeToMemory(pemBlock)
 	s := string(parsedBytes)

internal/configuration/cyberghost_test.go

@@ -1,7 +1,6 @@
 package configuration
 
 import (
-	"fmt"
 	"testing"
 
 	"github.com/stretchr/testify/assert"
@@ -71,11 +70,11 @@ zZjrL52saevO25cigVl+hxcnY8DTpbk=
 		err error
 	}{
 		"no input": {
-			err: fmt.Errorf("cannot decode PEM block from client key"),
+			err: errDecodePEMBlockClientKey,
 		},
 		"bad input": {
 			b:   []byte{1, 2, 3},
-			err: fmt.Errorf("cannot decode PEM block from client key"),
+			err: errDecodePEMBlockClientKey,
 		},
 		"valid key": {
 			b:   []byte(validPEM),
@@ -147,11 +146,11 @@ iOCYTbretAFZRhh6ycUN5hBeN8GMQxiMreMtDV4PEIQ=
 		err         error
 	}{
 		"no input": {
-			err: fmt.Errorf("cannot decode PEM block from client certificate"),
+			err: errDecodePEMBlockClientCert,
 		},
 		"bad input": {
 			b:   []byte{1, 2, 3},
-			err: fmt.Errorf("cannot decode PEM block from client certificate"),
+			err: errDecodePEMBlockClientCert,
 		},
 		"valid key": {
 			b:           []byte(validPEM),

internal/dns/state.go

@@ -1,6 +1,7 @@
 package dns
 
 import (
+	"errors"
 	"fmt"
 	"reflect"
 	"sync"
@@ -29,6 +30,8 @@ func (l *looper) GetStatus() (status models.LoopStatus) {
 	return l.state.status
 }
 
+var ErrInvalidStatus = errors.New("invalid status")
+
 func (l *looper) SetStatus(status models.LoopStatus) (outcome string, err error) {
 	l.state.statusMu.Lock()
 	defer l.state.statusMu.Unlock()
@@ -64,8 +67,8 @@ func (l *looper) SetStatus(status models.LoopStatus) (outcome string, err error)
 		l.state.status = constants.Stopped
 		return status.String(), nil
 	default:
-		return "", fmt.Errorf("status %q can only be %q or %q",
-			status, constants.Running, constants.Stopped)
+		return "", fmt.Errorf("%w: %s: it can only be one of: %s, %s",
+			ErrInvalidStatus, status, constants.Running, constants.Stopped)
 	}
 }
 

internal/firewall/ip6tables.go

@@ -41,16 +41,18 @@ func (c *configurator) runIP6tablesInstruction(ctx context.Context, instruction
 	}
 	flags := strings.Fields(instruction)
 	if output, err := c.commander.Run(ctx, "ip6tables", flags...); err != nil {
-		return fmt.Errorf("%w \"ip6tables %s\": %s: %s", ErrIP6Tables, instruction, output, err)
+		return fmt.Errorf("%w: \"ip6tables %s\": %s: %s", ErrIP6Tables, instruction, output, err)
 	}
 	return nil
 }
 
+var errPolicyNotValid = errors.New("policy is not valid")
+
 func (c *configurator) setIPv6AllPolicies(ctx context.Context, policy string) error {
 	switch policy {
 	case "ACCEPT", "DROP":
 	default:
-		return fmt.Errorf("policy %q not recognized", policy)
+		return fmt.Errorf("%w: %s", errPolicyNotValid, policy)
 	}
 	return c.runIP6tablesInstructions(ctx, []string{
 		"--policy INPUT " + policy,

internal/healthcheck/client.go

@@ -2,11 +2,16 @@ package healthcheck
 
 import (
 	"context"
+	"errors"
 	"fmt"
 	"io"
 	"net/http"
 )
 
+var (
+	ErrHTTPStatusNotOK = errors.New("HTTP response status is not OK")
+)
+
 type Checker interface {
 	Check(ctx context.Context, url string) error
 }
@@ -38,5 +43,5 @@ func (h *checker) Check(ctx context.Context, url string) error {
 	if err != nil {
 		return err
 	}
-	return fmt.Errorf("%s: %s", response.Status, string(b))
+	return fmt.Errorf("%w: %s: %s", ErrHTTPStatusNotOK, response.Status, string(b))
 }

internal/httpproxy/state.go

@@ -1,6 +1,7 @@
 package httpproxy
 
 import (
+	"errors"
 	"fmt"
 	"reflect"
 	"sync"
@@ -29,6 +30,8 @@ func (l *looper) GetStatus() (status models.LoopStatus) {
 	return l.state.status
 }
 
+var ErrInvalidStatus = errors.New("invalid status")
+
 func (l *looper) SetStatus(status models.LoopStatus) (outcome string, err error) {
 	l.state.statusMu.Lock()
 	defer l.state.statusMu.Unlock()
@@ -64,8 +67,8 @@ func (l *looper) SetStatus(status models.LoopStatus) (outcome string, err error)
 		l.state.status = status
 		return status.String(), nil
 	default:
-		return "", fmt.Errorf("status %q can only be %q or %q",
-			status, constants.Running, constants.Stopped)
+		return "", fmt.Errorf("%w: %s: it can only be one of: %s, %s",
+			ErrInvalidStatus, status, constants.Running, constants.Stopped)
 	}
 }
 

internal/openvpn/command.go

@@ -2,6 +2,7 @@ package openvpn
 
 import (
 	"context"
+	"errors"
 	"fmt"
 	"strings"
 
@@ -14,6 +15,8 @@ func (c *configurator) Start(ctx context.Context) (
 	return c.commander.Start(ctx, "openvpn", "--config", constants.OpenVPNConf)
 }
 
+var ErrVersionTooShort = errors.New("version output is too short")
+
 func (c *configurator) Version(ctx context.Context) (string, error) {
 	output, err := c.commander.Run(ctx, "openvpn", "--version")
 	if err != nil && err.Error() != "exit status 1" {
@@ -23,7 +26,7 @@ func (c *configurator) Version(ctx context.Context) (string, error) {
 	words := strings.Fields(firstLine)
 	const minWords = 2
 	if len(words) < minWords {
-		return "", fmt.Errorf("openvpn --version: first line is too short: %q", firstLine)
+		return "", fmt.Errorf("%w: %s", ErrVersionTooShort, firstLine)
 	}
 	return words[1], nil
 }

internal/openvpn/state.go

@@ -1,6 +1,7 @@
 package openvpn
 
 import (
+	"errors"
 	"fmt"
 	"reflect"
 	"sync"
@@ -43,6 +44,8 @@ func (l *looper) GetStatus() (status models.LoopStatus) {
 	return l.state.status
 }
 
+var ErrInvalidStatus = errors.New("invalid status")
+
 func (l *looper) SetStatus(status models.LoopStatus) (outcome string, err error) {
 	l.state.statusMu.Lock()
 	defer l.state.statusMu.Unlock()
@@ -78,8 +81,8 @@ func (l *looper) SetStatus(status models.LoopStatus) (outcome string, err error)
 		l.state.status = constants.Stopped
 		return status.String(), nil
 	default:
-		return "", fmt.Errorf("status %q can only be %q or %q",
-			status, constants.Running, constants.Stopped)
+		return "", fmt.Errorf("%w: %s: it can only be one of: %s, %s",
+			ErrInvalidStatus, status, constants.Running, constants.Stopped)
 	}
 }
 

internal/provider/privateinternetaccess/portforward.go

@@ -478,7 +478,7 @@ func writePortForwardedToFile(openFile os.OpenFileFunc,
 // replaceInErr is used to remove sensitive information from errors.
 func replaceInErr(err error, substitutions map[string]string) error {
 	s := replaceInString(err.Error(), substitutions)
-	return errors.New(s)
+	return errors.New(s) //nolint:goerr113
 }
 
 // replaceInString is used to remove sensitive information.

internal/publicip/publicip.go

@@ -3,6 +3,7 @@ package publicip
 
 import (
 	"context"
+	"errors"
 	"fmt"
 	"io"
 	"math/rand"
@@ -27,6 +28,8 @@ func NewIPGetter(client *http.Client) IPGetter {
 	}
 }
 
+var ErrParseIP = errors.New("cannot parse IP address")
+
 func (i *ipGetter) Get(ctx context.Context) (ip net.IP, err error) {
 	urls := []string{
 		"https://ifconfig.me/ip",
@@ -63,7 +66,7 @@ func (i *ipGetter) Get(ctx context.Context) (ip net.IP, err error) {
 	s := strings.ReplaceAll(string(content), "\n", "")
 	ip = net.ParseIP(s)
 	if ip == nil {
-		return nil, fmt.Errorf("cannot parse IP address from %q", s)
+		return nil, fmt.Errorf("%w: %s", ErrParseIP, s)
 	}
 	return ip, nil
 }

internal/publicip/state.go

@@ -1,6 +1,7 @@
 package publicip
 
 import (
+	"errors"
 	"fmt"
 	"net"
 	"reflect"
@@ -32,6 +33,8 @@ func (l *looper) GetStatus() (status models.LoopStatus) {
 	return l.state.status
 }
 
+var ErrInvalidStatus = errors.New("invalid status")
+
 func (l *looper) SetStatus(status models.LoopStatus) (outcome string, err error) {
 	l.state.statusMu.Lock()
 	defer l.state.statusMu.Unlock()
@@ -67,8 +70,8 @@ func (l *looper) SetStatus(status models.LoopStatus) (outcome string, err error)
 		l.state.status = status
 		return status.String(), nil
 	default:
-		return "", fmt.Errorf("status %q can only be %q or %q",
-			status, constants.Running, constants.Stopped)
+		return "", fmt.Errorf("%w: %s: it can only be one of: %s, %s",
+			ErrInvalidStatus, status, constants.Running, constants.Stopped)
 	}
 }
 

internal/server/server.go

@@ -3,6 +3,7 @@ package server
 
 import (
 	"context"
+	"errors"
 	"net/http"
 	"time"
 
@@ -51,7 +52,7 @@ func (s *server) Run(ctx context.Context, done chan<- struct{}) {
 	}()
 	s.logger.Info("listening on %s", s.address)
 	err := server.ListenAndServe()
-	if err != nil && ctx.Err() != context.Canceled {
+	if err != nil && errors.Is(ctx.Err(), context.Canceled) {
 		s.logger.Error(err)
 	}
 }

internal/server/wrappers.go

@@ -1,6 +1,7 @@
 package server
 
 import (
+	"errors"
 	"fmt"
 
 	"github.com/qdm12/gluetun/internal/constants"
@@ -11,15 +12,16 @@ type statusWrapper struct {
 	Status string `json:"status"`
 }
 
+var errInvalidStatus = errors.New("invalid status")
+
 func (sw *statusWrapper) getStatus() (status models.LoopStatus, err error) {
 	status = models.LoopStatus(sw.Status)
 	switch status {
 	case constants.Stopped, constants.Running:
 		return status, nil
 	default:
-		return "", fmt.Errorf(
-			"invalid status %q: possible values are: %s, %s",
-			sw.Status, constants.Stopped, constants.Running)
+		return "", fmt.Errorf("%w: %s: possible values are: %s, %s",
+			errInvalidStatus, sw.Status, constants.Stopped, constants.Running)
 	}
 }
 

internal/shadowsocks/state.go

@@ -1,6 +1,7 @@
 package shadowsocks
 
 import (
+	"errors"
 	"fmt"
 	"reflect"
 	"sync"
@@ -29,6 +30,8 @@ func (l *looper) GetStatus() (status models.LoopStatus) {
 	return l.state.status
 }
 
+var ErrInvalidStatus = errors.New("invalid status")
+
 func (l *looper) SetStatus(status models.LoopStatus) (outcome string, err error) {
 	l.state.statusMu.Lock()
 	defer l.state.statusMu.Unlock()
@@ -64,8 +67,8 @@ func (l *looper) SetStatus(status models.LoopStatus) (outcome string, err error)
 		l.state.status = status
 		return status.String(), nil
 	default:
-		return "", fmt.Errorf("status %q can only be %q or %q",
-			status, constants.Running, constants.Stopped)
+		return "", fmt.Errorf("%w: %s: it can only be one of: %s, %s",
+			ErrInvalidStatus, status, constants.Running, constants.Stopped)
 	}
 }
 

internal/updater/state.go

@@ -1,6 +1,7 @@
 package updater
 
 import (
+	"errors"
 	"fmt"
 	"reflect"
 	"sync"
@@ -29,6 +30,8 @@ func (l *looper) GetStatus() (status models.LoopStatus) {
 	return l.state.status
 }
 
+var ErrInvalidStatus = errors.New("invalid status")
+
 func (l *looper) SetStatus(status models.LoopStatus) (outcome string, err error) {
 	l.state.statusMu.Lock()
 	defer l.state.statusMu.Unlock()
@@ -64,8 +67,8 @@ func (l *looper) SetStatus(status models.LoopStatus) (outcome string, err error)
 		l.state.status = status
 		return status.String(), nil
 	default:
-		return "", fmt.Errorf("status %q can only be %q or %q",
-			status, constants.Running, constants.Stopped)
+		return "", fmt.Errorf("%w: %s: it can only be one of: %s, %s",
+			ErrInvalidStatus, status, constants.Running, constants.Stopped)
 	}
 }
 

internal/version/version.go

@@ -4,6 +4,7 @@ package version
 
 import (
 	"context"
+	"errors"
 	"fmt"
 	"net/http"
 	"time"
@@ -20,7 +21,7 @@ func GetMessage(ctx context.Context, buildInfo models.BuildInformation,
 		// Find # of commits between current commit and latest commit
 		commitsSince, err := getCommitsSince(ctx, client, buildInfo.Commit)
 		if err != nil {
-			return "", fmt.Errorf("cannot get version information: %w", err)
+			return "", err
 		} else if commitsSince == 0 {
 			return fmt.Sprintf("You are running on the bleeding edge of %s!", buildInfo.Version), nil
 		}
@@ -32,7 +33,7 @@ func GetMessage(ctx context.Context, buildInfo models.BuildInformation,
 	}
 	tagName, name, releaseTime, err := getLatestRelease(ctx, client)
 	if err != nil {
-		return "", fmt.Errorf("cannot get version information: %w", err)
+		return "", err
 	}
 	if tagName == buildInfo.Version {
 		return fmt.Sprintf("You are running the latest release %s", buildInfo.Version), nil
@@ -43,6 +44,8 @@ func GetMessage(ctx context.Context, buildInfo models.BuildInformation,
 		nil
 }
 
+var errReleaseNotFound = errors.New("release not found")
+
 func getLatestRelease(ctx context.Context, client *http.Client) (tagName, name string, time time.Time, err error) {
 	releases, err := getGithubReleases(ctx, client)
 	if err != nil {
@@ -54,9 +57,11 @@ func getLatestRelease(ctx context.Context, client *http.Client) (tagName, name s
 		}
 		return release.TagName, release.Name, release.PublishedAt, nil
 	}
-	return "", "", time, fmt.Errorf("no releases found")
+	return "", "", time, errReleaseNotFound
 }
 
+var errCommitNotFound = errors.New("commit not found")
+
 func getCommitsSince(ctx context.Context, client *http.Client, commitShort string) (n int, err error) {
 	commits, err := getGithubCommits(ctx, client)
 	if err != nil {
@@ -68,5 +73,5 @@ func getCommitsSince(ctx context.Context, client *http.Client, commitShort strin
 		}
 		n++
 	}
-	return 0, fmt.Errorf("no commit matching %q was found", commitShort)
+	return 0, fmt.Errorf("%w: %s", errCommitNotFound, commitShort)
 }