From 25a5aa3520696eea3f129e8ad5d94ebc7705352e Mon Sep 17 00:00:00 2001 From: Michael Eischer Date: Sat, 22 Nov 2025 19:46:39 +0100 Subject: [PATCH] dump: fix missing error handling if tree cannot be read --- internal/dump/common.go | 37 ++++++++++++++++++++---------------- internal/dump/common_test.go | 27 ++++++++++++++++++++++---- 2 files changed, 44 insertions(+), 20 deletions(-) diff --git a/internal/dump/common.go b/internal/dump/common.go index aea5c1291..1a6a9c2b2 100644 --- a/internal/dump/common.go +++ b/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 { diff --git a/internal/dump/common_test.go b/internal/dump/common_test.go index 5599e2717..bb0347189 100644 --- a/internal/dump/common_test.go +++ b/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") }) } }