dump: fix missing error handling if tree cannot be read

internal/dump/common.go

@@ -31,32 +31,37 @@ func New(format string, repo restic.Loader, w io.Writer) *Dumper {
 }
 
 func (d *Dumper) DumpTree(ctx context.Context, tree *data.Tree, rootPath string) error {
-	ctx, cancel := context.WithCancel(ctx)
-	defer cancel()
+	wg, ctx := errgroup.WithContext(ctx)
 
 	// ch is buffered to deal with variable download/write speeds.
 	ch := make(chan *data.Node, 10)
-	go sendTrees(ctx, d.repo, tree, rootPath, ch)
+	wg.Go(func() error {
+		return sendTrees(ctx, d.repo, tree, rootPath, ch)
+	})
 
-	switch d.format {
-	case "tar":
-		return d.dumpTar(ctx, ch)
-	case "zip":
-		return d.dumpZip(ctx, ch)
-	default:
-		panic("unknown dump format")
-	}
+	wg.Go(func() error {
+		switch d.format {
+		case "tar":
+			return d.dumpTar(ctx, ch)
+		case "zip":
+			return d.dumpZip(ctx, ch)
+		default:
+			panic("unknown dump format")
+		}
+	})
+	return wg.Wait()
 }
 
-func sendTrees(ctx context.Context, repo restic.BlobLoader, tree *data.Tree, rootPath string, ch chan *data.Node) {
+func sendTrees(ctx context.Context, repo restic.BlobLoader, tree *data.Tree, rootPath string, ch chan *data.Node) error {
 	defer close(ch)
 
-	for _, root := range tree.Nodes {
-		root.Path = path.Join(rootPath, root.Name)
-		if sendNodes(ctx, repo, root, ch) != nil {
-			break
+	for _, node := range tree.Nodes {
+		node.Path = path.Join(rootPath, node.Name)
+		if err := sendNodes(ctx, repo, node, ch); err != nil {
+			return err
 		}
 	}
+	return nil
 }
 
 func sendNodes(ctx context.Context, repo restic.BlobLoader, root *data.Node, ch chan *data.Node) error {

internal/dump/common_test.go

@@ -6,6 +6,7 @@ import (
 	"testing"
 
 	"github.com/restic/restic/internal/archiver"
+	"github.com/restic/restic/internal/backend"
 	"github.com/restic/restic/internal/data"
 	"github.com/restic/restic/internal/fs"
 	"github.com/restic/restic/internal/repository"
@@ -13,13 +14,13 @@ import (
 	rtest "github.com/restic/restic/internal/test"
 )
 
-func prepareTempdirRepoSrc(t testing.TB, src archiver.TestDir) (string, restic.Repository) {
+func prepareTempdirRepoSrc(t testing.TB, src archiver.TestDir) (string, restic.Repository, backend.Backend) {
 	tempdir := rtest.TempDir(t)
-	repo := repository.TestRepository(t)
+	repo, _, be := repository.TestRepositoryWithVersion(t, 0)
 
 	archiver.TestCreateFiles(t, tempdir, src)
 
-	return tempdir, repo
+	return tempdir, repo, be
 }
 
 type CheckDump func(t *testing.T, testDir string, testDump *bytes.Buffer) error
@@ -67,13 +68,22 @@ func WriteTest(t *testing.T, format string, cd CheckDump) {
 			},
 			target: "/",
 		},
+		{
+			name: "directory only",
+			args: archiver.TestDir{
+				"firstDir": archiver.TestDir{
+					"secondDir": archiver.TestDir{},
+				},
+			},
+			target: "/",
+		},
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
 			ctx, cancel := context.WithCancel(context.Background())
 			defer cancel()
 
-			tmpdir, repo := prepareTempdirRepoSrc(t, tt.args)
+			tmpdir, repo, be := prepareTempdirRepoSrc(t, tt.args)
 			arch := archiver.New(repo, fs.Track{FS: fs.Local{}}, archiver.Options{})
 
 			back := rtest.Chdir(t, tmpdir)
@@ -93,6 +103,15 @@ func WriteTest(t *testing.T, format string, cd CheckDump) {
 			if err := cd(t, tmpdir, dst); err != nil {
 				t.Errorf("WriteDump() = does not match: %v", err)
 			}
+
+			// test that dump returns an error if the repository is broken
+			tree, err = data.LoadTree(ctx, repo, *sn.Tree)
+			rtest.OK(t, err)
+			rtest.OK(t, be.Delete(ctx))
+			// use new dumper as the old one has the blobs cached
+			d = New(format, repo, dst)
+			err = d.DumpTree(ctx, tree, tt.target)
+			rtest.Assert(t, err != nil, "expected error, got nil")
 		})
 	}
 }