restic: Rework error handling of FindFilteredSnapshots and handle snapshotIDs

FindFilteredSnapshots no longer prints errors during snapshot loading on
stderr, but instead passes the error to the callback to allow the caller
to decide on what to do.

In addition, it moves the logic to handle an explicit snapshot list from
the main package to restic.

cmd/restic/find.go

@@ -37,70 +37,26 @@ func FindFilteredSnapshots(ctx context.Context, be restic.Lister, loader restic.
 	out := make(chan *restic.Snapshot)
 	go func() {
 		defer close(out)
-		if len(snapshotIDs) != 0 {
-			// memorize snapshots list to prevent repeated backend listings
 		be, err := backend.MemorizeList(ctx, be, restic.SnapshotFile)
 		if err != nil {
 			Warnf("could not load snapshots: %v\n", err)
 			return
 		}
 
-			var (
-				id         restic.ID
-				usedFilter bool
-			)
-			ids := make(restic.IDs, 0, len(snapshotIDs))
-			// Process all snapshot IDs given as arguments.
-			for _, s := range snapshotIDs {
-				if s == "latest" {
-					usedFilter = true
-					id, err = restic.FindLatestSnapshot(ctx, be, loader, paths, tags, hosts, nil)
+		err = restic.FindFilteredSnapshots(ctx, be, loader, hosts, tags, paths, snapshotIDs, func(id string, sn *restic.Snapshot, err error) error {
 			if err != nil {
-						Warnf("Ignoring %q, no snapshot matched given filter (Paths:%v Tags:%v Hosts:%v)\n", s, paths, tags, hosts)
-						continue
-					}
+				Warnf("Ignoring %q: %v\n", id, err)
 			} else {
-					id, err = restic.FindSnapshot(ctx, be, s)
-					if err != nil {
-						Warnf("Ignoring %q: %v\n", s, err)
-						continue
-					}
-				}
-				ids = append(ids, id)
-			}
-
-			// Give the user some indication their filters are not used.
-			if !usedFilter && (len(hosts) != 0 || len(tags) != 0 || len(paths) != 0) {
-				Warnf("Ignoring filters as there are explicit snapshot ids given\n")
-			}
-
-			for _, id := range ids.Uniq() {
-				sn, err := restic.LoadSnapshot(ctx, loader, id)
-				if err != nil {
-					Warnf("Ignoring %q, could not load snapshot: %v\n", id, err)
-					continue
-				}
 				select {
 				case <-ctx.Done():
-					return
+					return ctx.Err()
 				case out <- sn:
 				}
 			}
-			return
-		}
-
-		snapshots, err := restic.FindFilteredSnapshots(ctx, be, loader, hosts, tags, paths)
+			return nil
+		})
 		if err != nil {
 			Warnf("could not load snapshots: %v\n", err)
-			return
-		}
-
-		for _, sn := range snapshots {
-			select {
-			case <-ctx.Done():
-				return
-			case out <- sn:
-			}
 		}
 	}()
 	return out

internal/fuse/snapshots_dirstruct.go

@@ -292,7 +292,13 @@ func (d *SnapshotsDirStructure) updateSnapshots(ctx context.Context) error {
 		return nil
 	}
 
-	snapshots, err := restic.FindFilteredSnapshots(ctx, d.root.repo.Backend(), d.root.repo, d.root.cfg.Hosts, d.root.cfg.Tags, d.root.cfg.Paths)
+	var snapshots restic.Snapshots
+	err := restic.FindFilteredSnapshots(ctx, d.root.repo.Backend(), d.root.repo, d.root.cfg.Hosts, d.root.cfg.Tags, d.root.cfg.Paths, nil, func(id string, sn *restic.Snapshot, err error) error {
+		if sn != nil {
+			snapshots = append(snapshots, sn)
+		}
+		return nil
+	})
 	if err != nil {
 		return err
 	}

internal/restic/snapshot_find.go

@@ -2,8 +2,6 @@ package restic
 
 import (
 	"context"
-	"fmt"
-	"os"
 	"path/filepath"
 	"time"
 
@@ -90,28 +88,64 @@ func FindSnapshot(ctx context.Context, be Lister, s string) (ID, error) {
 	return ParseID(name)
 }
 
-// FindFilteredSnapshots yields Snapshots filtered from the list of all
-// snapshots.
-func FindFilteredSnapshots(ctx context.Context, be Lister, loader LoaderUnpacked, hosts []string, tags []TagList, paths []string) (Snapshots, error) {
-	results := make(Snapshots, 0, 20)
+type SnapshotFindCb func(string, *Snapshot, error) error
 
-	err := ForAllSnapshots(ctx, be, loader, nil, func(id ID, sn *Snapshot, err error) error {
+// FindFilteredSnapshots yields Snapshots, either given explicitly by `snapshotIDs` or filtered from the list of all snapshots.
+func FindFilteredSnapshots(ctx context.Context, be Lister, loader LoaderUnpacked, hosts []string, tags []TagList, paths []string, snapshotIDs []string, fn SnapshotFindCb) error {
+	if len(snapshotIDs) != 0 {
+		var err error
+		usedFilter := false
+
+		ids := NewIDSet()
+		// Process all snapshot IDs given as arguments.
+		for _, s := range snapshotIDs {
+			var id ID
+			if s == "latest" {
+				if usedFilter {
+					continue
+				}
+
+				usedFilter = true
+
+				id, err = FindLatestSnapshot(ctx, be, loader, paths, tags, hosts, nil)
+				if err == ErrNoSnapshotFound {
+					err = errors.Errorf("no snapshot matched given filter (Paths:%v Tags:%v Hosts:%v)", paths, tags, hosts)
+				}
+			} else {
+				id, err = FindSnapshot(ctx, be, s)
+			}
+
+			var sn *Snapshot
+			if ids.Has(id) {
+				continue
+			} else if !id.IsNull() {
+				ids.Insert(id)
+				sn, err = LoadSnapshot(ctx, loader, id)
+				s = id.String()
+			}
+
+			err = fn(s, sn, err)
 			if err != nil {
-			fmt.Fprintf(os.Stderr, "could not load snapshot %v: %v\n", id.Str(), err)
+				return err
+			}
+		}
+
+		// Give the user some indication their filters are not used.
+		if !usedFilter && (len(hosts) != 0 || len(tags) != 0 || len(paths) != 0) {
+			return fn("filters", nil, errors.Errorf("explicit snapshot ids are given"))
+		}
 		return nil
 	}
 
+	return ForAllSnapshots(ctx, be, loader, nil, func(id ID, sn *Snapshot, err error) error {
+		if err != nil {
+			return fn(id.String(), sn, err)
+		}
+
 		if !sn.HasHostname(hosts) || !sn.HasTagList(tags) || !sn.HasPaths(paths) {
 			return nil
 		}
 
-		results = append(results, sn)
-		return nil
+		return fn(id.String(), sn, err)
 	})
-
-	if err != nil {
-		return nil, err
-	}
-
-	return results, nil
 }