From f2fafbffaa5835492e45a474229de450223e0b70 Mon Sep 17 00:00:00 2001 From: Michael Eischer Date: Sat, 30 Dec 2023 22:39:26 +0100 Subject: [PATCH] restore: only report errors for blobs that actually failed to load Previously, errors would be reported for all blobs of a packfile that failed to stream. Now, only the not yet processed blobs are reported. --- internal/restorer/filerestorer.go | 17 +++++++++- internal/restorer/filerestorer_test.go | 44 ++++++++++++++++++++++++++ 2 files changed, 60 insertions(+), 1 deletion(-) diff --git a/internal/restorer/filerestorer.go b/internal/restorer/filerestorer.go index 3bb7489ba..1fc74c7f0 100644 --- a/internal/restorer/filerestorer.go +++ b/internal/restorer/filerestorer.go @@ -246,7 +246,10 @@ func (r *fileRestorer) downloadPack(ctx context.Context, pack *packInfo) error { return err } + // track already processed blobs for precise error reporting + processedBlobs := restic.NewBlobSet() err := repository.StreamPack(ctx, r.packLoader, r.key, pack.id, blobList, func(h restic.BlobHandle, blobData []byte, err error) error { + processedBlobs.Insert(h) blob := blobs[h.ID] if err != nil { for file := range blob.files { @@ -292,7 +295,19 @@ func (r *fileRestorer) downloadPack(ctx context.Context, pack *packInfo) error { }) if err != nil { - for file := range pack.files { + // only report error for not yet processed blobs + affectedFiles := make(map[*fileInfo]struct{}) + for _, blob := range blobList { + if processedBlobs.Has(blob.BlobHandle) { + continue + } + blob := blobs[blob.ID] + for file := range blob.files { + affectedFiles[file] = struct{}{} + } + } + + for file := range affectedFiles { if errFile := sanitizeError(file, err); errFile != nil { return errFile } diff --git a/internal/restorer/filerestorer_test.go b/internal/restorer/filerestorer_test.go index e798f2b8b..7d35da19c 100644 --- a/internal/restorer/filerestorer_test.go +++ b/internal/restorer/filerestorer_test.go @@ -316,3 +316,47 @@ func testPartialDownloadError(t *testing.T, part int) { rtest.OK(t, err) verifyRestore(t, r, repo) } + +func TestFatalDownloadError(t *testing.T) { + tempdir := rtest.TempDir(t) + content := []TestFile{ + { + name: "file1", + blobs: []TestBlob{ + {"data1-1", "pack1"}, + {"data1-2", "pack1"}, + }, + }, + { + name: "file2", + blobs: []TestBlob{ + {"data2-1", "pack1"}, + {"data2-2", "pack1"}, + {"data2-3", "pack1"}, + }, + }} + + repo := newTestRepo(content) + + loader := repo.loader + repo.loader = func(ctx context.Context, h restic.Handle, length int, offset int64, fn func(rd io.Reader) error) error { + // only return half the data to break file2 + return loader(ctx, h, length/2, offset, fn) + } + + r := newFileRestorer(tempdir, repo.loader, repo.key, repo.Lookup, 2, false, nil) + r.files = repo.files + + var errors []string + r.Error = func(s string, e error) error { + // ignore errors as in the `restore` command + errors = append(errors, s) + return nil + } + + err := r.restoreFiles(context.TODO()) + rtest.OK(t, err) + + rtest.Assert(t, len(errors) == 1, "unexpected number of restore errors, expected: 1, got: %v", len(errors)) + rtest.Assert(t, errors[0] == "file2", "expected error for file2, got: %v", errors[0]) +}