diff --git a/fs/operations/copy.go b/fs/operations/copy.go index 5e506adef..c2d45c206 100644 --- a/fs/operations/copy.go +++ b/fs/operations/copy.go @@ -12,7 +12,6 @@ import ( "io" "path" "strings" - "time" "unicode/utf8" "github.com/rclone/rclone/fs" @@ -336,13 +335,16 @@ func (c *copy) copy(ctx context.Context) (newDst fs.Object, err error) { retry = false if fserrors.IsRetryError(err) || fserrors.ShouldRetry(err) { retry = true - } else if t, ok := pacer.IsRetryAfter(err); ok { + } else if t, ok := pacer.IsRetryAfter(err); ok && tries+1 < c.maxTries { fs.Debugf(c.src, "Sleeping for %v (as indicated by the server) to obey Retry-After error: %v", t, err) - time.Sleep(t) - retry = true + if sleepWithContext(ctx, t) { + retry = true + } else { + fserrors.ContextError(ctx, &err) + } } if retry { - fs.Debugf(c.src, "Received error: %v - low level retry %d/%d", err, tries, c.maxTries) + fs.Debugf(c.src, "Received error: %v - low level retry %d/%d", err, tries+1, c.maxTries) c.tr.Reset(ctx) // skip incomplete accounting - will be overwritten by retry continue } diff --git a/fs/operations/operations.go b/fs/operations/operations.go index 44ba5b305..bc06c9d0a 100644 --- a/fs/operations/operations.go +++ b/fs/operations/operations.go @@ -745,6 +745,19 @@ func SameDir(fdst, fsrc fs.Info) bool { return fdstRootFolded == fsrcRootFolded } +// sleepWithContext sleeps for d returning true, or false if ctx +// finishes first. +func sleepWithContext(ctx context.Context, d time.Duration) bool { + timer := time.NewTimer(d) + defer timer.Stop() + select { + case <-timer.C: + return true + case <-ctx.Done(): + return false + } +} + // Retry runs fn up to maxTries times if it returns a retriable error func Retry(ctx context.Context, o any, maxTries int, fn func() error) (err error) { for tries := 1; tries <= maxTries; tries++ { @@ -762,8 +775,14 @@ func Retry(ctx context.Context, o any, maxTries int, fn func() error) (err error fs.Debugf(o, "Received error: %v - low level retry %d/%d", err, tries, maxTries) continue } else if t, ok := pacer.IsRetryAfter(err); ok { + if tries >= maxTries { + break + } fs.Debugf(o, "Sleeping for %v (as indicated by the server) to obey Retry-After error: %v", t, err) - time.Sleep(t) + if !sleepWithContext(ctx, t) { + fserrors.ContextError(ctx, &err) + break + } continue } break diff --git a/fs/operations/operations_test.go b/fs/operations/operations_test.go index debacc0ab..39cbfca98 100644 --- a/fs/operations/operations_test.go +++ b/fs/operations/operations_test.go @@ -566,6 +566,54 @@ func TestRetry(t *testing.T) { } +// Check the wait for a Retry-After error can be interrupted +func TestRetryAfterContextCancel(t *testing.T) { + ctx, cancel := context.WithCancel(context.Background()) + retryAfter := pacer.RetryAfterError(errors.New("BANG"), time.Hour) + calls := 0 + fn := func() error { + calls++ + go cancel() + return retryAfter + } + + done := make(chan error, 1) + go func() { + done <- operations.Retry(ctx, nil, 5, fn) + }() + select { + case err := <-done: + // The error from the call is returned, not the context error + assert.Equal(t, retryAfter, err) + assert.Equal(t, 1, calls) + case <-time.After(30 * time.Second): + t.Fatal("Retry didn't return - still sleeping for the Retry-After") + } +} + +// Check we don't wait for a Retry-After error on the last try +func TestRetryAfterLastTry(t *testing.T) { + ctx := context.Background() + retryAfter := pacer.RetryAfterError(errors.New("BANG"), time.Hour) + calls := 0 + fn := func() error { + calls++ + return retryAfter + } + + done := make(chan error, 1) + go func() { + done <- operations.Retry(ctx, nil, 1, fn) + }() + select { + case err := <-done: + assert.Equal(t, retryAfter, err) + assert.Equal(t, 1, calls) + case <-time.After(30 * time.Second): + t.Fatal("Retry didn't return - slept for the Retry-After on the last try") + } +} + func TestCat(t *testing.T) { ctx := context.Background() r := fstest.NewRun(t)