mirror of
https://github.com/restic/restic.git
synced 2026-11-06 07:19:21 +00:00
prune: report failure to list pack files as a fatal error
Motivation: when listing pack files fails during prune (for example due
to a request timeout), the error was returned unwrapped and main printed
it with "%+v", which dumps a stack trace. As discussed in the issue, the
error needs one more level of wrapping before it can be treated as a
fatal error.
Approach: in decidePackAction, wrap an error from listing pack files
with errors.Fatalf("failed to list pack files: %v"). Errors that are
already fatal (e.g. ErrSizeNotMatching) and errors after context
cancellation are returned unchanged. The underlying error is preserved
via Unwrap. Retry log messages are unchanged; only the final error
output is shortened and says where the failure occurred.
Validation: `go test ./internal/repository/ -run Prune` and
`go test ./cmd/restic/ -run Prune` pass. The new test
TestPrunePackListError (backend whose pack list fails) fails without
the change and passes with it. I did not reproduce a real REST backend
timeout.
Report: https://github.com/restic/restic/issues/22120
Signed-off-by: Pujitha Paladugu <10557236+pujitha24@users.noreply.github.com>
Assisted-by: claude-sonnet-5-5 (via Claude Code)
This commit is contained in:
3 files changed
+49
-1
No files matched your search
@@ -0,0 +1,8 @@
|
|||||||
|
Enhancement: Show a concise error when listing pack files fails during `prune`
|
||||||
|
|
||||||
|
When listing the pack files of a repository failed during `prune`, for
|
||||||
|
example due to a request timeout, restic printed the error together with a
|
||||||
|
stack trace. Restic now reports a short error message that states that
|
||||||
|
listing the pack files failed.
|
||||||
|
|
||||||
|
https://github.com/restic/restic/issues/22120
|
||||||
@@ -467,7 +467,10 @@ func decidePackAction(ctx context.Context, opts PruneOptions, repo *Repository,
|
|||||||
|
|
||||||
bar.Done()
|
bar.Done()
|
||||||
if err != nil {
|
if err != nil {
|
||||||
return PrunePlan{}, err
|
if errors.IsFatal(err) || ctx.Err() != nil {
|
||||||
|
return PrunePlan{}, err
|
||||||
|
}
|
||||||
|
return PrunePlan{}, errors.Fatalf("failed to list pack files: %v", err)
|
||||||
}
|
}
|
||||||
|
|
||||||
// At this point indexPacks contains only missing packs!
|
// At this point indexPacks contains only missing packs!
|
||||||
|
|||||||
@@ -2,12 +2,16 @@ package repository_test
|
|||||||
|
|
||||||
import (
|
import (
|
||||||
"context"
|
"context"
|
||||||
|
"errors"
|
||||||
"fmt"
|
"fmt"
|
||||||
"math"
|
"math"
|
||||||
"math/rand"
|
"math/rand"
|
||||||
|
"strings"
|
||||||
"testing"
|
"testing"
|
||||||
"time"
|
"time"
|
||||||
|
|
||||||
|
"github.com/restic/restic/internal/backend"
|
||||||
|
resticerrors "github.com/restic/restic/internal/errors"
|
||||||
"github.com/restic/restic/internal/repository"
|
"github.com/restic/restic/internal/repository"
|
||||||
"github.com/restic/restic/internal/repository/pack"
|
"github.com/restic/restic/internal/repository/pack"
|
||||||
"github.com/restic/restic/internal/restic"
|
"github.com/restic/restic/internal/restic"
|
||||||
@@ -191,3 +195,36 @@ func TestPruneSmall(t *testing.T) {
|
|||||||
rtest.Equals(t, lenPackfilesBefore > lenPackfilesAfter, true,
|
rtest.Equals(t, lenPackfilesBefore > lenPackfilesAfter, true,
|
||||||
fmt.Sprintf("the number packfiles before %d and after repack %d", lenPackfilesBefore, lenPackfilesAfter))
|
fmt.Sprintf("the number packfiles before %d and after repack %d", lenPackfilesBefore, lenPackfilesAfter))
|
||||||
}
|
}
|
||||||
|
|
||||||
|
type failingListBackend struct {
|
||||||
|
backend.Backend
|
||||||
|
err error
|
||||||
|
}
|
||||||
|
|
||||||
|
func (b *failingListBackend) List(ctx context.Context, t backend.FileType, fn func(backend.FileInfo) error) error {
|
||||||
|
if t == backend.PackFile {
|
||||||
|
return b.err
|
||||||
|
}
|
||||||
|
return b.Backend.List(ctx, t, fn)
|
||||||
|
}
|
||||||
|
|
||||||
|
func TestPrunePackListError(t *testing.T) {
|
||||||
|
random := rand.New(rand.NewSource(1))
|
||||||
|
repo, _, be := repository.TestRepositoryWithVersion(t, 0)
|
||||||
|
createRandomBlobs(t, random, repo, 2, 0.5, true)
|
||||||
|
|
||||||
|
listErr := errors.New("request timeout")
|
||||||
|
repo = repository.TestOpenBackend(t, &failingListBackend{Backend: be, err: listErr})
|
||||||
|
rtest.OK(t, repo.LoadIndex(context.TODO(), restic.NoopTerminalCounterFactory))
|
||||||
|
|
||||||
|
_, err := repository.PlanPrune(context.TODO(), repository.PruneOptions{
|
||||||
|
MaxRepackBytes: math.MaxUint64,
|
||||||
|
MaxUnusedBytes: func(used uint64) uint64 { return used },
|
||||||
|
}, repo, func(ctx context.Context, repo restic.Repository, usedBlobs restic.FindBlobSet) error {
|
||||||
|
return nil
|
||||||
|
}, restic.NewNoopPrinter())
|
||||||
|
rtest.Assert(t, err != nil, "expected an error")
|
||||||
|
rtest.Assert(t, resticerrors.IsFatal(err), "expected a fatal error, got %v", err)
|
||||||
|
rtest.Assert(t, errors.Is(err, listErr), "expected error to wrap the list error, got %v", err)
|
||||||
|
rtest.Assert(t, strings.Contains(err.Error(), "failed to list pack files"), "unexpected message %q", err)
|
||||||
|
}
|
||||||
Reference in new issue
Block a user