Merge pull request #4882 from MichaelEischer/improve-check-output

check: improve output if repository is damaged

changelog/unreleased/issue-828

@@ -1,11 +1,12 @@
 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
+truncated pack files. The `check` and `check --read-data` command will provide
-on using the command if necessary to repair a repository. See the guide at
+instructions on using the command if necessary to repair a repository. See the
-https://restic.readthedocs.io/en/stable/077_troubleshooting.html for further
+guide at https://restic.readthedocs.io/en/stable/077_troubleshooting.html for
-instructions.
+further instructions.
 
 https://github.com/restic/restic/issues/828
 https://github.com/restic/restic/pull/4644
 https://github.com/restic/restic/pull/4655
+https://github.com/restic/restic/pull/4882

cmd/restic/cmd_check.go

@@ -264,7 +264,7 @@ func runCheck(ctx context.Context, opts CheckOptions, gopts GlobalOptions, args
 		term.Print("Duplicate packs are non-critical, you can run `restic repair index' to correct this.\n")
 	}
 	if suggestLegacyIndexRebuild {
-		printer.E("Found indexes using the legacy format, you must run `restic repair index' to correct this.\n")
+		printer.E("error: Found indexes using the legacy format, you must run `restic repair index' to correct this.\n")
 	}
 	if mixedFound {
 		term.Print("Mixed packs with tree and data blobs are non-critical, you can run `restic prune` to correct this.\n")
@@ -274,28 +274,42 @@ func runCheck(ctx context.Context, opts CheckOptions, gopts GlobalOptions, args
 		for _, err := range errs {
 			printer.E("error: %v\n", err)
 		}
-		return errors.Fatal("LoadIndex returned errors")
+
+		printer.E("\nThe repository index is damaged and must be repaired. You must run `restic repair index' to correct this.\n\n")
+		return errors.Fatal("repository contains errors")
 	}
 
 	orphanedPacks := 0
 	errChan := make(chan error)
+	salvagePacks := restic.NewIDSet()
 
 	printer.P("check all packs\n")
 	go chkr.Packs(ctx, errChan)
 
 	for err := range errChan {
-		if checker.IsOrphanedPack(err) {
+		var packErr *checker.PackError
-			orphanedPacks++
+		if errors.As(err, &packErr) {
-			printer.P("%v\n", err)
+			if packErr.Orphaned {
+				orphanedPacks++
+				printer.V("%v\n", err)
+			} else {
+				if packErr.Truncated {
+					salvagePacks.Insert(packErr.ID)
+				}
+				errorsFound = true
+				printer.E("%v\n", err)
+			}
 		} else if err == checker.ErrLegacyLayout {
-			printer.P("repository still uses the S3 legacy layout\nPlease run `restic migrate s3legacy` to correct this.\n")
+			errorsFound = true
+			printer.E("error: repository still uses the S3 legacy layout\nYou must run `restic migrate s3legacy` to correct this.\n")
 		} else {
 			errorsFound = true
 			printer.E("%v\n", err)
 		}
 	}
 
-	if orphanedPacks > 0 {
+	if orphanedPacks > 0 && !errorsFound {
+		// hide notice if repository is damaged
 		printer.P("%d additional files were found in the repo, which likely contain duplicate data.\nThis is non-critical, you can run `restic prune` to correct this.\n", orphanedPacks)
 	}
 	if ctx.Err() != nil {
@@ -353,26 +367,14 @@ func runCheck(ctx context.Context, opts CheckOptions, gopts GlobalOptions, args
 
 		go chkr.ReadPacks(ctx, packs, p, errChan)
 
-		var salvagePacks restic.IDs
-
 		for err := range errChan {
 			errorsFound = true
 			printer.E("%v\n", err)
 			if err, ok := err.(*repository.ErrPackData); ok {
-				salvagePacks = append(salvagePacks, err.PackID)
+				salvagePacks.Insert(err.PackID)
 			}
 		}
 		p.Done()
-
-		if len(salvagePacks) > 0 {
-			printer.E("\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())
-			}
-			printer.E("restic repair packs %v\nrestic repair snapshots --forget\n\n", strings.Join(strIDs, " "))
-			printer.E("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")
-		}
 	}
 
 	switch {
@@ -416,11 +418,24 @@ func runCheck(ctx context.Context, opts CheckOptions, gopts GlobalOptions, args
 		doReadData(packs)
 	}
 
+	if len(salvagePacks) > 0 {
+		printer.E("\nThe repository contains damaged pack files. These damaged files 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())
+		}
+		printer.E("restic repair packs %v\nrestic repair snapshots --forget\n\n", strings.Join(strIDs, " "))
+		printer.E("Damaged pack files can be caused by backend problems, hardware problems or bugs in restic. Please open an issue at https://github.com/restic/restic/issues/new/choose for further troubleshooting!\n")
+	}
+
 	if ctx.Err() != nil {
 		return ctx.Err()
 	}
 
 	if errorsFound {
+		if len(salvagePacks) == 0 {
+			printer.E("\nThe repository is damaged and must be repaired. Please follow the troubleshooting guide at https://restic.readthedocs.io/en/stable/077_troubleshooting.html .\n\n")
+		}
 		return errors.Fatal("repository contains errors")
 	}
 	printer.P("no errors were found\n")

doc/045_working_with_repos.rst

@@ -368,10 +368,22 @@ detect this and yield the same error as when you tried to restore:
     $ restic -r /srv/restic-repo check
     ...
     load indexes
-    error: error loading index de30f323: load <index/de30f3231c>: invalid data returned
+    error: error loading index de30f3231ca2e6a59af4aa84216dfe2ef7339c549dc11b09b84000997b139628: LoadRaw(<index/de30f3231c>): invalid data returned
-    Fatal: LoadIndex returned errors
 
-If the repository structure is intact, restic will show that no errors were found:
+    The repository index is damaged and must be repaired. You must run `restic repair index' to correct this.
+
+    Fatal: repository contains errors
+
+.. warning::
+
+    If ``check`` reports an error in the repository, then you must repair the repository.
+    As long as a repository is damaged, restoring some files or directories will fail. New
+    snapshots are not guaranteed to be restorable either.
+
+    For instructions how to repair a damaged repository, see the :ref:`troubleshooting`
+    section or follow the instructions provided by the ``check`` command.
+
+If the repository structure is intact, restic will show that ``no errors were found``:
 
 .. code-block:: console
 

doc/077_troubleshooting.rst

@@ -10,6 +10,8 @@
   ^ for subsubsections
   " for paragraphs
 
+.. _troubleshooting:
+
 #########################
 Troubleshooting
 #########################

internal/checker/checker.go

@@ -183,22 +183,16 @@ func (c *Checker) LoadIndex(ctx context.Context, p *progress.Counter) (hints []e
 
 // PackError describes an error with a specific pack.
 type PackError struct {
-	ID       restic.ID
+	ID        restic.ID
-	Orphaned bool
+	Orphaned  bool
-	Err      error
+	Truncated bool
+	Err       error
 }
 
 func (e *PackError) Error() string {
 	return "pack " + e.ID.String() + ": " + e.Err.Error()
 }
 
-// IsOrphanedPack returns true if the error describes a pack which is not
-// contained in any index.
-func IsOrphanedPack(err error) bool {
-	var e *PackError
-	return errors.As(err, &e) && e.Orphaned
-}
-
 func isS3Legacy(b backend.Backend) bool {
 	be := backend.AsBackend[*s3.Backend](b)
 	return be != nil && be.Layout.Name() == "s3legacy"
@@ -250,7 +244,7 @@ func (c *Checker) Packs(ctx context.Context, errChan chan<- error) {
 			select {
 			case <-ctx.Done():
 				return
-			case errChan <- &PackError{ID: id, Err: errors.Errorf("unexpected file size: got %d, expected %d", reposize, size)}:
+			case errChan <- &PackError{ID: id, Truncated: true, Err: errors.Errorf("unexpected file size: got %d, expected %d", reposize, size)}:
 			}
 		}
 	}