From 8a6fc8535daa1cc8601511d17d30faa8921fa5ac Mon Sep 17 00:00:00 2001 From: Benjamin Legrand Date: Wed, 8 Dec 2021 17:14:45 +0100 Subject: [PATCH] accounting: fix global error acounting fs.CountError is called when an error is encountered. The method was calling GlobalStats().Error(err) which incremented the error at the global stats level. This led to calls to core/stats with group= filter returning an error count of 0 even if errors actually occured. This change requires the context to be provided when calling fs.CountError. Doing so, we can retrieve the correct StatsInfo to increment the errors from. Fixes #5865 --- cmd/cmd.go | 46 +++++++++++++++++------------- cmd/serve/dlna/dlna.go | 13 +++++---- cmd/serve/dlna/dlna_util.go | 8 ++++-- cmd/serve/http/http.go | 10 ++++--- cmd/serve/webdav/webdav.go | 5 ++-- fs/accounting/accounting.go | 4 ++- fs/accounting/stats_groups_test.go | 30 +++++++++++++++++++ fs/cache/cache.go | 2 +- fs/config.go | 2 +- fs/march/march.go | 4 +-- fs/operations/check.go | 25 +++++++++------- fs/operations/copy.go | 6 ++-- fs/operations/dedupe.go | 6 ++-- fs/operations/operations.go | 36 +++++++++++------------ fs/sync/sync.go | 6 ++-- fs/sync/sync_test.go | 4 +-- fs/walk/walk.go | 4 +-- lib/http/serve/dir.go | 10 ++++--- lib/http/serve/dir_test.go | 4 ++- 19 files changed, 139 insertions(+), 86 deletions(-) diff --git a/cmd/cmd.go b/cmd/cmd.go index 37a7d6f72..8f5f5f093 100644 --- a/cmd/cmd.go +++ b/cmd/cmd.go @@ -83,12 +83,13 @@ func ShowVersion() { // It returns a string with the file name if points to a file // otherwise "". func NewFsFile(remote string) (fs.Fs, string) { + ctx := context.Background() _, fsPath, err := fspath.SplitFs(remote) if err != nil { - err = fs.CountError(err) + err = fs.CountError(ctx, err) fs.Fatalf(nil, "Failed to create file system for %q: %v", remote, err) } - f, err := cache.Get(context.Background(), remote) + f, err := cache.Get(ctx, remote) switch err { case fs.ErrorIsFile: cache.Pin(f) // pin indefinitely since it was on the CLI @@ -97,7 +98,7 @@ func NewFsFile(remote string) (fs.Fs, string) { cache.Pin(f) // pin indefinitely since it was on the CLI return f, "" default: - err = fs.CountError(err) + err = fs.CountError(ctx, err) fs.Fatalf(nil, "Failed to create file system for %q: %v", remote, err) } return nil, "" @@ -108,18 +109,19 @@ func NewFsFile(remote string) (fs.Fs, string) { // This works the same as NewFsFile however it adds filters to the Fs // to limit it to a single file if the remote pointed to a file. func newFsFileAddFilter(remote string) (fs.Fs, string) { - fi := filter.GetConfig(context.Background()) + ctx := context.Background() + fi := filter.GetConfig(ctx) f, fileName := NewFsFile(remote) if fileName != "" { if !fi.InActive() { err := fmt.Errorf("can't limit to single files when using filters: %v", remote) - err = fs.CountError(err) + err = fs.CountError(ctx, err) fs.Fatal(nil, err.Error()) } // Limit transfers to this file err := fi.AddFile(fileName) if err != nil { - err = fs.CountError(err) + err = fs.CountError(ctx, err) fs.Fatalf(nil, "Failed to limit to single file %q: %v", remote, err) } } @@ -139,9 +141,10 @@ func NewFsSrc(args []string) fs.Fs { // // This must point to a directory func newFsDir(remote string) fs.Fs { - f, err := cache.Get(context.Background(), remote) + ctx := context.Background() + f, err := cache.Get(ctx, remote) if err != nil { - err = fs.CountError(err) + err = fs.CountError(ctx, err) fs.Fatalf(nil, "Failed to create file system for %q: %v", remote, err) } cache.Pin(f) // pin indefinitely since it was on the CLI @@ -175,6 +178,7 @@ func NewFsSrcFileDst(args []string) (fsrc fs.Fs, srcFileName string, fdst fs.Fs) // NewFsSrcDstFiles creates a new src and dst fs from the arguments // If src is a file then srcFileName and dstFileName will be non-empty func NewFsSrcDstFiles(args []string) (fsrc fs.Fs, srcFileName string, fdst fs.Fs, dstFileName string) { + ctx := context.Background() fsrc, srcFileName = newFsFileAddFilter(args[0]) // If copying a file... dstRemote := args[1] @@ -193,14 +197,14 @@ func NewFsSrcDstFiles(args []string) (fsrc fs.Fs, srcFileName string, fdst fs.Fs fs.Fatalf(nil, "%q is a directory", args[1]) } } - fdst, err := cache.Get(context.Background(), dstRemote) + fdst, err := cache.Get(ctx, dstRemote) switch err { case fs.ErrorIsFile: - _ = fs.CountError(err) + _ = fs.CountError(ctx, err) fs.Fatalf(nil, "Source doesn't exist or is a directory and destination is a file") case nil: default: - _ = fs.CountError(err) + _ = fs.CountError(ctx, err) fs.Fatalf(nil, "Failed to create file system for destination %q: %v", dstRemote, err) } cache.Pin(fdst) // pin indefinitely since it was on the CLI @@ -234,7 +238,8 @@ func ShowStats() bool { // Run the function with stats and retries if required func Run(Retry bool, showStats bool, cmd *cobra.Command, f func() error) { - ci := fs.GetConfig(context.Background()) + ctx := context.Background() + ci := fs.GetConfig(ctx) var cmdErr error stopStats := func() {} if !showStats && ShowStats() { @@ -248,7 +253,7 @@ func Run(Retry bool, showStats bool, cmd *cobra.Command, f func() error) { SigInfoHandler() for try := 1; try <= ci.Retries; try++ { cmdErr = f() - cmdErr = fs.CountError(cmdErr) + cmdErr = fs.CountError(ctx, cmdErr) lastErr := accounting.GlobalStats().GetLastError() if cmdErr == nil { cmdErr = lastErr @@ -436,19 +441,19 @@ func initConfig() { fs.Infof(nil, "Creating CPU profile %q\n", *cpuProfile) f, err := os.Create(*cpuProfile) if err != nil { - err = fs.CountError(err) + err = fs.CountError(ctx, err) fs.Fatal(nil, fmt.Sprint(err)) } err = pprof.StartCPUProfile(f) if err != nil { - err = fs.CountError(err) + err = fs.CountError(ctx, err) fs.Fatal(nil, fmt.Sprint(err)) } atexit.Register(func() { pprof.StopCPUProfile() err := f.Close() if err != nil { - err = fs.CountError(err) + err = fs.CountError(ctx, err) fs.Fatal(nil, fmt.Sprint(err)) } }) @@ -460,17 +465,17 @@ func initConfig() { fs.Infof(nil, "Saving Memory profile %q\n", *memProfile) f, err := os.Create(*memProfile) if err != nil { - err = fs.CountError(err) + err = fs.CountError(ctx, err) fs.Fatal(nil, fmt.Sprint(err)) } err = pprof.WriteHeapProfile(f) if err != nil { - err = fs.CountError(err) + err = fs.CountError(ctx, err) fs.Fatal(nil, fmt.Sprint(err)) } err = f.Close() if err != nil { - err = fs.CountError(err) + err = fs.CountError(ctx, err) fs.Fatal(nil, fmt.Sprint(err)) } }) @@ -478,7 +483,8 @@ func initConfig() { } func resolveExitCode(err error) { - ci := fs.GetConfig(context.Background()) + ctx := context.Background() + ci := fs.GetConfig(ctx) atexit.Run() if err == nil { if ci.ErrorOnNoTransfer { diff --git a/cmd/serve/dlna/dlna.go b/cmd/serve/dlna/dlna.go index 29af87b4a..199350877 100644 --- a/cmd/serve/dlna/dlna.go +++ b/cmd/serve/dlna/dlna.go @@ -190,16 +190,17 @@ func (s *server) ModelNumber() string { // Renders the root device descriptor. func (s *server) rootDescHandler(w http.ResponseWriter, r *http.Request) { + ctx := r.Context() tmpl, err := data.GetTemplate() if err != nil { - serveError(s, w, "Failed to load root descriptor template", err) + serveError(ctx, s, w, "Failed to load root descriptor template", err) return } buffer := new(bytes.Buffer) err = tmpl.Execute(buffer, s) if err != nil { - serveError(s, w, "Failed to render root descriptor XML", err) + serveError(ctx, s, w, "Failed to render root descriptor XML", err) return } @@ -215,15 +216,16 @@ func (s *server) rootDescHandler(w http.ResponseWriter, r *http.Request) { // Handle a service control HTTP request. func (s *server) serviceControlHandler(w http.ResponseWriter, r *http.Request) { + ctx := r.Context() soapActionString := r.Header.Get("SOAPACTION") soapAction, err := upnp.ParseActionHTTPHeader(soapActionString) if err != nil { - serveError(s, w, "Could not parse SOAPACTION header", err) + serveError(ctx, s, w, "Could not parse SOAPACTION header", err) return } var env soap.Envelope if err := xml.NewDecoder(r.Body).Decode(&env); err != nil { - serveError(s, w, "Could not parse SOAP request body", err) + serveError(ctx, s, w, "Could not parse SOAP request body", err) return } @@ -257,6 +259,7 @@ func (s *server) soapActionResponse(sa upnp.SoapAction, actionRequestXML []byte, // Serves actual resources (media files). func (s *server) resourceHandler(w http.ResponseWriter, r *http.Request) { + ctx := r.Context() remotePath := r.URL.Path node, err := s.vfs.Stat(r.URL.Path) if err != nil { @@ -277,7 +280,7 @@ func (s *server) resourceHandler(w http.ResponseWriter, r *http.Request) { file := node.(*vfs.File) in, err := file.Open(os.O_RDONLY) if err != nil { - serveError(node, w, "Could not open resource", err) + serveError(ctx, node, w, "Could not open resource", err) return } defer fs.CheckClose(in, &err) diff --git a/cmd/serve/dlna/dlna_util.go b/cmd/serve/dlna/dlna_util.go index a63f31f5b..ca61488af 100644 --- a/cmd/serve/dlna/dlna_util.go +++ b/cmd/serve/dlna/dlna_util.go @@ -1,6 +1,7 @@ package dlna import ( + "context" "crypto/md5" "encoding/xml" "fmt" @@ -142,9 +143,10 @@ func logging(next http.Handler) http.Handler { // Error recovery and general request logging are left to logging(). func traceLogging(next http.Handler) http.Handler { return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + ctx := r.Context() dump, err := httputil.DumpRequest(r, true) if err != nil { - serveError(nil, w, "error dumping request", err) + serveError(ctx, nil, w, "error dumping request", err) return } fs.Debugf(nil, "%s", dump) @@ -182,8 +184,8 @@ func withHeader(name string, value string, next http.Handler) http.Handler { } // serveError returns an http.StatusInternalServerError and logs the error -func serveError(what interface{}, w http.ResponseWriter, text string, err error) { - err = fs.CountError(err) +func serveError(ctx context.Context, what interface{}, w http.ResponseWriter, text string, err error) { + err = fs.CountError(ctx, err) fs.Errorf(what, "%s: %v", text, err) http.Error(w, text+".", http.StatusInternalServerError) } diff --git a/cmd/serve/http/http.go b/cmd/serve/http/http.go index 9ace8e663..7aba1e598 100644 --- a/cmd/serve/http/http.go +++ b/cmd/serve/http/http.go @@ -186,6 +186,7 @@ func (s *HTTP) handler(w http.ResponseWriter, r *http.Request) { // serveDir serves a directory index at dirRemote func (s *HTTP) serveDir(w http.ResponseWriter, r *http.Request, dirRemote string) { + ctx := r.Context() VFS, err := s.getVFS(r.Context()) if err != nil { http.Error(w, "Root directory not found", http.StatusNotFound) @@ -198,7 +199,7 @@ func (s *HTTP) serveDir(w http.ResponseWriter, r *http.Request, dirRemote string http.Error(w, "Directory not found", http.StatusNotFound) return } else if err != nil { - serve.Error(dirRemote, w, "Failed to list directory", err) + serve.Error(ctx, dirRemote, w, "Failed to list directory", err) return } if !node.IsDir() { @@ -208,7 +209,7 @@ func (s *HTTP) serveDir(w http.ResponseWriter, r *http.Request, dirRemote string dir := node.(*vfs.Dir) dirEntries, err := dir.ReadDirAll() if err != nil { - serve.Error(dirRemote, w, "Failed to list directory", err) + serve.Error(ctx, dirRemote, w, "Failed to list directory", err) return } @@ -234,6 +235,7 @@ func (s *HTTP) serveDir(w http.ResponseWriter, r *http.Request, dirRemote string // serveFile serves a file object at remote func (s *HTTP) serveFile(w http.ResponseWriter, r *http.Request, remote string) { + ctx := r.Context() VFS, err := s.getVFS(r.Context()) if err != nil { http.Error(w, "File not found", http.StatusNotFound) @@ -247,7 +249,7 @@ func (s *HTTP) serveFile(w http.ResponseWriter, r *http.Request, remote string) http.Error(w, "File not found", http.StatusNotFound) return } else if err != nil { - serve.Error(remote, w, "Failed to find file", err) + serve.Error(ctx, remote, w, "Failed to find file", err) return } if !node.IsFile() { @@ -287,7 +289,7 @@ func (s *HTTP) serveFile(w http.ResponseWriter, r *http.Request, remote string) // open the object in, err := file.Open(os.O_RDONLY) if err != nil { - serve.Error(remote, w, "Failed to open file", err) + serve.Error(ctx, remote, w, "Failed to open file", err) return } defer func() { diff --git a/cmd/serve/webdav/webdav.go b/cmd/serve/webdav/webdav.go index 622575e8d..0c5c16416 100644 --- a/cmd/serve/webdav/webdav.go +++ b/cmd/serve/webdav/webdav.go @@ -349,6 +349,7 @@ func (w *WebDAV) ServeHTTP(rw http.ResponseWriter, r *http.Request) { // serveDir serves a directory index at dirRemote // This is similar to serveDir in serve http. func (w *WebDAV) serveDir(rw http.ResponseWriter, r *http.Request, dirRemote string) { + ctx := r.Context() VFS, err := w.getVFS(r.Context()) if err != nil { http.Error(rw, "Root directory not found", http.StatusNotFound) @@ -361,7 +362,7 @@ func (w *WebDAV) serveDir(rw http.ResponseWriter, r *http.Request, dirRemote str http.Error(rw, "Directory not found", http.StatusNotFound) return } else if err != nil { - serve.Error(dirRemote, rw, "Failed to list directory", err) + serve.Error(ctx, dirRemote, rw, "Failed to list directory", err) return } if !node.IsDir() { @@ -372,7 +373,7 @@ func (w *WebDAV) serveDir(rw http.ResponseWriter, r *http.Request, dirRemote str dirEntries, err := dir.ReadDirAll() if err != nil { - serve.Error(dirRemote, rw, "Failed to list directory", err) + serve.Error(ctx, dirRemote, rw, "Failed to list directory", err) return } diff --git a/fs/accounting/accounting.go b/fs/accounting/accounting.go index d7d7e02dd..5a6be7df4 100644 --- a/fs/accounting/accounting.go +++ b/fs/accounting/accounting.go @@ -44,7 +44,9 @@ func Start(ctx context.Context) { // // We can't do this in an init() method as it uses fs.Config // and that isn't set up then. - fs.CountError = GlobalStats().Error + fs.CountError = func(ctx context.Context, err error) error { + return Stats(ctx).Error(err) + } } // Account limits and accounts for one transfer diff --git a/fs/accounting/stats_groups_test.go b/fs/accounting/stats_groups_test.go index dda9dccef..8c02229e7 100644 --- a/fs/accounting/stats_groups_test.go +++ b/fs/accounting/stats_groups_test.go @@ -7,6 +7,8 @@ import ( "testing" "time" + "github.com/rclone/rclone/fs" + "github.com/rclone/rclone/fs/fserrors" "github.com/rclone/rclone/fs/rc" "github.com/rclone/rclone/fstest/testy" "github.com/stretchr/testify/assert" @@ -206,6 +208,34 @@ func TestStatsGroupOperations(t *testing.T) { }) } +func TestCountError(t *testing.T) { + ctx := context.Background() + Start(ctx) + defer func() { + groups = newStatsGroups() + }() + t.Run("global stats", func(t *testing.T) { + GlobalStats().ResetCounters() + err := fs.CountError(ctx, fmt.Errorf("global err")) + assert.Equal(t, int64(1), GlobalStats().errors) + + assert.True(t, fserrors.IsCounted(err)) + }) + t.Run("group stats", func(t *testing.T) { + statGroupName := fmt.Sprintf("%s-error_group", t.Name()) + GlobalStats().ResetCounters() + stCtx := WithStatsGroup(ctx, statGroupName) + st := StatsGroup(stCtx, statGroupName) + + err := fs.CountError(stCtx, fmt.Errorf("group err")) + + assert.Equal(t, int64(0), GlobalStats().errors) + assert.Equal(t, int64(1), st.errors) + assert.True(t, fserrors.IsCounted(err)) + }) + +} + func percentDiff(start, end uint64) uint64 { return (start - end) * 100 / start } diff --git a/fs/cache/cache.go b/fs/cache/cache.go index e2c104395..63afa94e8 100644 --- a/fs/cache/cache.go +++ b/fs/cache/cache.go @@ -29,7 +29,7 @@ func createOnFirstUse() { c.SetExpireInterval(ci.FsCacheExpireInterval) c.SetFinalizer(func(value interface{}) { if s, ok := value.(fs.Shutdowner); ok { - _ = fs.CountError(s.Shutdown(context.Background())) + _ = fs.CountError(context.Background(), s.Shutdown(context.Background())) } }) }) diff --git a/fs/config.go b/fs/config.go index caf242fdf..1b11da821 100644 --- a/fs/config.go +++ b/fs/config.go @@ -41,7 +41,7 @@ var ( // // This is a function pointer to decouple the config // implementation from the fs - CountError = func(err error) error { return err } + CountError = func(ctx context.Context, err error) error { return err } // ConfigProvider is the config key used for provider options ConfigProvider = "provider" diff --git a/fs/march/march.go b/fs/march/march.go index 856be0e7c..1116220db 100644 --- a/fs/march/march.go +++ b/fs/march/march.go @@ -415,7 +415,7 @@ func (m *March) processJob(job listDirJob) ([]listDirJob, error) { } else { fs.Errorf(m.Fsrc, "error reading source root directory: %v", srcListErr) } - srcListErr = fs.CountError(srcListErr) + srcListErr = fs.CountError(m.Ctx, srcListErr) return nil, srcListErr } if dstListErr == fs.ErrorDirNotFound { @@ -426,7 +426,7 @@ func (m *March) processJob(job listDirJob) ([]listDirJob, error) { } else { fs.Errorf(m.Fdst, "error reading destination root directory: %v", dstListErr) } - dstListErr = fs.CountError(dstListErr) + dstListErr = fs.CountError(m.Ctx, dstListErr) return nil, dstListErr } diff --git a/fs/operations/check.go b/fs/operations/check.go index b9fe36aab..c3c747071 100644 --- a/fs/operations/check.go +++ b/fs/operations/check.go @@ -49,6 +49,7 @@ type CheckOpt struct { // checkMarch is used to march over two Fses in the same way as // sync/copy type checkMarch struct { + ctx context.Context ioMu sync.Mutex wg sync.WaitGroup tokens chan struct{} @@ -83,7 +84,7 @@ func (c *checkMarch) DstOnly(dst fs.DirEntry) (recurse bool) { } err := fmt.Errorf("file not in %v", c.opt.Fsrc) fs.Errorf(dst, "%v", err) - _ = fs.CountError(err) + _ = fs.CountError(c.ctx, err) c.differences.Add(1) c.srcFilesMissing.Add(1) c.report(dst, c.opt.MissingOnSrc, '-') @@ -105,7 +106,7 @@ func (c *checkMarch) SrcOnly(src fs.DirEntry) (recurse bool) { case fs.Object: err := fmt.Errorf("file not in %v", c.opt.Fdst) fs.Errorf(src, "%v", err) - _ = fs.CountError(err) + _ = fs.CountError(c.ctx, err) c.differences.Add(1) c.dstFilesMissing.Add(1) c.report(src, c.opt.MissingOnDst, '+') @@ -155,13 +156,13 @@ func (c *checkMarch) Match(ctx context.Context, dst, src fs.DirEntry) (recurse b differ, noHash, err := c.checkIdentical(ctx, dstX, srcX) if err != nil { fs.Errorf(src, "%v", err) - _ = fs.CountError(err) + _ = fs.CountError(ctx, err) c.report(src, c.opt.Error, '!') } else if differ { c.differences.Add(1) err := errors.New("files differ") // the checkFn has already logged the reason - _ = fs.CountError(err) + _ = fs.CountError(ctx, err) c.report(src, c.opt.Differ, '*') } else { c.matches.Add(1) @@ -177,7 +178,7 @@ func (c *checkMarch) Match(ctx context.Context, dst, src fs.DirEntry) (recurse b } else { err := fmt.Errorf("is file on %v but directory on %v", c.opt.Fsrc, c.opt.Fdst) fs.Errorf(src, "%v", err) - _ = fs.CountError(err) + _ = fs.CountError(ctx, err) c.differences.Add(1) c.dstFilesMissing.Add(1) c.report(src, c.opt.MissingOnDst, '+') @@ -190,7 +191,7 @@ func (c *checkMarch) Match(ctx context.Context, dst, src fs.DirEntry) (recurse b } err := fmt.Errorf("is file on %v but directory on %v", c.opt.Fdst, c.opt.Fsrc) fs.Errorf(dst, "%v", err) - _ = fs.CountError(err) + _ = fs.CountError(ctx, err) c.differences.Add(1) c.srcFilesMissing.Add(1) c.report(dst, c.opt.MissingOnSrc, '-') @@ -214,6 +215,7 @@ func CheckFn(ctx context.Context, opt *CheckOpt) error { return errors.New("internal error: nil check function") } c := &checkMarch{ + ctx: ctx, tokens: make(chan struct{}, ci.Checkers), opt: *opt, } @@ -430,6 +432,7 @@ func CheckSum(ctx context.Context, fsrc, fsum fs.Fs, sumFile string, hashType ha ci := fs.GetConfig(ctx) c := &checkMarch{ + ctx: ctx, tokens: make(chan struct{}, ci.Checkers), opt: *opt, } @@ -450,7 +453,7 @@ func CheckSum(ctx context.Context, fsrc, fsum fs.Fs, sumFile string, hashType ha // filesystem missed the file, sum wasn't consumed err := fmt.Errorf("file not in %v", opt.Fdst) fs.Errorf(filename, "%v", err) - _ = fs.CountError(err) + _ = fs.CountError(ctx, err) if lastErr == nil { lastErr = err } @@ -479,7 +482,7 @@ func (c *checkMarch) checkSum(ctx context.Context, obj fs.Object, download bool, if !sumFound { err = errors.New("sum not found") - _ = fs.CountError(err) + _ = fs.CountError(ctx, err) fs.Errorf(obj, "%v", err) c.differences.Add(1) c.srcFilesMissing.Add(1) @@ -528,12 +531,12 @@ func (c *checkMarch) checkSum(ctx context.Context, obj fs.Object, download bool, func (c *checkMarch) matchSum(ctx context.Context, sumHash, objHash string, obj fs.Object, err error, hashType hash.Type) { switch { case err != nil: - _ = fs.CountError(err) + _ = fs.CountError(ctx, err) fs.Errorf(obj, "Failed to calculate hash: %v", err) c.report(obj, c.opt.Error, '!') case sumHash == "": err = errors.New("duplicate file") - _ = fs.CountError(err) + _ = fs.CountError(ctx, err) fs.Errorf(obj, "%v", err) c.report(obj, c.opt.Error, '!') case objHash == "": @@ -548,7 +551,7 @@ func (c *checkMarch) matchSum(ctx context.Context, sumHash, objHash string, obj c.report(obj, c.opt.Match, '=') default: err = errors.New("files differ") - _ = fs.CountError(err) + _ = fs.CountError(ctx, err) fs.Debugf(nil, "%v = %s (sum)", hashType, sumHash) fs.Debugf(obj, "%v = %s (%v)", hashType, objHash, c.opt.Fdst) fs.Errorf(obj, "%v", err) diff --git a/fs/operations/copy.go b/fs/operations/copy.go index 4e2f09762..8228a14c7 100644 --- a/fs/operations/copy.go +++ b/fs/operations/copy.go @@ -331,7 +331,7 @@ func (c *copy) copy(ctx context.Context) (newDst fs.Object, err error) { } } if err != nil { - err = fs.CountError(err) + err = fs.CountError(ctx, err) fs.Errorf(c.src, "Failed to copy: %v", err) if !c.inplace { c.removeFailedPartialCopy(ctx, c.f, c.remoteForCopy) @@ -343,7 +343,7 @@ func (c *copy) copy(ctx context.Context) (newDst fs.Object, err error) { err = c.verify(ctx, newDst) if err != nil { fs.Errorf(newDst, "%v", err) - err = fs.CountError(err) + err = fs.CountError(ctx, err) c.removeFailedCopy(ctx, newDst) return nil, err } @@ -353,7 +353,7 @@ func (c *copy) copy(ctx context.Context) (newDst fs.Object, err error) { movedNewDst, err := c.dstFeatures.Move(ctx, newDst, c.remote) if err != nil { fs.Errorf(newDst, "partial file rename failed: %v", err) - err = fs.CountError(err) + err = fs.CountError(ctx, err) c.removeFailedCopy(ctx, newDst) return nil, err } diff --git a/fs/operations/dedupe.go b/fs/operations/dedupe.go index 3e0f72cd5..60c66c4a3 100644 --- a/fs/operations/dedupe.go +++ b/fs/operations/dedupe.go @@ -32,7 +32,7 @@ outer: _, err := f.NewObject(ctx, newName) for ; err != fs.ErrorObjectNotFound; suffix++ { if err != nil { - err = fs.CountError(err) + err = fs.CountError(ctx, err) fs.Errorf(o, "Failed to check for existing object: %v", err) continue outer } @@ -46,7 +46,7 @@ outer: if !SkipDestructive(ctx, o, "rename") { newObj, err := doMove(ctx, o, newName) if err != nil { - err = fs.CountError(err) + err = fs.CountError(ctx, err) fs.Errorf(o, "Failed to rename: %v", err) continue } @@ -374,7 +374,7 @@ func dedupeMergeDuplicateDirs(ctx context.Context, f fs.Fs, duplicateDirs [][]*d fs.Infof(fsDirs[0], "Merging contents of duplicate directories") err := mergeDirs(ctx, fsDirs) if err != nil { - err = fs.CountError(err) + err = fs.CountError(ctx, err) fs.Errorf(nil, "merge duplicate dirs: %v", err) } } diff --git a/fs/operations/operations.go b/fs/operations/operations.go index f14ecb61e..53d846525 100644 --- a/fs/operations/operations.go +++ b/fs/operations/operations.go @@ -101,11 +101,11 @@ func checkHashes(ctx context.Context, src fs.ObjectInfo, dst fs.Object, ht hash. return true, hash.None, srcHash, dstHash, nil } if srcErr != nil { - err = fs.CountError(srcErr) + err = fs.CountError(ctx, srcErr) fs.Errorf(src, "Failed to calculate src hash: %v", err) } if dstErr != nil { - err = fs.CountError(dstErr) + err = fs.CountError(ctx, dstErr) fs.Errorf(dst, "Failed to calculate dst hash: %v", err) } if err != nil { @@ -340,7 +340,7 @@ func equal(ctx context.Context, src fs.ObjectInfo, dst fs.Object, opt equalOpt) logger(ctx, Differ, src, dst, nil) return false } else if err != nil { - err = fs.CountError(err) + err = fs.CountError(ctx, err) fs.Errorf(dst, "Failed to set modification time: %v", err) } else { fs.Infof(src, "Updated modification time in destination") @@ -481,7 +481,7 @@ func move(ctx context.Context, fdst fs.Fs, dst fs.Object, remote string, src fs. fs.Debugf(src, "Can't move, switching to copy") _ = in.Close() default: - err = fs.CountError(err) + err = fs.CountError(ctx, err) fs.Errorf(src, "Couldn't move: %v", err) _ = in.Close() return newDst, err @@ -566,7 +566,7 @@ func DeleteFileWithBackupDir(ctx context.Context, dst fs.Object, backupDir fs.Fs } if err != nil { fs.Errorf(dst, "Couldn't %s: %v", action, err) - err = fs.CountError(err) + err = fs.CountError(ctx, err) } else if !skip { fs.Infof(dst, "%s", actioned) } @@ -974,7 +974,7 @@ func HashLister(ctx context.Context, ht hash.Type, outputBase64 bool, downloadFl }() sum, err := HashSum(ctx, ht, outputBase64, downloadFlag, o) if err != nil { - fs.Errorf(o, "%v", fs.CountError(err)) + fs.Errorf(o, "%v", fs.CountError(ctx, err)) return } SyncFprintf(w, "%*s %s\n", width, sum, o.Remote()) @@ -1053,7 +1053,7 @@ func Mkdir(ctx context.Context, f fs.Fs, dir string) error { fs.Debugf(fs.LogDirName(f, dir), "Making directory") err := f.Mkdir(ctx, dir) if err != nil { - err = fs.CountError(err) + err = fs.CountError(ctx, err) return err } return nil @@ -1075,7 +1075,7 @@ func MkdirMetadata(ctx context.Context, f fs.Fs, dir string, metadata fs.Metadat fs.Debugf(fs.LogDirName(f, dir), "Making directory with metadata") newDst, err = do(ctx, dir, metadata) if err != nil { - err = fs.CountError(err) + err = fs.CountError(ctx, err) return nil, err } if mtime, ok := metadata["mtime"]; ok { @@ -1133,7 +1133,7 @@ func TryRmdir(ctx context.Context, f fs.Fs, dir string) error { func Rmdir(ctx context.Context, f fs.Fs, dir string) error { err := TryRmdir(ctx, f, dir) if err != nil { - err = fs.CountError(err) + err = fs.CountError(ctx, err) return err } return err @@ -1162,7 +1162,7 @@ func Purge(ctx context.Context, f fs.Fs, dir string) (err error) { err = Rmdirs(ctx, f, dir, false) } if err != nil { - err = fs.CountError(err) + err = fs.CountError(ctx, err) return err } return nil @@ -1207,7 +1207,7 @@ func listToChan(ctx context.Context, f fs.Fs, dir string) fs.ObjectsChan { }) if err != nil && err != fs.ErrorDirNotFound { err = fmt.Errorf("failed to list: %w", err) - err = fs.CountError(err) + err = fs.CountError(ctx, err) fs.Errorf(nil, "%v", err) } }() @@ -1267,7 +1267,7 @@ func Cat(ctx context.Context, f fs.Fs, w io.Writer, offset, count int64, sep []b var in io.ReadCloser in, err = Open(ctx, o, options...) if err != nil { - err = fs.CountError(err) + err = fs.CountError(ctx, err) fs.Errorf(o, "Failed to open: %v", err) return } @@ -1280,13 +1280,13 @@ func Cat(ctx context.Context, f fs.Fs, w io.Writer, offset, count int64, sep []b defer mu.Unlock() _, err = io.Copy(w, in) if err != nil { - err = fs.CountError(err) + err = fs.CountError(ctx, err) fs.Errorf(o, "Failed to send to output: %v", err) } if len(sep) > 0 { _, err = w.Write(sep) if err != nil { - err = fs.CountError(err) + err = fs.CountError(ctx, err) fs.Errorf(o, "Failed to send separator to output: %v", err) } } @@ -1423,7 +1423,7 @@ func rcatSrc(ctx context.Context, fdst fs.Fs, dstFileName string, in io.ReadClos src := object.NewStaticObjectInfo(dstFileName, modTime, int64(readCounter.BytesRead()), false, sums, fdst).WithMetadata(meta) if !equal(ctx, src, dst, opt) { err = fmt.Errorf("corrupted on transfer") - err = fs.CountError(err) + err = fs.CountError(ctx, err) fs.Errorf(dst, "%v", err) return dst, err } @@ -1450,7 +1450,7 @@ func Rmdirs(ctx context.Context, f fs.Fs, dir string, leaveRoot bool) error { dirEmpty[dir] = !leaveRoot err := walk.Walk(ctx, f, dir, false, ci.MaxDepth, func(dirPath string, entries fs.DirEntries, err error) error { if err != nil { - err = fs.CountError(err) + err = fs.CountError(ctx, err) fs.Errorf(f, "Failed to list %q: %v", dirPath, err) return nil } @@ -1526,7 +1526,7 @@ func Rmdirs(ctx context.Context, f fs.Fs, dir string, leaveRoot bool) error { g.Go(func() error { err := TryRmdir(gCtx, f, dir) if err != nil { - err = fs.CountError(err) + err = fs.CountError(ctx, err) fs.Errorf(dir, "Failed to rmdir: %v", err) errCount.Add(err) } @@ -2096,7 +2096,7 @@ func TouchDir(ctx context.Context, f fs.Fs, remote string, t time.Time, recursiv err := o.SetModTime(ctx, t) if err != nil { err = fmt.Errorf("failed to touch: %w", err) - err = fs.CountError(err) + err = fs.CountError(ctx, err) fs.Errorf(o, "%v", err) } } diff --git a/fs/sync/sync.go b/fs/sync/sync.go index 67998268f..e0a3e23ac 100644 --- a/fs/sync/sync.go +++ b/fs/sync/sync.go @@ -400,7 +400,7 @@ func (s *syncCopyMove) pairChecker(in *pipe, out *pipe, fraction int, wg *sync.W if needTransfer { // If files are treated as immutable, fail if destination exists and does not match if s.ci.Immutable && pair.Dst != nil { - err := fs.CountError(fserrors.NoRetryError(fs.ErrorImmutableModified)) + err := fs.CountError(s.ctx, fserrors.NoRetryError(fs.ErrorImmutableModified)) fs.Errorf(pair.Dst, "Source and destination exist but do not match: %v", err) s.processError(err) } else { @@ -1204,7 +1204,7 @@ func (s *syncCopyMove) setDelayedDirModTimes(ctx context.Context) error { _, err = operations.SetDirModTime(gCtx, s.fdst, item.dst, item.dir, item.modTime) } if err != nil { - err = fs.CountError(err) + err = fs.CountError(ctx, err) fs.Errorf(item.dir, "Failed to update directory timestamp or metadata: %v", err) errCount.Add(err) } @@ -1399,7 +1399,7 @@ func MoveDir(ctx context.Context, fdst, fsrc fs.Fs, deleteEmptySrcDirs bool, cop fs.Infof(fdst, "Server side directory move succeeded") return nil default: - err = fs.CountError(err) + err = fs.CountError(ctx, err) fs.Errorf(fdst, "Server side directory move failed: %v", err) return err } diff --git a/fs/sync/sync_test.go b/fs/sync/sync_test.go index 288616cd5..f4f1c556a 100644 --- a/fs/sync/sync_test.go +++ b/fs/sync/sync_test.go @@ -946,7 +946,7 @@ func TestSyncIgnoreErrors(t *testing.T) { accounting.GlobalStats().ResetCounters() ctx = predictDstFromLogger(ctx) - _ = fs.CountError(errors.New("boom")) + _ = fs.CountError(ctx, errors.New("boom")) assert.NoError(t, Sync(ctx, r.Fremote, r.Flocal, false)) testLoggerVsLsf(ctx, r.Fremote, operations.GetLoggerOpt(ctx).JSON, t) @@ -1267,7 +1267,7 @@ func TestSyncAfterRemovingAFileAndAddingAFileSubDirWithErrors(t *testing.T) { ctx = predictDstFromLogger(ctx) accounting.GlobalStats().ResetCounters() - _ = fs.CountError(errors.New("boom")) + _ = fs.CountError(ctx, errors.New("boom")) err := Sync(ctx, r.Fremote, r.Flocal, false) assert.Equal(t, fs.ErrorNotDeleting, err) testLoggerVsLsf(ctx, r.Fremote, operations.GetLoggerOpt(ctx).JSON, t) diff --git a/fs/walk/walk.go b/fs/walk/walk.go index ada7a284d..51e567a4f 100644 --- a/fs/walk/walk.go +++ b/fs/walk/walk.go @@ -170,7 +170,7 @@ func listRwalk(ctx context.Context, f fs.Fs, path string, includeAll bool, maxLe // Carry on listing but return the error at the end if err != nil { listErr = err - err = fs.CountError(err) + err = fs.CountError(ctx, err) fs.Errorf(path, "error listing: %v", err) return nil } @@ -415,7 +415,7 @@ func walk(ctx context.Context, f fs.Fs, path string, includeAll bool, maxLevel i // NB once we have passed entries to fn we mustn't touch it again if err != nil && err != ErrorSkipDir { traversing.Done() - err = fs.CountError(err) + err = fs.CountError(ctx, err) fs.Errorf(job.remote, "error listing: %v", err) closeQuit() // Send error to error channel if space diff --git a/lib/http/serve/dir.go b/lib/http/serve/dir.go index 664f81cf2..5a92ddeb8 100644 --- a/lib/http/serve/dir.go +++ b/lib/http/serve/dir.go @@ -2,6 +2,7 @@ package serve import ( "bytes" + "context" "fmt" "html/template" "net/http" @@ -124,8 +125,8 @@ func (d *Directory) AddEntry(remote string, isDir bool) { } // Error logs the error and if a ResponseWriter is given it writes an http.StatusInternalServerError -func Error(what interface{}, w http.ResponseWriter, text string, err error) { - err = fs.CountError(err) +func Error(ctx context.Context, what interface{}, w http.ResponseWriter, text string, err error) { + err = fs.CountError(ctx, err) fs.Errorf(what, "%s: %v", text, err) if w != nil { http.Error(w, text+".", http.StatusInternalServerError) @@ -223,6 +224,7 @@ const ( // Serve serves a directory func (d *Directory) Serve(w http.ResponseWriter, r *http.Request) { + ctx := r.Context() // Account the transfer tr := accounting.Stats(r.Context()).NewTransferRemoteSize(d.DirRemote, -1, nil, nil) defer tr.Done(r.Context(), nil) @@ -232,12 +234,12 @@ func (d *Directory) Serve(w http.ResponseWriter, r *http.Request) { buf := &bytes.Buffer{} err := d.HTMLTemplate.Execute(buf, d) if err != nil { - Error(d.DirRemote, w, "Failed to render template", err) + Error(ctx, d.DirRemote, w, "Failed to render template", err) return } w.Header().Set("Content-Length", fmt.Sprintf("%d", buf.Len())) _, err = buf.WriteTo(w) if err != nil { - Error(d.DirRemote, nil, "Failed to drain template buffer", err) + Error(ctx, d.DirRemote, nil, "Failed to drain template buffer", err) } } diff --git a/lib/http/serve/dir_test.go b/lib/http/serve/dir_test.go index 7bedc0471..19ab22f57 100644 --- a/lib/http/serve/dir_test.go +++ b/lib/http/serve/dir_test.go @@ -1,6 +1,7 @@ package serve import ( + "context" "errors" "html/template" "io" @@ -88,9 +89,10 @@ func TestAddEntry(t *testing.T) { } func TestError(t *testing.T) { + ctx := context.Background() w := httptest.NewRecorder() err := errors.New("help") - Error("potato", w, "sausage", err) + Error(ctx, "potato", w, "sausage", err) resp := w.Result() assert.Equal(t, http.StatusInternalServerError, resp.StatusCode) body, _ := io.ReadAll(resp.Body)