Misc error handling fixes (#21884)

* forget: fix error handling if snapshot key cannot be marshaled

* backend/b2: correctly close file if too short

* backend/rest: correctly close body on out of bounds read

* backend/sftp: fix file handle leak if chmod for a file fails

* backend/util: fix errorhandling in DefaultDelete test helper

* prune/repair pack: document ignored errors

* selfupdate: improve error handling

* repository: show correct error if zstd decoder cannot be initialized

cmd/restic/cmd_forget.go

@@ -262,7 +262,7 @@ func runForget(ctx context.Context, opts ForgetOptions, pruneOptions PruneOption
 			}
 
 			var key data.SnapshotGroupKey
-			if json.Unmarshal([]byte(k), &key) != nil {
+			if err := json.Unmarshal([]byte(k), &key); err != nil {
 				return err
 			}
 

internal/backend/b2/b2.go

@@ -238,6 +238,7 @@ func (be *b2Backend) Save(ctx context.Context, h backend.Handle, rd backend.Rewi
 
 	// sanity check
 	if n != rd.Length() {
+		_ = w.Close()
 		return errors.Errorf("wrote %d bytes instead of the expected %d bytes", n, rd.Length())
 	}
 	return errors.Wrap(w.Close(), "Close")

internal/backend/rest/rest.go

@@ -245,6 +245,7 @@ func (b *Backend) openReader(ctx context.Context, h backend.Handle, length int,
 	}
 
 	if feature.Flag.Enabled(feature.BackendErrorRedesign) && length > 0 && resp.ContentLength != int64(length) {
+		_ = drainAndClose(resp)
 		return nil, &restError{h, http.StatusRequestedRangeNotSatisfiable, "partial out of bounds read"}
 	}
 

internal/backend/sftp/sftp.go

@@ -363,6 +363,7 @@ func (r *SFTP) Save(_ context.Context, h backend.Handle, rd backend.RewindReader
 	if err == nil {
 		err = f.Chmod(r.Modes.File)
 		if err != nil {
+			_ = f.Close()
 			return errors.Wrapf(err, "Chmod %v", tmpFilename)
 		}
 	}

internal/backend/util/defaults.go

@@ -38,7 +38,7 @@ func DefaultDelete(ctx context.Context, be backend.Backend) error {
 			return be.Remove(ctx, backend.Handle{Type: t, Name: fi.Name})
 		})
 		if err != nil {
-			return nil
+			return err
 		}
 	}
 	err := be.Remove(ctx, backend.Handle{Type: backend.ConfigFile})

internal/repository/checker.go

@@ -271,7 +271,7 @@ func (c *Checker) ReadPacks(ctx context.Context, filter func(packs map[restic.ID
 			bufRd := bufio.NewReaderSize(nil, maxStreamBufferSize)
 			dec, err := zstd.NewReader(nil)
 			if err != nil {
-				panic(dec)
+				panic(err)
 			}
 			defer dec.Close()
 			for {

internal/repository/prune.go

@@ -611,6 +611,7 @@ func (plan *PrunePlan) Execute(ctx context.Context, printer progress.Printer) er
 	// unreferenced packs can be safely deleted first
 	if len(plan.removePacksFirst) != 0 {
 		printer.P("deleting unreferenced packs\n")
+		// ignoring errors is fine here as keeping too many packs cannot damage the repository
 		_ = deleteFiles(ctx, true, &internalRepository{repo}, plan.removePacksFirst, restic.PackFile, printer)
 		// forget unused data
 		plan.removePacksFirst = nil
@@ -668,6 +669,7 @@ func (plan *PrunePlan) Execute(ctx context.Context, printer progress.Printer) er
 
 	if len(plan.removePacks) != 0 {
 		printer.P("removing %d old packs", len(plan.removePacks))
+		// ignoring errors is fine here as keeping too many packs cannot damage the repository
 		_ = deleteFiles(ctx, true, &internalRepository{repo}, plan.removePacks, restic.PackFile, printer)
 	}
 	if ctx.Err() != nil {

internal/repository/repair_pack.go

@@ -79,6 +79,7 @@ func resolveBlobsForPacks(ctx context.Context, repo *Repository, ids restic.IDSe
 		if ids.Has(id) {
 			blobs, err := repo.listPack(ctx, id, size)
 			if err != nil {
+				// ignore errors for broken pack files to be able to salvage as much as possible
 				return nil
 			}
 			packToBlobs[id] = blobs

internal/selfupdate/download.go

@@ -80,9 +80,12 @@ func extractToFile(buf []byte, filename, target string, printf func(string, ...i
 		return err
 	}
 	if err = newFile.Sync(); err != nil {
+		_ = newFile.Close()
+		_ = os.Remove(newFile.Name())
 		return err
 	}
 	if err = newFile.Close(); err != nil {
+		_ = os.Remove(newFile.Name())
 		return err
 	}
 

internal/selfupdate/github.go

@@ -87,6 +87,7 @@ func GitHubLatestRelease(ctx context.Context, owner, repo string) (Release, erro
 			var msg githubError
 			jerr := json.NewDecoder(res.Body).Decode(&msg)
 			if jerr == nil {
+				_ = res.Body.Close()
 				return Release{}, fmt.Errorf("unexpected status %v (%v) returned, message:\n  %v", res.StatusCode, res.Status, msg.Message)
 			}
 		}
@@ -137,6 +138,7 @@ func getGithubData(ctx context.Context, url string) ([]byte, error) {
 	}
 
 	if res.StatusCode != http.StatusOK {
+		_ = res.Body.Close()
 		return nil, fmt.Errorf("unexpected status %v (%v) returned", res.StatusCode, res.Status)
 	}