From f9850b79b520537634c9b5db87370490822624d5 Mon Sep 17 00:00:00 2001 From: Michael Eischer Date: Sat, 13 May 2023 22:19:19 +0200 Subject: [PATCH 1/3] rest/sftp: Remove redundant fatal from error message This caused restic to exit with error messages like `Fatal: parsing repository location failed: Fatal: sftp path [...]` `Fatal: create repository at rest:http://localhost:12345/ failed: Fatal: config file already exists` --- internal/backend/rest/rest.go | 4 ++-- internal/backend/sftp/config.go | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/internal/backend/rest/rest.go b/internal/backend/rest/rest.go index 7be5a07c7..642fc8b4a 100644 --- a/internal/backend/rest/rest.go +++ b/internal/backend/rest/rest.go @@ -62,7 +62,7 @@ func Create(ctx context.Context, cfg Config, rt http.RoundTripper) (*Backend, er _, err = be.Stat(ctx, restic.Handle{Type: restic.ConfigFile}) if err == nil { - return nil, errors.Fatal("config file already exists") + return nil, errors.New("config file already exists") } url := *cfg.URL @@ -76,7 +76,7 @@ func Create(ctx context.Context, cfg Config, rt http.RoundTripper) (*Backend, er } if resp.StatusCode != http.StatusOK { - return nil, errors.Fatalf("server response unexpected: %v (%v)", resp.Status, resp.StatusCode) + return nil, fmt.Errorf("server response unexpected: %v (%v)", resp.Status, resp.StatusCode) } _, err = io.Copy(io.Discard, resp.Body) diff --git a/internal/backend/sftp/config.go b/internal/backend/sftp/config.go index 76d6d145d..1a7309de3 100644 --- a/internal/backend/sftp/config.go +++ b/internal/backend/sftp/config.go @@ -80,7 +80,7 @@ func ParseConfig(s string) (interface{}, error) { p := path.Clean(dir) if strings.HasPrefix(p, "~") { - return nil, errors.Fatal("sftp path starts with the tilde (~) character, that fails for most sftp servers.\nUse a relative directory, most servers interpret this as relative to the user's home directory.") + return nil, errors.New("sftp path starts with the tilde (~) character, that fails for most sftp servers.\nUse a relative directory, most servers interpret this as relative to the user's home directory") } cfg := NewConfig() From a013014c247ff5e78abbd5844c952483075a37d0 Mon Sep 17 00:00:00 2001 From: Michael Eischer Date: Sat, 13 May 2023 22:34:02 +0200 Subject: [PATCH 2/3] backup: return normal error if --group-by cannot be parsed Co-authored-by: greatroar <61184462+greatroar@users.noreply.github.com> --- internal/restic/snapshot_group.go | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/internal/restic/snapshot_group.go b/internal/restic/snapshot_group.go index 9efae2ff6..964a230b3 100644 --- a/internal/restic/snapshot_group.go +++ b/internal/restic/snapshot_group.go @@ -2,10 +2,9 @@ package restic import ( "encoding/json" + "fmt" "sort" "strings" - - "github.com/restic/restic/internal/errors" ) type SnapshotGroupByOptions struct { @@ -26,7 +25,7 @@ func splitSnapshotGroupBy(s string) (SnapshotGroupByOptions, error) { l.Tag = true case "": default: - return SnapshotGroupByOptions{}, errors.Fatal("unknown grouping option: '" + option + "'") + return SnapshotGroupByOptions{}, fmt.Errorf("unknown grouping option: %q", option) } } return l, nil From 5773b86d028cd4a2be2d94cec970aef44ad6b621 Mon Sep 17 00:00:00 2001 From: Michael Eischer Date: Sat, 13 May 2023 22:43:42 +0200 Subject: [PATCH 3/3] repository: Push all usage of errors.Fatal out of the package As the `Fatal` error type only includes a string, it becomes impossible to inspect the contained error. This is for a example a problem for the fuse implementation, which must be able to detect context.Canceled errors. Co-authored-by: greatroar <61184462+greatroar@users.noreply.github.com> --- cmd/restic/cmd_copy.go | 6 +++++- cmd/restic/cmd_init.go | 2 +- cmd/restic/cmd_prune.go | 2 +- cmd/restic/global.go | 2 +- internal/repository/key.go | 2 +- internal/repository/repack.go | 2 +- internal/repository/repository.go | 16 ++++++++-------- 7 files changed, 18 insertions(+), 14 deletions(-) diff --git a/cmd/restic/cmd_copy.go b/cmd/restic/cmd_copy.go index 13767d98a..eaa0ef81a 100644 --- a/cmd/restic/cmd_copy.go +++ b/cmd/restic/cmd_copy.go @@ -6,6 +6,7 @@ import ( "github.com/restic/restic/internal/backend" "github.com/restic/restic/internal/debug" + "github.com/restic/restic/internal/errors" "github.com/restic/restic/internal/repository" "github.com/restic/restic/internal/restic" "golang.org/x/sync/errgroup" @@ -236,5 +237,8 @@ func copyTree(ctx context.Context, srcRepo restic.Repository, dstRepo restic.Rep bar := newProgressMax(!quiet, uint64(len(packList)), "packs copied") _, err = repository.Repack(ctx, srcRepo, dstRepo, packList, copyBlobs, bar) bar.Done() - return err + if err != nil { + return errors.Fatal(err.Error()) + } + return nil } diff --git a/cmd/restic/cmd_init.go b/cmd/restic/cmd_init.go index a878f3e16..c01fada16 100644 --- a/cmd/restic/cmd_init.go +++ b/cmd/restic/cmd_init.go @@ -93,7 +93,7 @@ func runInit(ctx context.Context, opts InitOptions, gopts GlobalOptions, args [] PackSize: gopts.PackSize * 1024 * 1024, }) if err != nil { - return err + return errors.Fatal(err.Error()) } err = s.Init(ctx, version, gopts.password, chunkerPolynomial) diff --git a/cmd/restic/cmd_prune.go b/cmd/restic/cmd_prune.go index 1138bb55b..26f21b1f3 100644 --- a/cmd/restic/cmd_prune.go +++ b/cmd/restic/cmd_prune.go @@ -729,7 +729,7 @@ func doPrune(ctx context.Context, opts PruneOptions, gopts GlobalOptions, repo r _, err := repository.Repack(ctx, repo, repo, plan.repackPacks, plan.keepBlobs, bar) bar.Done() if err != nil { - return errors.Fatalf("%s", err) + return errors.Fatal(err.Error()) } // Also remove repacked packs diff --git a/cmd/restic/global.go b/cmd/restic/global.go index a1fa2befc..bbad925d5 100644 --- a/cmd/restic/global.go +++ b/cmd/restic/global.go @@ -456,7 +456,7 @@ func OpenRepository(ctx context.Context, opts GlobalOptions) (*repository.Reposi PackSize: opts.PackSize * 1024 * 1024, }) if err != nil { - return nil, err + return nil, errors.Fatal(err.Error()) } passwordTriesLeft := 1 diff --git a/internal/repository/key.go b/internal/repository/key.go index b3e13ade4..fd20b8e5f 100644 --- a/internal/repository/key.go +++ b/internal/repository/key.go @@ -21,7 +21,7 @@ var ( ErrNoKeyFound = errors.New("wrong password or no key found") // ErrMaxKeysReached is returned when the maximum number of keys was checked and no key could be found. - ErrMaxKeysReached = errors.Fatal("maximum number of keys reached") + ErrMaxKeysReached = errors.New("maximum number of keys reached") ) // Key represents an encrypted master key for a repository. diff --git a/internal/repository/repack.go b/internal/repository/repack.go index 6adff69f4..c82e63f28 100644 --- a/internal/repository/repack.go +++ b/internal/repository/repack.go @@ -29,7 +29,7 @@ func Repack(ctx context.Context, repo restic.Repository, dstRepo restic.Reposito debug.Log("repacking %d packs while keeping %d blobs", len(packs), keepBlobs.Len()) if repo == dstRepo && dstRepo.Connections() < 2 { - return nil, errors.Fatal("repack step requires a backend connection limit of at least two") + return nil, errors.New("repack step requires a backend connection limit of at least two") } wg, wgCtx := errgroup.WithContext(ctx) diff --git a/internal/repository/repository.go b/internal/repository/repository.go index 35826d9c9..9d1b40c64 100644 --- a/internal/repository/repository.go +++ b/internal/repository/repository.go @@ -110,16 +110,16 @@ func (c *CompressionMode) Type() string { // New returns a new repository with backend be. func New(be restic.Backend, opts Options) (*Repository, error) { if opts.Compression == CompressionInvalid { - return nil, errors.Fatalf("invalid compression mode") + return nil, errors.New("invalid compression mode") } if opts.PackSize == 0 { opts.PackSize = DefaultPackSize } if opts.PackSize > MaxPackSize { - return nil, errors.Fatalf("pack size larger than limit of %v MiB", MaxPackSize/1024/1024) + return nil, fmt.Errorf("pack size larger than limit of %v MiB", MaxPackSize/1024/1024) } else if opts.PackSize < MinPackSize { - return nil, errors.Fatalf("pack size smaller than minimum of %v MiB", MinPackSize/1024/1024) + return nil, fmt.Errorf("pack size smaller than minimum of %v MiB", MinPackSize/1024/1024) } repo := &Repository{ @@ -593,7 +593,7 @@ func (r *Repository) LoadIndex(ctx context.Context) error { }) if err != nil { - return errors.Fatal(err.Error()) + return err } err = r.idx.MergeFinalIndexes() @@ -613,7 +613,7 @@ func (r *Repository) LoadIndex(ctx context.Context) error { } }) if invalidIndex { - return errors.Fatal("index uses feature not supported by repository version 1") + return errors.New("index uses feature not supported by repository version 1") } } @@ -678,7 +678,7 @@ func (r *Repository) CreateIndexFromPacks(ctx context.Context, packsize map[rest err = wg.Wait() if err != nil { - return invalid, errors.Fatal(err.Error()) + return invalid, err } return invalid, nil @@ -723,9 +723,9 @@ func (r *Repository) SearchKey(ctx context.Context, password string, maxKeys int r.keyID = key.ID() cfg, err := restic.LoadConfig(ctx, r) if err == crypto.ErrUnauthenticated { - return errors.Fatalf("config or key %v is damaged: %v", key.ID(), err) + return fmt.Errorf("config or key %v is damaged: %w", key.ID(), err) } else if err != nil { - return errors.Fatalf("config cannot be loaded: %v", err) + return fmt.Errorf("config cannot be loaded: %w", err) } r.setConfig(cfg)