all: Make all error implementations pointer types (#6726)

This matches the convention of the stdlib and avoids ambiguity: when
customErr{} and &customErr{} both implement error, client code needs to
check for both.

Memory use should remain the same, since storing a non-pointer type in
an interface value still copies the value to the heap.

cmd/syncthing/main.go

@@ -522,7 +522,7 @@ type errNoUpgrade struct {
 	current, latest string
 }
 
-func (e errNoUpgrade) Error() string {
+func (e *errNoUpgrade) Error() string {
 	return fmt.Sprintf("no upgrade available (current %q >= latest %q).", e.current, e.latest)
 }
 
@@ -538,7 +538,7 @@ func checkUpgrade() (upgrade.Release, error) {
 	}
 
 	if upgrade.CompareVersions(release.Tag, build.Version) <= 0 {
-		return upgrade.Release{}, errNoUpgrade{build.Version, release.Tag}
+		return upgrade.Release{}, &errNoUpgrade{build.Version, release.Tag}
 	}
 
 	l.Infof("Upgrade available (current %q < latest %q)", build.Version, release.Tag)
@@ -646,7 +646,7 @@ func syncthingMain(runtimeOptions RuntimeOptions) {
 			err = upgrade.To(release)
 		}
 		if err != nil {
-			if _, ok := err.(errNoUpgrade); ok || err == errTooEarlyUpgradeCheck || err == errTooEarlyUpgrade {
+			if _, ok := err.(*errNoUpgrade); ok || err == errTooEarlyUpgradeCheck || err == errTooEarlyUpgrade {
 				l.Debugln("Initial automatic upgrade:", err)
 			} else {
 				l.Infoln("Initial automatic upgrade:", err)
@@ -996,7 +996,7 @@ func setPauseState(cfg config.Wrapper, paused bool) {
 }
 
 func exitCodeForUpgrade(err error) int {
-	if _, ok := err.(errNoUpgrade); ok {
+	if _, ok := err.(*errNoUpgrade); ok {
 		return syncthing.ExitNoUpgradeAvailable.AsInt()
 	}
 	return syncthing.ExitError.AsInt()

lib/db/backend/backend.go

@@ -146,30 +146,20 @@ func OpenMemory() Backend {
 
 type errClosed struct{}
 
-func (errClosed) Error() string { return "database is closed" }
+func (*errClosed) Error() string { return "database is closed" }
 
 type errNotFound struct{}
 
-func (errNotFound) Error() string { return "key not found" }
+func (*errNotFound) Error() string { return "key not found" }
 
 func IsClosed(err error) bool {
-	if _, ok := err.(errClosed); ok {
+	_, ok := err.(*errClosed)
-		return true
+	return ok
-	}
-	if _, ok := err.(*errClosed); ok {
-		return true
-	}
-	return false
 }
 
 func IsNotFound(err error) bool {
-	if _, ok := err.(errNotFound); ok {
+	_, ok := err.(*errNotFound)
-		return true
+	return ok
-	}
-	if _, ok := err.(*errNotFound); ok {
-		return true
-	}
-	return false
 }
 
 // releaser manages counting on top of a waitgroup
@@ -209,7 +199,7 @@ func (cg *closeWaitGroup) Add(i int) error {
 	cg.closeMut.RLock()
 	defer cg.closeMut.RUnlock()
 	if cg.closed {
-		return errClosed{}
+		return &errClosed{}
 	}
 	cg.WaitGroup.Add(i)
 	return nil

lib/db/backend/badger_backend.go

@@ -402,10 +402,10 @@ func wrapBadgerErr(err error) error {
 		return nil
 	}
 	if err == badger.ErrDiscardedTxn {
-		return errClosed{}
+		return &errClosed{}
 	}
 	if err == badger.ErrKeyNotFound {
-		return errNotFound{}
+		return &errNotFound{}
 	}
 	return err
 }

lib/db/backend/leveldb_backend.go

@@ -207,14 +207,11 @@ func (it *leveldbIterator) Error() error {
 
 // wrapLeveldbErr wraps errors so that the backend package can recognize them
 func wrapLeveldbErr(err error) error {
-	if err == nil {
-		return nil
-	}
 	if err == leveldb.ErrClosed {
-		return errClosed{}
+		return &errClosed{}
 	}
 	if err == leveldb.ErrNotFound {
-		return errNotFound{}
+		return &errNotFound{}
 	}
 	return err
 }

lib/db/backend/leveldb_open.go

@@ -149,12 +149,12 @@ func open(location string, opts *opt.Options) (*leveldb.DB, error) {
 		// the database and reindexing...
 		l.Infoln("Database corruption detected, unable to recover. Reinitializing...")
 		if err := os.RemoveAll(location); err != nil {
-			return nil, errorSuggestion{err, "failed to delete corrupted database"}
+			return nil, &errorSuggestion{err, "failed to delete corrupted database"}
 		}
 		db, err = leveldb.OpenFile(location, opts)
 	}
 	if err != nil {
-		return nil, errorSuggestion{err, "is another instance of Syncthing running?"}
+		return nil, &errorSuggestion{err, "is another instance of Syncthing running?"}
 	}
 
 	if debugEnvValue("CompactEverything", 0) != 0 {
@@ -227,6 +227,6 @@ type errorSuggestion struct {
 	suggestion string
 }
 
-func (e errorSuggestion) Error() string {
+func (e *errorSuggestion) Error() string {
 	return fmt.Sprintf("%s (%s)", e.inner.Error(), e.suggestion)
 }

lib/db/db_test.go

@@ -477,7 +477,7 @@ func TestDowngrade(t *testing.T) {
 	// Pretend we just opened the DB and attempt to update it again
 	err := UpdateSchema(db)
 
-	if err, ok := err.(databaseDowngradeError); !ok {
+	if err, ok := err.(*databaseDowngradeError); !ok {
 		t.Fatal("Expected error due to database downgrade, got", err)
 	} else if err.minSyncthingVersion != dbMinSyncthingVersion {
 		t.Fatalf("Error has %v as min Syncthing version, expected %v", err.minSyncthingVersion, dbMinSyncthingVersion)

lib/db/schemaupdater.go

@@ -38,7 +38,7 @@ type databaseDowngradeError struct {
 	minSyncthingVersion string
 }
 
-func (e databaseDowngradeError) Error() string {
+func (e *databaseDowngradeError) Error() string {
 	if e.minSyncthingVersion == "" {
 		return "newer Syncthing required"
 	}
@@ -67,7 +67,7 @@ func (db *schemaUpdater) updateSchema() error {
 	}
 
 	if prevVersion > dbVersion {
-		err := databaseDowngradeError{}
+		err := &databaseDowngradeError{}
 		if minSyncthingVersion, ok, dbErr := miscDB.String("dbMinSyncthingVersion"); dbErr != nil {
 			return dbErr
 		} else if ok {

lib/discover/global.go

@@ -65,11 +65,13 @@ type serverOptions struct {
 
 // A lookupError is any other error but with a cache validity time attached.
 type lookupError struct {
-	error
+	msg      string
 	cacheFor time.Duration
 }
 
-func (e lookupError) CacheFor() time.Duration {
+func (e *lookupError) Error() string { return e.msg }
+
+func (e *lookupError) CacheFor() time.Duration {
 	return e.cacheFor
 }
 
@@ -142,8 +144,8 @@ func NewGlobal(server string, cert tls.Certificate, addrList AddressLister, evLo
 // Lookup returns the list of addresses where the given device is available
 func (c *globalClient) Lookup(ctx context.Context, device protocol.DeviceID) (addresses []string, err error) {
 	if c.noLookup {
-		return nil, lookupError{
+		return nil, &lookupError{
-			error:    errors.New("lookups not supported"),
+			msg:      "lookups not supported",
 			cacheFor: time.Hour,
 		}
 	}
@@ -167,8 +169,8 @@ func (c *globalClient) Lookup(ctx context.Context, device protocol.DeviceID) (ad
 		l.Debugln("globalClient.Lookup", qURL, resp.Status)
 		err := errors.New(resp.Status)
 		if secs, atoiErr := strconv.Atoi(resp.Header.Get("Retry-After")); atoiErr == nil && secs > 0 {
-			err = lookupError{
+			err = &lookupError{
-				error:    err,
+				msg:      resp.Status,
 				cacheFor: time.Duration(secs) * time.Second,
 			}
 		}

lib/osutil/traversessymlink.go

@@ -19,7 +19,7 @@ type TraversesSymlinkError struct {
 	path string
 }
 
-func (e TraversesSymlinkError) Error() string {
+func (e *TraversesSymlinkError) Error() string {
 	return fmt.Sprintf("traverses symlink: %s", e.path)
 }
 
@@ -28,7 +28,7 @@ type NotADirectoryError struct {
 	path string
 }
 
-func (e NotADirectoryError) Error() string {
+func (e *NotADirectoryError) Error() string {
 	return fmt.Sprintf("not a directory: %s", e.path)
 }
 

lib/relay/client/methods.go

@@ -5,7 +5,6 @@ package client
 import (
 	"context"
 	"crypto/tls"
-	"errors"
 	"fmt"
 	"net"
 	"net/url"
@@ -22,7 +21,7 @@ type incorrectResponseCodeErr struct {
 	msg  string
 }
 
-func (e incorrectResponseCodeErr) Error() string {
+func (e *incorrectResponseCodeErr) Error() string {
 	return fmt.Sprintf("incorrect response code %d: %s", e.code, e.msg)
 }
 
@@ -62,7 +61,7 @@ func GetInvitationFromRelay(ctx context.Context, uri *url.URL, id syncthingproto
 
 	switch msg := message.(type) {
 	case protocol.Response:
-		return protocol.SessionInvitation{}, incorrectResponseCodeErr{msg.Code, msg.Message}
+		return protocol.SessionInvitation{}, &incorrectResponseCodeErr{msg.Code, msg.Message}
 	case protocol.SessionInvitation:
 		l.Debugln("Received invitation", msg, "via", conn.LocalAddr())
 		ip := net.IP(msg.Address)
@@ -132,7 +131,7 @@ func TestRelay(ctx context.Context, uri *url.URL, certs []tls.Certificate, sleep
 		if err == nil {
 			return nil
 		}
-		if !errors.As(err, &incorrectResponseCodeErr{}) {
+		if _, ok := err.(*incorrectResponseCodeErr); !ok {
 			return fmt.Errorf("getting invitation: %w", err)
 		}
 		time.Sleep(sleep)

lib/relay/client/static.go

@@ -201,7 +201,7 @@ func (c *staticClient) join() error {
 	switch msg := message.(type) {
 	case protocol.Response:
 		if msg.Code != 0 {
-			return incorrectResponseCodeErr{msg.Code, msg.Message}
+			return &incorrectResponseCodeErr{msg.Code, msg.Message}
 		}
 
 	case protocol.RelayFull:

lib/upnp/upnp.go

@@ -79,7 +79,7 @@ type UnsupportedDeviceTypeError struct {
 	deviceType string
 }
 
-func (e UnsupportedDeviceTypeError) Error() string {
+func (e *UnsupportedDeviceTypeError) Error() string {
 	return fmt.Sprintf("Unsupported UPnP device of type %s", e.deviceType)
 }