From 63a71f70e338e22bc0c06985d49777f8dd4749b6 Mon Sep 17 00:00:00 2001 From: Michael Eischer Date: Mon, 24 Mar 2025 12:24:54 +0100 Subject: [PATCH 1/3] backend/sftp: ensure all errors are wrapped with the method name --- internal/backend/sftp/sftp.go | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/internal/backend/sftp/sftp.go b/internal/backend/sftp/sftp.go index a65db464c..8b003ab07 100644 --- a/internal/backend/sftp/sftp.go +++ b/internal/backend/sftp/sftp.go @@ -184,7 +184,7 @@ func (r *SFTP) mkdirAllDataSubdirs(ctx context.Context, nconn uint) error { if err := r.c.Mkdir(d); err == nil { return nil } - return r.c.MkdirAll(d) + return errors.Wrap(r.c.MkdirAll(d), "MkdirAll") }) } @@ -311,13 +311,17 @@ func (r *SFTP) Save(_ context.Context, h backend.Handle, rd backend.RewindReader } } + if err != nil { + return errors.Wrap(err, "OpenFile") + } + // pkg/sftp doesn't allow creating with a mode. // Chmod while the file is still empty. if err == nil { err = f.Chmod(r.Modes.File) - } - if err != nil { - return errors.Wrap(err, "OpenFile") + if err != nil { + return errors.Wrap(err, "Chmod") + } } defer func() { @@ -456,7 +460,7 @@ func (r *SFTP) Remove(_ context.Context, h backend.Handle) error { return err } - return r.c.Remove(r.Filename(h)) + return errors.Wrap(r.c.Remove(r.Filename(h)), "Remove") } // List runs fn for each file in the backend which has the type t. When an @@ -528,7 +532,7 @@ func (r *SFTP) Close() error { return nil } - err := r.c.Close() + err := errors.Wrap(r.c.Close(), "Close") debug.Log("Close returned error %v", err) // wait for closeTimeout before killing the process From 66ec735ac238576c17abecb453ba4bf85e5dc187 Mon Sep 17 00:00:00 2001 From: Michael Eischer Date: Mon, 24 Mar 2025 12:33:53 +0100 Subject: [PATCH 2/3] backend/sftp: include file path in error messages --- internal/backend/sftp/sftp.go | 26 +++++++++++++------------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/internal/backend/sftp/sftp.go b/internal/backend/sftp/sftp.go index 8b003ab07..d2bad336c 100644 --- a/internal/backend/sftp/sftp.go +++ b/internal/backend/sftp/sftp.go @@ -184,7 +184,7 @@ func (r *SFTP) mkdirAllDataSubdirs(ctx context.Context, nconn uint) error { if err := r.c.Mkdir(d); err == nil { return nil } - return errors.Wrap(r.c.MkdirAll(d), "MkdirAll") + return errors.Wrapf(r.c.MkdirAll(d), "MkdirAll %v", d) }) } @@ -312,7 +312,7 @@ func (r *SFTP) Save(_ context.Context, h backend.Handle, rd backend.RewindReader } if err != nil { - return errors.Wrap(err, "OpenFile") + return errors.Wrapf(err, "OpenFile %v", tmpFilename) } // pkg/sftp doesn't allow creating with a mode. @@ -320,7 +320,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 { - return errors.Wrap(err, "Chmod") + return errors.Wrapf(err, "Chmod %v", tmpFilename) } } @@ -342,18 +342,18 @@ func (r *SFTP) Save(_ context.Context, h backend.Handle, rd backend.RewindReader if err != nil { _ = f.Close() err = r.checkNoSpace(dirname, rd.Length(), err) - return errors.Wrap(err, "Write") + return errors.Wrapf(err, "Write %v", tmpFilename) } // sanity check if wbytes != rd.Length() { _ = f.Close() - return errors.Errorf("wrote %d bytes instead of the expected %d bytes", wbytes, rd.Length()) + return errors.Errorf("Write %v: wrote %d bytes instead of the expected %d bytes", tmpFilename, wbytes, rd.Length()) } err = f.Close() if err != nil { - return errors.Wrap(err, "Close") + return errors.Wrapf(err, "Close %v", tmpFilename) } // Prefer POSIX atomic rename if available. @@ -362,7 +362,7 @@ func (r *SFTP) Save(_ context.Context, h backend.Handle, rd backend.RewindReader } else { err = r.c.Rename(tmpFilename, filename) } - return errors.Wrap(err, "Rename") + return errors.Wrapf(err, "Rename %v", tmpFilename) } // checkNoSpace checks if err was likely caused by lack of available space @@ -448,7 +448,7 @@ func (r *SFTP) Stat(_ context.Context, h backend.Handle) (backend.FileInfo, erro fi, err := r.c.Lstat(r.Filename(h)) if err != nil { - return backend.FileInfo{}, errors.Wrap(err, "Lstat") + return backend.FileInfo{}, errors.Wrapf(err, "Lstat %v", r.Filename(h)) } return backend.FileInfo{Size: fi.Size(), Name: h.Name}, nil @@ -460,7 +460,7 @@ func (r *SFTP) Remove(_ context.Context, h backend.Handle) error { return err } - return errors.Wrap(r.c.Remove(r.Filename(h)), "Remove") + return errors.Wrapf(r.c.Remove(r.Filename(h)), "Remove %v", r.Filename(h)) } // List runs fn for each file in the backend which has the type t. When an @@ -483,7 +483,7 @@ func (r *SFTP) List(ctx context.Context, t backend.FileType, fn func(backend.Fil debug.Log("ignoring non-existing directory") return nil } - return walker.Err() + return errors.Wrapf(walker.Err(), "Walk %v", basedir) } if walker.Path() == basedir { @@ -554,7 +554,7 @@ func (r *SFTP) Close() error { func (r *SFTP) deleteRecursive(ctx context.Context, name string) error { entries, err := r.c.ReadDir(name) if err != nil { - return errors.Wrapf(err, "ReadDir(%v)", name) + return errors.Wrapf(err, "ReadDir %v", name) } for _, fi := range entries { @@ -571,7 +571,7 @@ func (r *SFTP) deleteRecursive(ctx context.Context, name string) error { err = r.c.RemoveDirectory(itemName) if err != nil { - return errors.Wrap(err, "RemoveDirectory") + return errors.Wrapf(err, "RemoveDirectory %v", itemName) } continue @@ -579,7 +579,7 @@ func (r *SFTP) deleteRecursive(ctx context.Context, name string) error { err := r.c.Remove(itemName) if err != nil { - return errors.Wrap(err, "ReadDir") + return errors.Wrapf(err, "Remove %v", itemName) } } From 3adf7d4efb485c7e454b49155aef16d1f9ba0f28 Mon Sep 17 00:00:00 2001 From: Michael Eischer Date: Mon, 24 Mar 2025 12:45:15 +0100 Subject: [PATCH 3/3] backend/sftp: wrap further errors --- internal/backend/sftp/sftp.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/internal/backend/sftp/sftp.go b/internal/backend/sftp/sftp.go index d2bad336c..c735b9b3a 100644 --- a/internal/backend/sftp/sftp.go +++ b/internal/backend/sftp/sftp.go @@ -420,14 +420,14 @@ func (r *SFTP) Load(ctx context.Context, h backend.Handle, length int, offset in func (r *SFTP) openReader(_ context.Context, h backend.Handle, length int, offset int64) (io.ReadCloser, error) { f, err := r.c.Open(r.Filename(h)) if err != nil { - return nil, err + return nil, errors.Wrapf(err, "Open %v", r.Filename(h)) } if offset > 0 { _, err = f.Seek(offset, 0) if err != nil { _ = f.Close() - return nil, err + return nil, errors.Wrapf(err, "Seek %v", r.Filename(h)) } } @@ -566,7 +566,7 @@ func (r *SFTP) deleteRecursive(ctx context.Context, name string) error { if fi.IsDir() { err := r.deleteRecursive(ctx, itemName) if err != nil { - return errors.Wrap(err, "ReadDir") + return err } err = r.c.RemoveDirectory(itemName)