Merge pull request #4665 from MichaelEischer/check-repair-packs

check: Suggest usage of `repair packs` if pack files are damaged

changelog/unreleased/pull-4644

@@ -0,0 +1,10 @@
+Enhancement: Improve `repair packs` command
+
+The `repair packs` command has been improved to also be able to process
+truncated pack files. The `check --read-data` command will provide instructions
+on using the command if necessary to repair a repository. See the guide at
+https://restic.readthedocs.io/en/stable/077_troubleshooting.html for further
+instructions.
+
+https://github.com/restic/restic/pull/4644
+https://github.com/restic/restic/pull/4655

cmd/restic/cmd_check.go

@@ -336,20 +336,18 @@ func runCheck(ctx context.Context, opts CheckOptions, gopts GlobalOptions, args
 			errorsFound = true
 			Warnf("%v\n", err)
 			if err, ok := err.(*checker.ErrPackData); ok {
-				if strings.Contains(err.Error(), "wrong data returned, hash is") {
-					salvagePacks = append(salvagePacks, err.PackID)
-				}
+				salvagePacks = append(salvagePacks, err.PackID)
 			}
 		}
 		p.Done()
 
 		if len(salvagePacks) > 0 {
-			Warnf("\nThe repository contains pack files with damaged blobs. These blobs must be removed to repair the repository. This can be done using the following commands:\n\n")
+			Warnf("\nThe repository contains pack files with damaged blobs. These blobs must be removed to repair the repository. This can be done using the following commands. Please read the troubleshooting guide at https://restic.readthedocs.io/en/stable/077_troubleshooting.html first.\n\n")
 			var strIDs []string
 			for _, id := range salvagePacks {
 				strIDs = append(strIDs, id.String())
 			}
-			Warnf("RESTIC_FEATURES=repair-packs-v1 restic repair packs %v\nrestic repair snapshots --forget\n\n", strings.Join(strIDs, " "))
+			Warnf("restic repair packs %v\nrestic repair snapshots --forget\n\n", strings.Join(strIDs, " "))
 			Warnf("Corrupted blobs are either caused by hardware problems or bugs in restic. Please open an issue at https://github.com/restic/restic/issues/new/choose for further troubleshooting!\n")
 		}
 	}

cmd/restic/cmd_repair_packs.go

