all: Modernize error wrapping (syncthing#8491)

This replaces old style errors.Wrap with modern fmt.Errorf and removes
the (direct) dependency on github.com/pkg/errors. A couple of cases are
adjusted by hand as previously errors.Wrap(nil, ...) would return nil,
which is not what fmt.Errorf does.

cmd/syncthing/cli/config.go

@@ -8,11 +8,11 @@ package cli
 
 import (
 	"encoding/json"
+	"errors"
 	"fmt"
 	"reflect"
 
 	"github.com/AudriusButkevicius/recli"
-	"github.com/pkg/errors"
 	"github.com/syncthing/syncthing/lib/config"
 	"github.com/urfave/cli"
 )

cmd/syncthing/cli/main.go

@@ -8,14 +8,14 @@ package cli
 
 import (
 	"bufio"
+	"errors"
 	"fmt"
 	"io"
 	"os"
 	"strings"
 
 	"github.com/alecthomas/kong"
 	"github.com/flynn-archive/go-shlex"
-	"github.com/pkg/errors"
 	"github.com/urfave/cli"
 
 	"github.com/syncthing/syncthing/cmd/syncthing/cmdutil"
@@ -51,7 +51,7 @@ func runInternal(c preCli, cliArgs []string) error {
 	// Not set as default above because the strings can be really long.
 	err := cmdutil.SetConfigDataLocationsFromFlags(c.HomeDir, c.ConfDir, c.DataDir)
 	if err != nil {
-		return errors.Wrap(err, "Command line options:")
+		return fmt.Errorf("Command line options: %w", err)
 	}
 	clientFactory := &apiClientFactory{
 		cfg: config.GUIConfiguration{
@@ -124,7 +124,7 @@ func runInternal(c preCli, cliArgs []string) error {
 					for scanner.Scan() {
 						input, err := shlex.Split(scanner.Text())
 						if err != nil {
-							return errors.Wrap(err, "parsing input")
+							return fmt.Errorf("parsing input: %w", err)
 						}
 						if len(input) == 0 {
 							continue

cmd/syncthing/main.go

@@ -10,6 +10,7 @@ import (
 	"bytes"
 	"context"
 	"crypto/tls"
+	"errors"
 	"fmt"
 	"io"
 	"log"
@@ -50,8 +51,6 @@ import (
 	"github.com/syncthing/syncthing/lib/svcutil"
 	"github.com/syncthing/syncthing/lib/syncthing"
 	"github.com/syncthing/syncthing/lib/upgrade"
-
-	"github.com/pkg/errors"
 )
 
 const (

go.mod

@@ -37,7 +37,7 @@ require (
 	github.com/miscreant/miscreant.go v0.0.0-20200214223636-26d376326b75
 	github.com/oschwald/geoip2-golang v1.5.0
 	github.com/pierrec/lz4/v4 v4.1.15
-	github.com/pkg/errors v0.9.1
+	github.com/pkg/errors v0.9.1 // indirect
 	github.com/prometheus/client_golang v1.12.2
 	github.com/prometheus/common v0.32.1 // indirect
 	github.com/prometheus/procfs v0.7.3 // indirect

lib/config/config.go

@@ -10,6 +10,7 @@ package config
 import (
 	"encoding/json"
 	"encoding/xml"
+	"errors"
 	"fmt"
 	"io"
 	"net"
@@ -19,8 +20,6 @@ import (
 	"strconv"
 	"strings"
 
-	"github.com/pkg/errors"
-
 	"github.com/syncthing/syncthing/lib/build"
 	"github.com/syncthing/syncthing/lib/fs"
 	"github.com/syncthing/syncthing/lib/protocol"
@@ -116,13 +115,13 @@ func New(myID protocol.DeviceID) Configuration {
 func (cfg *Configuration) ProbeFreePorts() error {
 	port, err := getFreePort("127.0.0.1", DefaultGUIPort)
 	if err != nil {
-		return errors.Wrap(err, "get free port (GUI)")
+		return fmt.Errorf("get free port (GUI): %w", err)
 	}
 	cfg.GUI.RawAddress = fmt.Sprintf("127.0.0.1:%d", port)
 
 	port, err = getFreePort("0.0.0.0", DefaultTCPPort)
 	if err != nil {
-		return errors.Wrap(err, "get free port (BEP)")
+		return fmt.Errorf("get free port (BEP): %w", err)
 	}
 	if port == DefaultTCPPort {
 		cfg.Options.RawListenAddresses = []string{"default"}

lib/connections/quic_dial.go

@@ -12,12 +12,12 @@ package connections
 import (
 	"context"
 	"crypto/tls"
+	"fmt"
 	"net"
 	"net/url"
 	"time"
 
 	"github.com/lucas-clemente/quic-go"
-	"github.com/pkg/errors"
 
 	"github.com/syncthing/syncthing/lib/config"
 	"github.com/syncthing/syncthing/lib/connections/registry"
@@ -79,7 +79,7 @@ func (d *quicDialer) Dial(ctx context.Context, _ protocol.DeviceID, uri *url.URL
 		if createdConn != nil {
 			_ = createdConn.Close()
 		}
-		return internalConn{}, errors.Wrap(err, "dial")
+		return internalConn{}, fmt.Errorf("dial: %w", err)
 	}
 
 	stream, err := session.OpenStreamSync(ctx)
@@ -89,7 +89,7 @@ func (d *quicDialer) Dial(ctx context.Context, _ protocol.DeviceID, uri *url.URL
 		if createdConn != nil {
 			_ = createdConn.Close()
 		}
-		return internalConn{}, errors.Wrap(err, "open stream")
+		return internalConn{}, fmt.Errorf("open stream: %w", err)
 	}
 
 	return newInternalConn(&quicTlsConn{session, stream, createdConn}, connTypeQUICClient, quicPriority), nil

lib/connections/relay_listen.go

@@ -9,12 +9,11 @@ package connections
 import (
 	"context"
 	"crypto/tls"
+	"errors"
 	"net/url"
 	"sync"
 	"time"
 
-	"github.com/pkg/errors"
-
 	"github.com/syncthing/syncthing/lib/config"
 	"github.com/syncthing/syncthing/lib/connections/registry"
 	"github.com/syncthing/syncthing/lib/dialer"

lib/connections/service.go

@@ -13,6 +13,7 @@ import (
 	"context"
 	"crypto/tls"
 	"crypto/x509"
+	"errors"
 	"fmt"
 	"math"
 	"net"
@@ -37,7 +38,6 @@ import (
 	_ "github.com/syncthing/syncthing/lib/pmp"
 	_ "github.com/syncthing/syncthing/lib/upnp"
 
-	"github.com/pkg/errors"
 	"github.com/thejerf/suture/v4"
 	"golang.org/x/time/rate"
 )

lib/model/folder.go

@@ -8,15 +8,14 @@ package model
 
 import (
 	"context"
+	"errors"
 	"fmt"
 	"math/rand"
 	"path/filepath"
 	"sort"
 	"sync/atomic"
 	"time"
 
-	"github.com/pkg/errors"
-
 	"github.com/syncthing/syncthing/lib/config"
 	"github.com/syncthing/syncthing/lib/db"
 	"github.com/syncthing/syncthing/lib/events"
@@ -317,7 +316,7 @@ func (f *folder) getHealthErrorAndLoadIgnores() error {
 	}
 	if f.Type != config.FolderTypeReceiveEncrypted {
 		if err := f.ignores.Load(".stignore"); err != nil && !fs.IsNotExist(err) {
-			return errors.Wrap(err, "loading ignores")
+			return fmt.Errorf("loading ignores: %w", err)
 		}
 	}
 	return nil
@@ -1060,6 +1059,7 @@ func (f *folder) monitorWatch(ctx context.Context) {
 				l.Warnf("Filesystem watching (kqueue) is enabled on %v with a lot of files/directories, and that requires a lot of resources and might slow down your system significantly", f.Description())
 			}
 		case <-ctx.Done():
+			aggrCancel() // for good measure and keeping the linters happy
 			return
 		}
 	}

lib/model/folder_sendrecv.go

@@ -8,6 +8,7 @@ package model
 
 import (
 	"bytes"
+	"errors"
 	"fmt"
 	"io"
 	"path/filepath"
@@ -16,8 +17,6 @@ import (
 	"strings"
 	"time"
 
-	"github.com/pkg/errors"
-
 	"github.com/syncthing/syncthing/lib/build"
 	"github.com/syncthing/syncthing/lib/config"
 	"github.com/syncthing/syncthing/lib/db"
@@ -593,7 +592,7 @@ func (f *sendReceiveFolder) handleDir(file protocol.FileInfo, snap *db.Snapshot,
 		// Check that it is what we have in the database.
 		curFile, hasCurFile := snap.Get(protocol.LocalDeviceID, file.Name)
 		if err := f.scanIfItemChanged(file.Name, info, curFile, hasCurFile, scanChan); err != nil {
-			err = errors.Wrap(err, "handling dir")
+			err = fmt.Errorf("handling dir: %w", err)
 			f.newPullError(file.Name, err)
 			return
 		}
@@ -647,13 +646,13 @@ func (f *sendReceiveFolder) handleDir(file protocol.FileInfo, snap *db.Snapshot,
 		if err = f.inWritableDir(mkdir, file.Name); err == nil {
 			dbUpdateChan <- dbUpdateJob{file, dbUpdateHandleDir}
 		} else {
-			f.newPullError(file.Name, errors.Wrap(err, "creating directory"))
+			f.newPullError(file.Name, fmt.Errorf("creating directory: %w", err))
 		}
 		return
 	// Weird error when stat()'ing the dir. Probably won't work to do
 	// anything else with it if we can't even stat() it.
 	case err != nil:
-		f.newPullError(file.Name, errors.Wrap(err, "checking file to be replaced"))
+		f.newPullError(file.Name, fmt.Errorf("checking file to be replaced: %w", err))
 		return
 	}
 
@@ -675,7 +674,7 @@ func (f *sendReceiveFolder) checkParent(file string, scanChan chan<- string) boo
 	parent := filepath.Dir(file)
 
 	if err := osutil.TraversesSymlink(f.mtimefs, parent); err != nil {
-		f.newPullError(file, errors.Wrap(err, "checking parent dirs"))
+		f.newPullError(file, fmt.Errorf("checking parent dirs: %w", err))
 		return false
 	}
 
@@ -700,7 +699,7 @@ func (f *sendReceiveFolder) checkParent(file string, scanChan chan<- string) boo
 	}
 	l.Debugf("%v creating parent directory of %v", f, file)
 	if err := f.mtimefs.MkdirAll(parent, 0755); err != nil {
-		f.newPullError(file, errors.Wrap(err, "creating parent dir"))
+		f.newPullError(file, fmt.Errorf("creating parent dir: %w", err))
 		return false
 	}
 	if f.Type != config.FolderTypeReceiveEncrypted {
@@ -761,7 +760,7 @@ func (f *sendReceiveFolder) handleSymlink(file protocol.FileInfo, snap *db.Snaps
 	if err = f.inWritableDir(createLink, file.Name); err == nil {
 		dbUpdateChan <- dbUpdateJob{file, dbUpdateHandleSymlink}
 	} else {
-		f.newPullError(file.Name, errors.Wrap(err, "symlink create"))
+		f.newPullError(file.Name, fmt.Errorf("symlink create: %w", err))
 	}
 }
 
@@ -810,7 +809,7 @@ func (f *sendReceiveFolder) deleteDir(file protocol.FileInfo, snap *db.Snapshot,
 
 	defer func() {
 		if err != nil {
-			f.newPullError(file.Name, errors.Wrap(err, "delete dir"))
+			f.newPullError(file.Name, fmt.Errorf("delete dir: %w", err))
 		}
 		f.evLogger.Log(events.ItemFinished, map[string]interface{}{
 			"folder": f.folderID,
@@ -860,7 +859,7 @@ func (f *sendReceiveFolder) deleteFileWithCurrent(file, cur protocol.FileInfo, h
 
 	defer func() {
 		if err != nil {
-			f.newPullError(file.Name, errors.Wrap(err, "delete file"))
+			f.newPullError(file.Name, fmt.Errorf("delete file: %w", err))
 		}
 		f.evLogger.Log(events.ItemFinished, map[string]interface{}{
 			"folder": f.folderID,
@@ -1307,7 +1306,7 @@ func (f *sendReceiveFolder) copierRoutine(in <-chan copyBlocksState, pullChan ch
 		for _, block := range state.blocks {
 			select {
 			case <-f.ctx.Done():
-				state.fail(errors.Wrap(f.ctx.Err(), "folder stopped"))
+				state.fail(fmt.Errorf("folder stopped: %w", f.ctx.Err()))
 				break blocks
 			default:
 			}
@@ -1336,7 +1335,7 @@ func (f *sendReceiveFolder) copierRoutine(in <-chan copyBlocksState, pullChan ch
 
 					err = f.limitedWriteAt(dstFd, buf, block.Offset)
 					if err != nil {
-						state.fail(errors.Wrap(err, "dst write"))
+						state.fail(fmt.Errorf("dst write: %w", err))
 					}
 					if offset == block.Offset {
 						state.copiedFromOrigin()
@@ -1386,7 +1385,7 @@ func (f *sendReceiveFolder) copierRoutine(in <-chan copyBlocksState, pullChan ch
 						err = f.limitedWriteAt(dstFd, buf, block.Offset)
 					}
 					if err != nil {
-						state.fail(errors.Wrap(err, "dst write"))
+						state.fail(fmt.Errorf("dst write: %w", err))
 					}
 					if path == state.file.Name {
 						state.copiedFromOrigin()
@@ -1536,7 +1535,7 @@ loop:
 	for {
 		select {
 		case <-f.ctx.Done():
-			state.fail(errors.Wrap(f.ctx.Err(), "folder stopped"))
+			state.fail(fmt.Errorf("folder stopped: %w", f.ctx.Err()))
 			break loop
 		default:
 		}
@@ -1547,9 +1546,9 @@ loop:
 		found := activity.leastBusy(candidates)
 		if found == -1 {
 			if lastError != nil {
-				state.fail(errors.Wrap(lastError, "pull"))
+				state.fail(fmt.Errorf("pull: %w", lastError))
 			} else {
-				state.fail(errors.Wrap(errNoDevice, "pull"))
+				state.fail(fmt.Errorf("pull: %w", errNoDevice))
 			}
 			break
 		}
@@ -1587,7 +1586,7 @@ loop:
 		// Save the block data we got from the cluster
 		err = f.limitedWriteAt(fd, buf, state.block.Offset)
 		if err != nil {
-			state.fail(errors.Wrap(err, "save"))
+			state.fail(fmt.Errorf("save: %w", err))
 		} else {
 			state.pullDone(state.block)
 		}
@@ -1822,14 +1821,14 @@ func (f *sendReceiveFolder) moveForConflict(name, lastModBy string, scanChan cha
 	if isConflict(name) {
 		l.Infoln("Conflict for", name, "which is already a conflict copy; not copying again.")
 		if err := f.mtimefs.Remove(name); err != nil && !fs.IsNotExist(err) {
-			return errors.Wrap(err, contextRemovingOldItem)
+			return fmt.Errorf("%s: %w", contextRemovingOldItem, err)
 		}
 		return nil
 	}
 
 	if f.MaxConflicts == 0 {
 		if err := f.mtimefs.Remove(name); err != nil && !fs.IsNotExist(err) {
-			return errors.Wrap(err, contextRemovingOldItem)
+			return fmt.Errorf("%s: %w", contextRemovingOldItem, err)
 		}
 		return nil
 	}
@@ -1888,7 +1887,9 @@ func (f *sendReceiveFolder) newPullError(path string, err error) {
 // deleteItemOnDisk deletes the file represented by old that is about to be replaced by new.
 func (f *sendReceiveFolder) deleteItemOnDisk(item protocol.FileInfo, snap *db.Snapshot, scanChan chan<- string) (err error) {
 	defer func() {
-		err = errors.Wrap(err, contextRemovingOldItem)
+		if err != nil {
+			err = fmt.Errorf("%s: %w", contextRemovingOldItem, err)
+		}
 	}()
 
 	switch {
@@ -2056,7 +2057,7 @@ func (f *sendReceiveFolder) scanIfItemChanged(name string, stat fs.FileInfo, ite
 	// touching the item.
 	statItem, err := scanner.CreateFileInfo(stat, item.Name, f.mtimefs, f.SyncOwnership)
 	if err != nil {
-		return errors.Wrap(err, "comparing item on disk to db")
+		return fmt.Errorf("comparing item on disk to db: %w", err)
 	}
 
 	if !statItem.IsEquivalentOptional(item, protocol.FileInfoComparison{
@@ -2122,10 +2123,10 @@ func (f *sendReceiveFolder) copyOwnershipFromParent(path string) error {
 
 	info, err := f.mtimefs.Lstat(filepath.Dir(path))
 	if err != nil {
-		return errors.Wrap(err, "copy owner from parent")
+		return fmt.Errorf("copy owner from parent: %w", err)
 	}
 	if err := f.mtimefs.Lchown(path, strconv.Itoa(info.Owner()), strconv.Itoa(info.Group())); err != nil {
-		return errors.Wrap(err, "copy owner from parent")
+		return fmt.Errorf("copy owner from parent: %w", err)
 	}
 	return nil
 }