operations: don't sleep for a Retry-After error when the transfer is cancelled

The wait to obey a Retry-After error from the server used
time.Sleep() so it carried on sleeping when the context was
cancelled, e.g. by job/stop or --max-duration, and it also slept
after the last try when there was nothing left to retry.

Copying a file also logged the low level retry number counting from
0 rather than 1 as everything else does.

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
 		}

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

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)