@@ -40,13 +40,6 @@ func init() {
 }
 
 func runRepairPacks(ctx context.Context, gopts GlobalOptions, term *termstatus.Terminal, args []string) error {
-	// FIXME discuss and add proper feature flag mechanism
-	flag, _ := os.LookupEnv("RESTIC_FEATURES")
-	if flag != "repair-packs-v1" {
-		return errors.Fatal("This command is experimental and may change/be removed without notice between restic versions. " +
-			"Set the environment variable 'RESTIC_FEATURES=repair-packs-v1' to enable it.")
-	}
-
 	ids := restic.NewIDSet()
 	for _, arg := range args {
 		id, err := restic.ParseID(arg)

doc/077_troubleshooting.rst

@@ -76,6 +76,10 @@ Similarly, if a repository is repeatedly damaged, please open an `issue on Githu
 somewhere. Please include the check output and additional information that might
 help locate the problem.
 
+If ``check`` detects damaged pack files, it will show instructions on how to repair
+them using the ``repair pack`` command. Use that command instead of the "Repair the
+index" section in this guide.
+
 
 2. Backup the repository
 ************************
@@ -104,6 +108,11 @@ whether your issue is already known and solved. Please take a look at the
 3. Repair the index
 *******************
 
+.. note::
+
+  If the `check` command tells you to run `restic repair pack`, then use that
+  command instead. It will repair the damaged pack files and also update the index.
+
 Restic relies on its index to contain correct information about what data is
 stored in the repository. Thus, the first step to repair a repository is to
 repair the index:

internal/checker/checker.go

@@ -516,12 +516,20 @@ func (c *Checker) GetPacks() map[restic.ID]int64 {
 	return c.packs
 }
 
+type partialReadError struct {
+	err error
+}
+
+func (e *partialReadError) Error() string {
+	return e.err.Error()
+}
+
 // checkPack reads a pack and checks the integrity of all blobs.
 func checkPack(ctx context.Context, r restic.Repository, id restic.ID, blobs []restic.Blob, size int64, bufRd *bufio.Reader, dec *zstd.Decoder) error {
 	debug.Log("checking pack %v", id.String())
 
 	if len(blobs) == 0 {
-		return errors.Errorf("pack %v is empty or not indexed", id)
+		return &ErrPackData{PackID: id, errs: []error{errors.New("pack is empty or not indexed")}}
 	}
 
 	// sanity check blobs in index
@@ -542,7 +550,7 @@ func checkPack(ctx context.Context, r restic.Repository, id restic.ID, blobs []r
 	var errs []error
 	if nonContinuousPack {
 		debug.Log("Index for pack contains gaps / overlaps, blobs: %v", blobs)
-		errs = append(errs, errors.New("Index for pack contains gaps / overlapping blobs"))
+		errs = append(errs, errors.New("index for pack contains gaps / overlapping blobs"))
 	}
 
 	// calculate hash on-the-fly while reading the pack and capture pack header
@@ -559,12 +567,12 @@ func checkPack(ctx context.Context, r restic.Repository, id restic.ID, blobs []r
 			if err == repository.ErrPackEOF {
 				break
 			} else if err != nil {
-				return err
+				return &partialReadError{err}
 			}
 			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, err)
-				errs = append(errs, errors.Errorf("blob %v: %v", val.Handle.ID, err))
+				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))
 			}
 		}
 
@@ -574,7 +582,7 @@ func checkPack(ctx context.Context, r restic.Repository, id restic.ID, blobs []r
 		if minHdrStart > curPos {
 			_, err := bufRd.Discard(minHdrStart - curPos)
 			if err != nil {
-				return err
+				return &partialReadError{err}
 			}
 		}
 
@@ -582,30 +590,38 @@ func checkPack(ctx context.Context, r restic.Repository, id restic.ID, blobs []r
 		var err error
 		hdrBuf, err = io.ReadAll(bufRd)
 		if err != nil {
-			return err
+			return &partialReadError{err}
 		}
 
 		hash = restic.IDFromHash(hrd.Sum(nil))
 		return nil
 	})
 	if err != nil {
+		var e *partialReadError
+		isPartialReadError := errors.As(err, &e)
 		// failed to load the pack file, return as further checks cannot succeed anyways
-		debug.Log("  error streaming pack: %v", err)
-		return errors.Errorf("pack %v failed to download: %v", id, err)
+		debug.Log("  error streaming pack (partial %v): %v", isPartialReadError, err)
+		if isPartialReadError {
+			return &ErrPackData{PackID: id, errs: append(errs, errors.Errorf("partial download error: %w", err))}
+		}
+
+		// The check command suggests to repair files for which a `ErrPackData` is returned. However, this file
+		// completely failed to download such that there's no point in repairing anything.
+		return errors.Errorf("download error: %w", err)
 	}
 	if !hash.Equal(id) {
-		debug.Log("Pack ID does not match, want %v, got %v", id, hash)
-		return errors.Errorf("Pack ID does not match, want %v, got %v", id, hash)
+		debug.Log("pack ID does not match, want %v, got %v", id, hash)
+		return &ErrPackData{PackID: id, errs: append(errs, errors.Errorf("unexpected pack id %v", hash))}
 	}
 
 	blobs, hdrSize, err := pack.List(r.Key(), bytes.NewReader(hdrBuf), int64(len(hdrBuf)))
 	if err != nil {
-		return err
+		return &ErrPackData{PackID: id, errs: append(errs, err)}
 	}
 
 	if uint32(idxHdrSize) != hdrSize {
 		debug.Log("Pack header size does not match, want %v, got %v", idxHdrSize, hdrSize)
-		errs = append(errs, errors.Errorf("Pack header size does not match, want %v, got %v", idxHdrSize, hdrSize))
+		errs = append(errs, errors.Errorf("pack header size does not match, want %v, got %v", idxHdrSize, hdrSize))
 	}
 
 	idx := r.Index()
@@ -619,7 +635,7 @@ func checkPack(ctx context.Context, r restic.Repository, id restic.ID, blobs []r
 			}
 		}
 		if !idxHas {
-			errs = append(errs, errors.Errorf("Blob %v is not contained in index or position is incorrect", blob.ID))
+			errs = append(errs, errors.Errorf("blob %v is not contained in index or position is incorrect", blob.ID))
 			continue
 		}
 	}