all: Propagate errors from NamespacedKV (#6203)

As foretold by the prophecy, "once the database refactor is merged, then
shall appear a request to propagate errors from the store known
throughout the land as the NamedspacedKV, and it shall be good".

lib/api/api.go

@@ -769,11 +769,21 @@ func (s *service) getSystemConnections(w http.ResponseWriter, r *http.Request) {
 }
 
 func (s *service) getDeviceStats(w http.ResponseWriter, r *http.Request) {
-	sendJSON(w, s.model.DeviceStatistics())
+	stats, err := s.model.DeviceStatistics()
+	if err != nil {
+		http.Error(w, "Internal Server Error", http.StatusInternalServerError)
+		return
+	}
+	sendJSON(w, stats)
 }
 
 func (s *service) getFolderStats(w http.ResponseWriter, r *http.Request) {
-	sendJSON(w, s.model.FolderStatistics())
+	stats, err := s.model.FolderStatistics()
+	if err != nil {
+		http.Error(w, "Internal Server Error", http.StatusInternalServerError)
+		return
+	}
+	sendJSON(w, stats)
 }
 
 func (s *service) getDBFile(w http.ResponseWriter, r *http.Request) {

lib/api/mocked_model_test.go

@@ -48,12 +48,12 @@ func (m *mockedModel) ConnectionStats() map[string]interface{} {
 	return nil
 }
 
-func (m *mockedModel) DeviceStatistics() map[string]stats.DeviceStatistics {
+func (m *mockedModel) DeviceStatistics() (map[string]stats.DeviceStatistics, error) {
-	return nil
+	return nil, nil
 }
 
-func (m *mockedModel) FolderStatistics() map[string]stats.FolderStatistics {
+func (m *mockedModel) FolderStatistics() (map[string]stats.FolderStatistics, error) {
-	return nil
+	return nil, nil
 }
 
 func (m *mockedModel) CurrentFolderFile(folder string, file string) (protocol.FileInfo, bool) {

lib/db/namespaced.go

@@ -9,6 +9,8 @@ package db
 import (
 	"encoding/binary"
 	"time"
+
+	"github.com/syncthing/syncthing/lib/db/backend"
 )
 
 // NamespacedKV is a simple key-value store using a specific namespace within
@@ -42,13 +44,13 @@ func (n *NamespacedKV) PutInt64(key string, val int64) error {
 
 // Int64 returns the stored value interpreted as an int64 and a boolean that
 // is false if no value was stored at the key.
-func (n *NamespacedKV) Int64(key string) (int64, bool) {
+func (n *NamespacedKV) Int64(key string) (int64, bool, error) {
 	valBs, err := n.db.Get(n.prefixedKey(key))
 	if err != nil {
-		return 0, false
+		return 0, false, filterNotFound(err)
 	}
 	val := binary.BigEndian.Uint64(valBs)
-	return int64(val), true
+	return int64(val), true, nil
 }
 
 // PutTime stores a new time.Time. Any existing value (even if of another
@@ -60,14 +62,14 @@ func (n *NamespacedKV) PutTime(key string, val time.Time) error {
 
 // Time returns the stored value interpreted as a time.Time and a boolean
 // that is false if no value was stored at the key.
-func (n NamespacedKV) Time(key string) (time.Time, bool) {
+func (n NamespacedKV) Time(key string) (time.Time, bool, error) {
 	var t time.Time
 	valBs, err := n.db.Get(n.prefixedKey(key))
 	if err != nil {
-		return t, false
+		return t, false, filterNotFound(err)
 	}
 	err = t.UnmarshalBinary(valBs)
-	return t, err == nil
+	return t, err == nil, err
 }
 
 // PutString stores a new string. Any existing value (even if of another type)
@@ -78,12 +80,12 @@ func (n *NamespacedKV) PutString(key, val string) error {
 
 // String returns the stored value interpreted as a string and a boolean that
 // is false if no value was stored at the key.
-func (n NamespacedKV) String(key string) (string, bool) {
+func (n NamespacedKV) String(key string) (string, bool, error) {
 	valBs, err := n.db.Get(n.prefixedKey(key))
 	if err != nil {
-		return "", false
+		return "", false, filterNotFound(err)
 	}
-	return string(valBs), true
+	return string(valBs), true, nil
 }
 
 // PutBytes stores a new byte slice. Any existing value (even if of another type)
@@ -94,12 +96,12 @@ func (n *NamespacedKV) PutBytes(key string, val []byte) error {
 
 // Bytes returns the stored value as a raw byte slice and a boolean that
 // is false if no value was stored at the key.
-func (n NamespacedKV) Bytes(key string) ([]byte, bool) {
+func (n NamespacedKV) Bytes(key string) ([]byte, bool, error) {
 	valBs, err := n.db.Get(n.prefixedKey(key))
 	if err != nil {
-		return nil, false
+		return nil, false, filterNotFound(err)
 	}
-	return valBs, true
+	return valBs, true, nil
 }
 
 // PutBool stores a new boolean. Any existing value (even if of another type)
@@ -113,12 +115,12 @@ func (n *NamespacedKV) PutBool(key string, val bool) error {
 
 // Bool returns the stored value as a boolean and a boolean that
 // is false if no value was stored at the key.
-func (n NamespacedKV) Bool(key string) (bool, bool) {
+func (n NamespacedKV) Bool(key string) (bool, bool, error) {
 	valBs, err := n.db.Get(n.prefixedKey(key))
 	if err != nil {
-		return false, false
+		return false, false, filterNotFound(err)
 	}
-	return valBs[0] == 0x0, true
+	return valBs[0] == 0x0, true, nil
 }
 
 // Delete deletes the specified key. It is allowed to delete a nonexistent
@@ -150,3 +152,10 @@ func NewFolderStatisticsNamespace(db *Lowlevel, folder string) *NamespacedKV {
 func NewMiscDataNamespace(db *Lowlevel) *NamespacedKV {
 	return NewNamespacedKV(db, string(KeyTypeMiscData))
 }
+
+func filterNotFound(err error) error {
+	if backend.IsNotFound(err) {
+		return nil
+	}
+	return err
+}

lib/db/namespaced_test.go

@@ -21,7 +21,9 @@ func TestNamespacedInt(t *testing.T) {
 
 	// Key is missing to start with
 
-	if v, ok := n1.Int64("test"); v != 0 || ok {
+	if v, ok, err := n1.Int64("test"); err != nil {
+		t.Error("Unexpected error:", err)
+	} else if v != 0 || ok {
 		t.Errorf("Incorrect return v %v != 0 || ok %v != false", v, ok)
 	}
 
@@ -31,13 +33,17 @@ func TestNamespacedInt(t *testing.T) {
 
 	// It should now exist in n1
 
-	if v, ok := n1.Int64("test"); v != 42 || !ok {
+	if v, ok, err := n1.Int64("test"); err != nil {
+		t.Error("Unexpected error:", err)
+	} else if v != 42 || !ok {
 		t.Errorf("Incorrect return v %v != 42 || ok %v != true", v, ok)
 	}
 
 	// ... but not in n2, which is in a different namespace
 
-	if v, ok := n2.Int64("test"); v != 0 || ok {
+	if v, ok, err := n2.Int64("test"); err != nil {
+		t.Error("Unexpected error:", err)
+	} else if v != 0 || ok {
 		t.Errorf("Incorrect return v %v != 0 || ok %v != false", v, ok)
 	}
 
@@ -47,7 +53,9 @@ func TestNamespacedInt(t *testing.T) {
 
 	// It should no longer exist
 
-	if v, ok := n1.Int64("test"); v != 0 || ok {
+	if v, ok, err := n1.Int64("test"); err != nil {
+		t.Error("Unexpected error:", err)
+	} else if v != 0 || ok {
 		t.Errorf("Incorrect return v %v != 0 || ok %v != false", v, ok)
 	}
 }
@@ -57,7 +65,9 @@ func TestNamespacedTime(t *testing.T) {
 
 	n1 := NewNamespacedKV(ldb, "foo")
 
-	if v, ok := n1.Time("test"); !v.IsZero() || ok {
+	if v, ok, err := n1.Time("test"); err != nil {
+		t.Error("Unexpected error:", err)
+	} else if !v.IsZero() || ok {
 		t.Errorf("Incorrect return v %v != %v || ok %v != false", v, time.Time{}, ok)
 	}
 
@@ -66,7 +76,9 @@ func TestNamespacedTime(t *testing.T) {
 		t.Fatal(err)
 	}
 
-	if v, ok := n1.Time("test"); !v.Equal(now) || !ok {
+	if v, ok, err := n1.Time("test"); err != nil {
+		t.Error("Unexpected error:", err)
+	} else if !v.Equal(now) || !ok {
 		t.Errorf("Incorrect return v %v != %v || ok %v != true", v, now, ok)
 	}
 }
@@ -76,7 +88,9 @@ func TestNamespacedString(t *testing.T) {
 
 	n1 := NewNamespacedKV(ldb, "foo")
 
-	if v, ok := n1.String("test"); v != "" || ok {
+	if v, ok, err := n1.String("test"); err != nil {
+		t.Error("Unexpected error:", err)
+	} else if v != "" || ok {
 		t.Errorf("Incorrect return v %q != \"\" || ok %v != false", v, ok)
 	}
 
@@ -84,7 +98,9 @@ func TestNamespacedString(t *testing.T) {
 		t.Fatal(err)
 	}
 
-	if v, ok := n1.String("test"); v != "yo" || !ok {
+	if v, ok, err := n1.String("test"); err != nil {
+		t.Error("Unexpected error:", err)
+	} else if v != "yo" || !ok {
 		t.Errorf("Incorrect return v %q != \"yo\" || ok %v != true", v, ok)
 	}
 }
@@ -104,25 +120,37 @@ func TestNamespacedReset(t *testing.T) {
 		t.Fatal(err)
 	}
 
-	if v, ok := n1.String("test1"); v != "yo1" || !ok {
+	if v, ok, err := n1.String("test1"); err != nil {
+		t.Error("Unexpected error:", err)
+	} else if v != "yo1" || !ok {
 		t.Errorf("Incorrect return v %q != \"yo1\" || ok %v != true", v, ok)
 	}
-	if v, ok := n1.String("test2"); v != "yo2" || !ok {
+	if v, ok, err := n1.String("test2"); err != nil {
+		t.Error("Unexpected error:", err)
+	} else if v != "yo2" || !ok {
 		t.Errorf("Incorrect return v %q != \"yo2\" || ok %v != true", v, ok)
 	}
-	if v, ok := n1.String("test3"); v != "yo3" || !ok {
+	if v, ok, err := n1.String("test3"); err != nil {
+		t.Error("Unexpected error:", err)
+	} else if v != "yo3" || !ok {
 		t.Errorf("Incorrect return v %q != \"yo3\" || ok %v != true", v, ok)
 	}
 
 	reset(n1)
 
-	if v, ok := n1.String("test1"); v != "" || ok {
+	if v, ok, err := n1.String("test1"); err != nil {
+		t.Error("Unexpected error:", err)
+	} else if v != "" || ok {
 		t.Errorf("Incorrect return v %q != \"\" || ok %v != false", v, ok)
 	}
-	if v, ok := n1.String("test2"); v != "" || ok {
+	if v, ok, err := n1.String("test2"); err != nil {
+		t.Error("Unexpected error:", err)
+	} else if v != "" || ok {
 		t.Errorf("Incorrect return v %q != \"\" || ok %v != false", v, ok)
 	}
-	if v, ok := n1.String("test3"); v != "" || ok {
+	if v, ok, err := n1.String("test3"); err != nil {
+		t.Error("Unexpected error:", err)
+	} else if v != "" || ok {
 		t.Errorf("Incorrect return v %q != \"\" || ok %v != false", v, ok)
 	}
 }

lib/db/schemaupdater.go

@@ -49,11 +49,16 @@ type schemaUpdater struct {
 
 func (db *schemaUpdater) updateSchema() error {
 	miscDB := NewMiscDataNamespace(db.Lowlevel)
-	prevVersion, _ := miscDB.Int64("dbVersion")
+	prevVersion, _, err := miscDB.Int64("dbVersion")
+	if err != nil {
+		return err
+	}
 
 	if prevVersion > dbVersion {
 		err := databaseDowngradeError{}
-		if minSyncthingVersion, ok := miscDB.String("dbMinSyncthingVersion"); ok {
+		if minSyncthingVersion, ok, dbErr := miscDB.String("dbMinSyncthingVersion"); dbErr != nil {
+			return dbErr
+		} else if ok {
 			err.minSyncthingVersion = minSyncthingVersion
 		}
 		return err

lib/fs/mtimefs.go

@@ -12,7 +12,7 @@ import (
 
 // The database is where we store the virtual mtimes
 type database interface {
-	Bytes(key string) (data []byte, ok bool)
+	Bytes(key string) (data []byte, ok bool, err error)
 	PutBytes(key string, data []byte) error
 	Delete(key string) error
 }
@@ -72,7 +72,10 @@ func (f *MtimeFS) Stat(name string) (FileInfo, error) {
 		return nil, err
 	}
 
-	real, virtual := f.load(name)
+	real, virtual, err := f.load(name)
+	if err != nil {
+		return nil, err
+	}
 	if real == info.ModTime() {
 		info = mtimeFileInfo{
 			FileInfo: info,
@@ -89,7 +92,10 @@ func (f *MtimeFS) Lstat(name string) (FileInfo, error) {
 		return nil, err
 	}
 
-	real, virtual := f.load(name)
+	real, virtual, err := f.load(name)
+	if err != nil {
+		return nil, err
+	}
 	if real == info.ModTime() {
 		info = mtimeFileInfo{
 			FileInfo: info,
@@ -103,7 +109,11 @@ func (f *MtimeFS) Lstat(name string) (FileInfo, error) {
 func (f *MtimeFS) Walk(root string, walkFn WalkFunc) error {
 	return f.Filesystem.Walk(root, func(path string, info FileInfo, err error) error {
 		if info != nil {
-			real, virtual := f.load(path)
+			real, virtual, loadErr := f.load(path)
+			if loadErr != nil && err == nil {
+				// The iterator gets to deal with the error
+				err = loadErr
+			}
 			if real == info.ModTime() {
 				info = mtimeFileInfo{
 					FileInfo: info,
@@ -162,22 +172,24 @@ func (f *MtimeFS) save(name string, real, virtual time.Time) {
 	f.db.PutBytes(name, bs)
 }
 
-func (f *MtimeFS) load(name string) (real, virtual time.Time) {
+func (f *MtimeFS) load(name string) (real, virtual time.Time, err error) {
 	if f.caseInsensitive {
 		name = UnicodeLowercase(name)
 	}
 
-	data, exists := f.db.Bytes(name)
+	data, exists, err := f.db.Bytes(name)
-	if !exists {
+	if err != nil {
-		return
+		return time.Time{}, time.Time{}, err
+	} else if !exists {
+		return time.Time{}, time.Time{}, nil
 	}
 
 	var mtime dbMtime
 	if err := mtime.Unmarshal(data); err != nil {
-		return
+		return time.Time{}, time.Time{}, err
 	}
 
-	return mtime.real, mtime.virtual
+	return mtime.real, mtime.virtual, nil
 }
 
 // The mtimeFileInfo is an os.FileInfo that lies about the ModTime().
@@ -202,7 +214,10 @@ func (f *mtimeFile) Stat() (FileInfo, error) {
 		return nil, err
 	}
 
-	real, virtual := f.fs.load(f.Name())
+	real, virtual, err := f.fs.load(f.Name())
+	if err != nil {
+		return nil, err
+	}
 	if real == info.ModTime() {
 		info = mtimeFileInfo{
 			FileInfo: info,

lib/fs/mtimefs_test.go

@@ -241,7 +241,7 @@ func (s mapStore) PutBytes(key string, data []byte) error {
 	return nil
 }
 
-func (s mapStore) Bytes(key string) (data []byte, ok bool) {
+func (s mapStore) Bytes(key string) (data []byte, ok bool, err error) {
 	data, ok = s[key]
 	return
 }

lib/model/model.go

@@ -59,7 +59,7 @@ type service interface {
 	Errors() []FileError
 	WatchError() error
 	ForceRescan(file protocol.FileInfo) error
-	GetStatistics() stats.FolderStatistics
+	GetStatistics() (stats.FolderStatistics, error)
 
 	getState() (folderState, time.Time, error)
 }
@@ -108,8 +108,8 @@ type Model interface {
 
 	Completion(device protocol.DeviceID, folder string) FolderCompletion
 	ConnectionStats() map[string]interface{}
-	DeviceStatistics() map[string]stats.DeviceStatistics
+	DeviceStatistics() (map[string]stats.DeviceStatistics, error)
-	FolderStatistics() map[string]stats.FolderStatistics
+	FolderStatistics() (map[string]stats.FolderStatistics, error)
 	UsageReportingStats(version int, preview bool) map[string]interface{}
 
 	StartDeadlockDetector(timeout time.Duration)
@@ -706,25 +706,33 @@ func (m *model) ConnectionStats() map[string]interface{} {
 }
 
 // DeviceStatistics returns statistics about each device
-func (m *model) DeviceStatistics() map[string]stats.DeviceStatistics {
+func (m *model) DeviceStatistics() (map[string]stats.DeviceStatistics, error) {
 	m.fmut.RLock()
 	defer m.fmut.RUnlock()
 	res := make(map[string]stats.DeviceStatistics, len(m.deviceStatRefs))
 	for id, sr := range m.deviceStatRefs {
-		res[id.String()] = sr.GetStatistics()
+		stats, err := sr.GetStatistics()
+		if err != nil {
+			return nil, err
+		}
+		res[id.String()] = stats
 	}
-	return res
+	return res, nil
 }
 
 // FolderStatistics returns statistics about each folder
-func (m *model) FolderStatistics() map[string]stats.FolderStatistics {
+func (m *model) FolderStatistics() (map[string]stats.FolderStatistics, error) {
 	res := make(map[string]stats.FolderStatistics)
 	m.fmut.RLock()
 	defer m.fmut.RUnlock()
 	for id, runner := range m.folderRunners {
-		res[id] = runner.GetStatistics()
+		stats, err := runner.GetStatistics()
+		if err != nil {
+			return nil, err
+		}
+		res[id] = stats
 	}
-	return res
+	return res, nil
 }
 
 type FolderCompletion struct {

lib/model/model_test.go

@@ -3392,7 +3392,10 @@ func TestDeviceWasSeen(t *testing.T) {
 
 	m.deviceWasSeen(device1)
 
-	stats := m.DeviceStatistics()
+	stats, err := m.DeviceStatistics()
+	if err != nil {
+		t.Error("Unexpected error:", err)
+	}
 	entry := stats[device1.String()]
 	if time.Since(entry.LastSeen) > time.Second {
 		t.Error("device should have been seen now")

lib/stats/device.go

@@ -28,24 +28,30 @@ func NewDeviceStatisticsReference(ldb *db.Lowlevel, device string) *DeviceStatis
 	}
 }
 
-func (s *DeviceStatisticsReference) GetLastSeen() time.Time {
+func (s *DeviceStatisticsReference) GetLastSeen() (time.Time, error) {
-	t, ok := s.ns.Time("lastSeen")
+	t, ok, err := s.ns.Time("lastSeen")
-	if !ok {
+	if err != nil {
+		return time.Time{}, err
+	} else if !ok {
 		// The default here is 1970-01-01 as opposed to the default
 		// time.Time{} from s.ns
-		return time.Unix(0, 0)
+		return time.Unix(0, 0), nil
 	}
 	l.Debugln("stats.DeviceStatisticsReference.GetLastSeen:", s.device, t)
-	return t
+	return t, nil
 }
 
-func (s *DeviceStatisticsReference) WasSeen() {
+func (s *DeviceStatisticsReference) WasSeen() error {
 	l.Debugln("stats.DeviceStatisticsReference.WasSeen:", s.device)
-	s.ns.PutTime("lastSeen", time.Now())
+	return s.ns.PutTime("lastSeen", time.Now())
 }
 
-func (s *DeviceStatisticsReference) GetStatistics() DeviceStatistics {
+func (s *DeviceStatisticsReference) GetStatistics() (DeviceStatistics, error) {
-	return DeviceStatistics{
+	lastSeen, err := s.GetLastSeen()
-		LastSeen: s.GetLastSeen(),
+	if err != nil {
+		return DeviceStatistics{}, err
 	}
+	return DeviceStatistics{
+		LastSeen: lastSeen,
+	}, nil
 }

lib/stats/folder.go

@@ -35,45 +35,69 @@ func NewFolderStatisticsReference(ldb *db.Lowlevel, folder string) *FolderStatis
 	}
 }
 
-func (s *FolderStatisticsReference) GetLastFile() LastFile {
+func (s *FolderStatisticsReference) GetLastFile() (LastFile, error) {
-	at, ok := s.ns.Time("lastFileAt")
+	at, ok, err := s.ns.Time("lastFileAt")
-	if !ok {
+	if err != nil {
-		return LastFile{}
+		return LastFile{}, err
+	} else if !ok {
+		return LastFile{}, nil
 	}
-	file, ok := s.ns.String("lastFileName")
+	file, ok, err := s.ns.String("lastFileName")
-	if !ok {
+	if err != nil {
-		return LastFile{}
+		return LastFile{}, err
+	} else if !ok {
+		return LastFile{}, nil
+	}
+	deleted, _, err := s.ns.Bool("lastFileDeleted")
+	if err != nil {
+		return LastFile{}, err
 	}
-	deleted, _ := s.ns.Bool("lastFileDeleted")
 	return LastFile{
 		At:       at,
 		Filename: file,
 		Deleted:  deleted,
-	}
+	}, nil
 }
 
-func (s *FolderStatisticsReference) ReceivedFile(file string, deleted bool) {
+func (s *FolderStatisticsReference) ReceivedFile(file string, deleted bool) error {
 	l.Debugln("stats.FolderStatisticsReference.ReceivedFile:", s.folder, file)
-	s.ns.PutTime("lastFileAt", time.Now())
+	if err := s.ns.PutTime("lastFileAt", time.Now()); err != nil {
-	s.ns.PutString("lastFileName", file)
+		return err
-	s.ns.PutBool("lastFileDeleted", deleted)
-}
-
-func (s *FolderStatisticsReference) ScanCompleted() {
-	s.ns.PutTime("lastScan", time.Now())
-}
-
-func (s *FolderStatisticsReference) GetLastScanTime() time.Time {
-	lastScan, ok := s.ns.Time("lastScan")
-	if !ok {
-		return time.Time{}
 	}
-	return lastScan
+	if err := s.ns.PutString("lastFileName", file); err != nil {
+		return err
+	}
+	if err := s.ns.PutBool("lastFileDeleted", deleted); err != nil {
+		return err
+	}
+	return nil
 }
 
-func (s *FolderStatisticsReference) GetStatistics() FolderStatistics {
+func (s *FolderStatisticsReference) ScanCompleted() error {
+	return s.ns.PutTime("lastScan", time.Now())
+}
+
+func (s *FolderStatisticsReference) GetLastScanTime() (time.Time, error) {
+	lastScan, ok, err := s.ns.Time("lastScan")
+	if err != nil {
+		return time.Time{}, err
+	} else if !ok {
+		return time.Time{}, nil
+	}
+	return lastScan, nil
+}
+
+func (s *FolderStatisticsReference) GetStatistics() (FolderStatistics, error) {
+	lastFile, err := s.GetLastFile()
+	if err != nil {
+		return FolderStatistics{}, err
+	}
+	lastScanTime, err := s.GetLastScanTime()
+	if err != nil {
+		return FolderStatistics{}, err
+	}
 	return FolderStatistics{
-		LastFile: s.GetLastFile(),
+		LastFile: lastFile,
-		LastScan: s.GetLastScanTime(),
+		LastScan: lastScanTime,
-	}
+	}, nil
 }

lib/syncthing/syncthing.go

@@ -210,7 +210,11 @@ func (a *App) startup() error {
 	// Grab the previously running version string from the database.
 
 	miscDB := db.NewMiscDataNamespace(a.ll)
-	prevVersion, _ := miscDB.String("prevVersion")
+	prevVersion, _, err := miscDB.String("prevVersion")
+	if err != nil {
+		l.Warnln("Database:", err)
+		return err
+	}
 
 	// Strip away prerelease/beta stuff and just compare the release
 	// numbers. 0.14.44 to 0.14.45-banana is an upgrade, 0.14.45-banana to