From a5e814bd8d4f8de12aca71bd33f89a43005d5f01 Mon Sep 17 00:00:00 2001 From: Michael Eischer Date: Sun, 7 Sep 2025 17:13:19 +0200 Subject: [PATCH] check: fix error reporting on download retry --- changelog/unreleased/issue-5467 | 11 ++++++++++ internal/checker/checker_test.go | 37 +++++++++++++++++++++++++++----- internal/repository/check.go | 7 +++++- 3 files changed, 49 insertions(+), 6 deletions(-) create mode 100644 changelog/unreleased/issue-5467 diff --git a/changelog/unreleased/issue-5467 b/changelog/unreleased/issue-5467 new file mode 100644 index 000000000..f6dbd9309 --- /dev/null +++ b/changelog/unreleased/issue-5467 @@ -0,0 +1,11 @@ +Bugfix: Improve handling of download retries in `check` command + +In very rare cases, the `check` command could unnecessarily report a +repository damage if the backend returns incomplete, corrupted data on +the first download try which is resolved by a download retry. + +Now, only the check result of the last download attempt is reported as +intended. + +https://github.com/restic/restic/issues/5467 +https://github.com/restic/restic/pull/5495 diff --git a/internal/checker/checker_test.go b/internal/checker/checker_test.go index 92bbb1da6..64f9b61ff 100644 --- a/internal/checker/checker_test.go +++ b/internal/checker/checker_test.go @@ -293,7 +293,7 @@ type errorBackend struct { func (b errorBackend) Load(ctx context.Context, h backend.Handle, length int, offset int64, consumer func(rd io.Reader) error) error { return b.Backend.Load(ctx, h, length, offset, func(rd io.Reader) error { if b.ProduceErrors { - return consumer(errorReadCloser{rd}) + return consumer(errorReadCloser{Reader: rd}) } return consumer(rd) }) @@ -301,12 +301,21 @@ func (b errorBackend) Load(ctx context.Context, h backend.Handle, length int, of type errorReadCloser struct { io.Reader + shortenBy int + maxErrorOffset int // if 0, the error can be injected at any offset } func (erd errorReadCloser) Read(p []byte) (int, error) { n, err := erd.Reader.Read(p) if n > 0 { - induceError(p[:n]) + maxOffset := n + if erd.maxErrorOffset > 0 { + maxOffset = min(erd.maxErrorOffset, maxOffset) + } + induceError(p[:maxOffset]) + } + if n > erd.shortenBy { + n -= erd.shortenBy } return n, err } @@ -320,17 +329,26 @@ func induceError(data []byte) { // errorOnceBackend randomly modifies data when reading a file for the first time. type errorOnceBackend struct { backend.Backend - m sync.Map + m sync.Map + shortenBy int + maxErrorOffset int } func (b *errorOnceBackend) Load(ctx context.Context, h backend.Handle, length int, offset int64, consumer func(rd io.Reader) error) error { _, isRetry := b.m.LoadOrStore(h, struct{}{}) - return b.Backend.Load(ctx, h, length, offset, func(rd io.Reader) error { + err := b.Backend.Load(ctx, h, length, offset, func(rd io.Reader) error { if !isRetry && h.Type != restic.ConfigFile { - return consumer(errorReadCloser{rd}) + return consumer(errorReadCloser{Reader: rd, shortenBy: b.shortenBy, maxErrorOffset: b.maxErrorOffset}) } return consumer(rd) }) + if err == nil { + return nil + } + // retry if the consumer returned an error + return b.Backend.Load(ctx, h, length, offset, func(rd io.Reader) error { + return consumer(rd) + }) } func TestCheckerModifiedData(t *testing.T) { @@ -368,6 +386,15 @@ func TestCheckerModifiedData(t *testing.T) { } }, }, + { + // ignore if a backend returns incomplete garbled data on the first try + "corruptPartialOnceBackend", + &errorOnceBackend{Backend: be, shortenBy: 10, maxErrorOffset: 100}, + func() {}, + func(t *testing.T, err error) { + test.Assert(t, err == nil, "unexpected error found, got %v", err) + }, + }, } { t.Run(test.name, func(t *testing.T) { checkRepo := repository.TestOpenBackend(t, test.be) diff --git a/internal/repository/check.go b/internal/repository/check.go index 2bf2ac8f3..477c72a97 100644 --- a/internal/repository/check.go +++ b/internal/repository/check.go @@ -88,10 +88,14 @@ func checkPackInner(ctx context.Context, r *Repository, id restic.ID, blobs []re // calculate hash on-the-fly while reading the pack and capture pack header var hash restic.ID var hdrBuf []byte + // must use a separate slice from `errs` here as we're only interested in the last retry + var blobErrors []error h := backend.Handle{Type: backend.PackFile, Name: id.String()} err := r.be.Load(ctx, h, int(size), 0, func(rd io.Reader) error { hrd := hashing.NewReader(rd, sha256.New()) bufRd.Reset(hrd) + // reset blob errors for each retry + blobErrors = nil it := newPackBlobIterator(id, newBufReader(bufRd), 0, blobs, r.Key(), dec) for { @@ -108,7 +112,7 @@ func checkPackInner(ctx context.Context, r *Repository, id restic.ID, blobs []re debug.Log(" check blob %v: %v", val.Handle.ID, val.Handle) if val.Err != nil { debug.Log(" error verifying blob %v: %v", val.Handle.ID, val.Err) - errs = append(errs, errors.Errorf("blob %v: %v", val.Handle.ID, val.Err)) + blobErrors = append(blobErrors, errors.Errorf("blob %v: %v", val.Handle.ID, val.Err)) } } @@ -134,6 +138,7 @@ func checkPackInner(ctx context.Context, r *Repository, id restic.ID, blobs []re hash = restic.IDFromHash(hrd.Sum(nil)) return nil }) + errs = append(errs, blobErrors...) if err != nil { var e *partialReadError isPartialReadError := errors.As(err, &e)