From 200f09522dfe034ced28075705c41de9a5fefebf Mon Sep 17 00:00:00 2001 From: Alexander Neumann Date: Sat, 30 Jan 2021 17:25:10 +0100 Subject: [PATCH] Add more error checks --- cmd/restic/cmd_backup_test.go | 8 ++++---- cmd/restic/cmd_cache.go | 2 +- cmd/restic/cmd_find.go | 5 ++++- cmd/restic/cmd_ls.go | 6 ++++-- cmd/restic/cmd_recover.go | 5 ++++- cmd/restic/cmd_snapshots.go | 5 ++++- cmd/restic/cmd_snapshots_test.go | 3 ++- cmd/restic/delete.go | 2 +- cmd/restic/exclude.go | 4 +++- cmd/restic/integration_test.go | 30 +++++++++++++++++++++++------- internal/archiver/archiver.go | 17 +++++++++++++---- 11 files changed, 63 insertions(+), 24 deletions(-) diff --git a/cmd/restic/cmd_backup_test.go b/cmd/restic/cmd_backup_test.go index 90e9a5a4b..49642b872 100644 --- a/cmd/restic/cmd_backup_test.go +++ b/cmd/restic/cmd_backup_test.go @@ -32,7 +32,7 @@ func TestCollectTargets(t *testing.T) { // All mentioned files must exist for collectTargets. f, err := os.Create(filepath.Join(dir, filename)) rtest.OK(t, err) - f.Close() + rtest.OK(t, f.Close()) expect = append(expect, f.Name()) } @@ -41,7 +41,7 @@ func TestCollectTargets(t *testing.T) { rtest.OK(t, err) // Empty lines should be ignored. A line starting with '#' is a comment. fmt.Fprintf(f1, "\n%s*\n # here's a comment\n", f1.Name()) - f1.Close() + rtest.OK(t, f1.Close()) f2, err := os.Create(filepath.Join(dir, "fromfile-verbatim")) rtest.OK(t, err) @@ -49,7 +49,7 @@ func TestCollectTargets(t *testing.T) { // Empty lines should be ignored. CR+LF is allowed. fmt.Fprintf(f2, "%s\r\n\n", filepath.Join(dir, filename)) } - f2.Close() + rtest.OK(t, f2.Close()) f3, err := os.Create(filepath.Join(dir, "fromfile-raw")) rtest.OK(t, err) @@ -57,7 +57,7 @@ func TestCollectTargets(t *testing.T) { fmt.Fprintf(f3, "%s\x00", filepath.Join(dir, filename)) } rtest.OK(t, err) - f3.Close() + rtest.OK(t, f3.Close()) opts := BackupOptions{ FilesFrom: []string{f1.Name()}, diff --git a/cmd/restic/cmd_cache.go b/cmd/restic/cmd_cache.go index 89fb2c0df..1b36219eb 100644 --- a/cmd/restic/cmd_cache.go +++ b/cmd/restic/cmd_cache.go @@ -148,7 +148,7 @@ func runCache(opts CacheOptions, gopts GlobalOptions, args []string) error { }) } - tab.Write(gopts.stdout) + _ = tab.Write(gopts.stdout) Printf("%d cache dirs in %s\n", len(dirs), cachedir) return nil diff --git a/cmd/restic/cmd_find.go b/cmd/restic/cmd_find.go index 0986b5a13..378d6cbdc 100644 --- a/cmd/restic/cmd_find.go +++ b/cmd/restic/cmd_find.go @@ -563,7 +563,10 @@ func runFind(opts FindOptions, gopts GlobalOptions, args []string) error { } if opts.PackID { - f.packsToBlobs(ctx, []string{f.pat.pattern[0]}) // TODO: support multiple packs + err := f.packsToBlobs(ctx, []string{f.pat.pattern[0]}) // TODO: support multiple packs + if err != nil { + return err + } } for sn := range FindFilteredSnapshots(ctx, repo, opts.Hosts, opts.Tags, opts.Paths, opts.Snapshots) { diff --git a/cmd/restic/cmd_ls.go b/cmd/restic/cmd_ls.go index db908a72c..90106e8a5 100644 --- a/cmd/restic/cmd_ls.go +++ b/cmd/restic/cmd_ls.go @@ -159,16 +159,17 @@ func runLs(opts LsOptions, gopts GlobalOptions, args []string) error { enc := json.NewEncoder(gopts.stdout) printSnapshot = func(sn *restic.Snapshot) { - enc.Encode(lsSnapshot{ + err = enc.Encode(lsSnapshot{ Snapshot: sn, ID: sn.ID(), ShortID: sn.ID().Str(), StructType: "snapshot", }) + panic(err) } printNode = func(path string, node *restic.Node) { - enc.Encode(lsNode{ + err = enc.Encode(lsNode{ Name: node.Name, Type: node.Type, Path: path, @@ -181,6 +182,7 @@ func runLs(opts LsOptions, gopts GlobalOptions, args []string) error { ChangeTime: node.ChangeTime, StructType: "node", }) + panic(err) } } else { printSnapshot = func(sn *restic.Snapshot) { diff --git a/cmd/restic/cmd_recover.go b/cmd/restic/cmd_recover.go index 8f11b6860..c6b111d22 100644 --- a/cmd/restic/cmd_recover.go +++ b/cmd/restic/cmd_recover.go @@ -117,7 +117,10 @@ func runRecover(gopts GlobalOptions) error { ModTime: time.Now(), ChangeTime: time.Now(), } - tree.Insert(&node) + err = tree.Insert(&node) + if err != nil { + return err + } } treeID, err := repo.SaveTree(gopts.ctx, tree) diff --git a/cmd/restic/cmd_snapshots.go b/cmd/restic/cmd_snapshots.go index 300ba99ee..091861bfa 100644 --- a/cmd/restic/cmd_snapshots.go +++ b/cmd/restic/cmd_snapshots.go @@ -243,7 +243,10 @@ func PrintSnapshots(stdout io.Writer, list restic.Snapshots, reasons []restic.Ke } } - tab.Write(stdout) + err := tab.Write(stdout) + if err != nil { + Warnf("error printing: %v\n", err) + } } // PrintSnapshotGroupHeader prints which group of the group-by option the diff --git a/cmd/restic/cmd_snapshots_test.go b/cmd/restic/cmd_snapshots_test.go index 04af1a3c1..777c4272f 100644 --- a/cmd/restic/cmd_snapshots_test.go +++ b/cmd/restic/cmd_snapshots_test.go @@ -11,7 +11,8 @@ import ( func TestEmptySnapshotGroupJSON(t *testing.T) { for _, grouped := range []bool{false, true} { var w strings.Builder - printSnapshotGroupJSON(&w, nil, grouped) + err := printSnapshotGroupJSON(&w, nil, grouped) + rtest.OK(t, err) rtest.Equals(t, "[]", strings.TrimSpace(w.String())) } diff --git a/cmd/restic/delete.go b/cmd/restic/delete.go index e9b2481e0..26ca08196 100644 --- a/cmd/restic/delete.go +++ b/cmd/restic/delete.go @@ -9,7 +9,7 @@ import ( // DeleteFiles deletes the given fileList of fileType in parallel // it will print a warning if there is an error, but continue deleting the remaining files func DeleteFiles(gopts GlobalOptions, repo restic.Repository, fileList restic.IDSet, fileType restic.FileType) { - deleteFiles(gopts, true, repo, fileList, fileType) + _ = deleteFiles(gopts, true, repo, fileList, fileType) } // DeleteFilesChecked deletes the given fileList of fileType in parallel diff --git a/cmd/restic/exclude.go b/cmd/restic/exclude.go index e63c689ec..1ddd8932c 100644 --- a/cmd/restic/exclude.go +++ b/cmd/restic/exclude.go @@ -180,7 +180,9 @@ func isDirExcludedByFile(dir, tagFilename, header string) bool { Warnf("could not open exclusion tagfile: %v", err) return false } - defer f.Close() + defer func() { + _ = f.Close() + }() buf := make([]byte, len(header)) _, err = io.ReadFull(f, buf) // EOF is handled with a dedicated message, otherwise the warning were too cryptic diff --git a/cmd/restic/integration_test.go b/cmd/restic/integration_test.go index d17602d0b..1e39301f3 100644 --- a/cmd/restic/integration_test.go +++ b/cmd/restic/integration_test.go @@ -566,9 +566,9 @@ func TestBackupErrors(t *testing.T) { // Assume failure inaccessibleFile := filepath.Join(env.testdata, "0", "0", "9", "0") - os.Chmod(inaccessibleFile, 0000) + rtest.OK(t, os.Chmod(inaccessibleFile, 0000)) defer func() { - os.Chmod(inaccessibleFile, 0644) + rtest.OK(t, os.Chmod(inaccessibleFile, 0644)) }() opts := BackupOptions{} gopts := env.gopts @@ -933,7 +933,7 @@ func testRunKeyAddNewKeyUserHost(t testing.TB, gopts GlobalOptions) { keyHostname = "" }() - cmdKey.Flags().Parse([]string{"--user=john", "--host=example.com"}) + rtest.OK(t, cmdKey.Flags().Parse([]string{"--user=john", "--host=example.com"})) t.Log("adding key for john@example.com") rtest.OK(t, runKey(gopts, []string{"add"})) @@ -1106,7 +1106,7 @@ func TestRestoreLatest(t *testing.T) { testRunBackup(t, "", []string{filepath.Base(env.testdata)}, opts, env.gopts) testRunCheck(t, env.gopts) - os.Remove(p) + rtest.OK(t, os.Remove(p)) rtest.OK(t, appendRandomData(p, 101)) testRunBackup(t, "", []string{filepath.Base(env.testdata)}, opts, env.gopts) testRunCheck(t, env.gopts) @@ -1747,16 +1747,32 @@ func copyFile(dst string, src string) error { if err != nil { return err } - defer srcFile.Close() dstFile, err := os.Create(dst) if err != nil { + // ignore subsequent errors + _ = srcFile.Close() return err } - defer dstFile.Close() _, err = io.Copy(dstFile, srcFile) - return err + if err != nil { + // ignore subsequent errors + _ = srcFile.Close() + _ = dstFile.Close() + } + + err = srcFile.Close() + if err != nil { + return err + } + + err = dstFile.Close() + if err != nil { + return err + } + + return nil } var diffOutputRegexPatterns = []string{ diff --git a/internal/archiver/archiver.go b/internal/archiver/archiver.go index 2b0ac4ada..235898df0 100644 --- a/internal/archiver/archiver.go +++ b/internal/archiver/archiver.go @@ -405,7 +405,10 @@ func (arch *Archiver) Save(ctx context.Context, snPath, target string, previous debug.Log("%v hasn't changed, but contents are missing!", target) // There are contents missing - inform user! err := errors.Errorf("parts of %v not found in the repository index; storing the file again", target) - arch.error(abstarget, fi, err) + err = arch.error(abstarget, fi, err) + if err != nil { + return FutureNode{}, false, err + } } // reopen file and do an fstat() on the open file to check it is still @@ -457,7 +460,10 @@ func (arch *Archiver) Save(ctx context.Context, snPath, target string, previous start := time.Now() oldSubtree, err := arch.loadSubtree(ctx, previous) if err != nil { - arch.error(abstarget, fi, err) + err = arch.error(abstarget, fi, err) + } + if err != nil { + return FutureNode{}, false, err } fn.isTree = true @@ -592,7 +598,10 @@ func (arch *Archiver) SaveTree(ctx context.Context, snPath string, atree *Tree, oldNode := previous.Find(name) oldSubtree, err := arch.loadSubtree(ctx, oldNode) if err != nil { - arch.error(join(snPath, name), nil, err) + err = arch.error(join(snPath, name), nil, err) + } + if err != nil { + return nil, err } // not a leaf node, archive subtree @@ -751,7 +760,7 @@ func (arch *Archiver) loadParentTree(ctx context.Context, snapshotID restic.ID) tree, err := arch.Repo.LoadTree(ctx, *sn.Tree) if err != nil { debug.Log("unable to load tree %v: %v", *sn.Tree, err) - arch.error("/", nil, arch.wrapLoadTreeError(*sn.Tree, err)) + _ = arch.error("/", nil, arch.wrapLoadTreeError(*sn.Tree, err)) return nil } return tree