Files
rclone/backend/zoho/throttle_test.go
Erol OzcanandNick Craig-Wood 171e86369a zoho: treat R008 unauthorized as directory not found
Zoho's WorkDrive listing API returns "401 R008 Unauthorized access"
(not a 404) when a folder id no longer resolves to a listable folder,
because it was deleted or never existed. A freshly refreshed token still
gets it, so it is not a token problem and retrying it is futile - and can
escalate to a 429 F7008 rate-limit penalty.

Handle it as a missing directory instead: shouldRetry no longer retries a
bare R008 401, listAll maps it to fs.ErrorDirNotFound, and
readMetaDataForPath flushes the stale parent from the dircache and reports
the object as not found so a later create re-resolves the parent. This lets
the VFS self-heal a stale cached directory id instead of hard-failing the
operation, and stops the VFS integration tests failing on a stale directory id.

Fixes #9578
2026-07-09 18:13:55 +01:00

165 lines
6.1 KiB
Go

package zoho
import (
"context"
"errors"
"net/http"
"sync"
"testing"
"time"
"github.com/rclone/rclone/lib/pacer"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)
// newTestFs returns a bare *Fs carrying just the throttle state shouldRetry
// touches, armed exactly as NewFs arms it. No network or pacer is involved, so
// the 429/retry logic can be exercised in isolation.
func newTestFs() *Fs {
f := &Fs{throttle: &throttleState{}}
f.throttle.progress.Store(true)
return f
}
func TestShouldRetry(t *testing.T) {
ctx := context.Background()
// A 429 with a numeric Retry-After is honoured, plus retryAfterMargin.
t.Run("429 honours Retry-After", func(t *testing.T) {
f := newTestFs()
resp := &http.Response{StatusCode: 429, Header: http.Header{"Retry-After": {"5"}}}
retry, err := f.shouldRetry(ctx, resp, assert.AnError)
assert.True(t, retry)
wait, ok := pacer.IsRetryAfter(err)
require.True(t, ok)
assert.Equal(t, 5*time.Second+retryAfterMargin, wait)
})
// A 429 without a Retry-After header falls back to 60s + margin.
t.Run("429 without Retry-After falls back to 60s", func(t *testing.T) {
f := newTestFs()
resp := &http.Response{StatusCode: 429, Header: http.Header{}}
retry, err := f.shouldRetry(ctx, resp, assert.AnError)
assert.True(t, retry)
wait, ok := pacer.IsRetryAfter(err)
require.True(t, ok)
assert.Equal(t, 60*time.Second+retryAfterMargin, wait)
})
// An unparseable Retry-After is ignored in favour of the 60s fallback.
t.Run("429 with unparseable Retry-After falls back", func(t *testing.T) {
f := newTestFs()
resp := &http.Response{StatusCode: 429, Header: http.Header{"Retry-After": {"soon"}}}
retry, err := f.shouldRetry(ctx, resp, assert.AnError)
assert.True(t, retry)
wait, ok := pacer.IsRetryAfter(err)
require.True(t, ok)
assert.Equal(t, 60*time.Second+retryAfterMargin, wait)
})
// A missing OAuth scope is fatal and must not be retried.
t.Run("401 missing scope aborts", func(t *testing.T) {
f := newTestFs()
resp := &http.Response{StatusCode: 401, Status: "401 INVALID_OAUTHSCOPE"}
retry, _ := f.shouldRetry(ctx, resp, assert.AnError)
assert.False(t, retry)
})
// An expired OAuth token is retried so the token can refresh.
t.Run("401 expired token retries", func(t *testing.T) {
f := newTestFs()
resp := &http.Response{StatusCode: 401, Header: http.Header{"Www-Authenticate": {`Bearer error="expired_token"`}}}
retry, _ := f.shouldRetry(ctx, resp, assert.AnError)
assert.True(t, retry)
})
// A bare 401 R008 body means the folder was deleted (Zoho no longer accepts
// its id). It is unrecoverable, so it is NOT retried.
t.Run("401 R008 deleted folder is not retried", func(t *testing.T) {
f := newTestFs()
resp := &http.Response{StatusCode: 401}
err := errors.New(`HTTP error 401: {"errors":[{"id":"R008","title":"Unauthorized access"}]}`)
retry, _ := f.shouldRetry(ctx, resp, err)
assert.False(t, retry)
})
// A cancelled context is never retried.
t.Run("cancelled context aborts", func(t *testing.T) {
f := newTestFs()
cctx, cancel := context.WithCancel(ctx)
cancel()
retry, _ := f.shouldRetry(cctx, nil, nil)
assert.False(t, retry)
})
}
// isMissingResourceErr must match only a genuine 401 R008 body, so an unrelated
// error that merely mentions R008 (or an R008 body on some other status) is not
// mistaken for a deleted resource.
func TestIsMissingResourceErr(t *testing.T) {
r008 := errors.New(`HTTP error 401: {"errors":[{"id":"R008","title":"Unauthorized access"}]}`)
// A 401 R008 body is a missing resource.
assert.True(t, isMissingResourceErr(&http.Response{StatusCode: 401}, r008))
// An R008 body on a non-401 status is not (guards the ungated substring).
assert.False(t, isMissingResourceErr(&http.Response{StatusCode: 500}, r008))
// A 401 that is not R008 is not a missing resource.
assert.False(t, isMissingResourceErr(&http.Response{StatusCode: 401}, errors.New("HTTP error 401: expired_token")))
// No response or no error is not a missing resource.
assert.False(t, isMissingResourceErr(nil, r008))
assert.False(t, isMissingResourceErr(&http.Response{StatusCode: 401}, nil))
}
// TestThrottleEpisode covers the once-per-episode logging state machine that
// logThrottle/shouldRetry drive through throttleState, without sleeping: the
// penalty window is moved by hand instead of waited out.
func TestThrottleEpisode(t *testing.T) {
ctx := context.Background()
f := newTestFs() // progress armed: the first 429 would log at NOTICE
resp429 := &http.Response{StatusCode: 429, Header: http.Header{"Retry-After": {"1"}}}
respOK := &http.Response{StatusCode: 200}
// The first 429 consumes the armed flag (logs once) and opens a penalty window.
_, _ = f.shouldRetry(ctx, resp429, assert.AnError)
assert.False(t, f.throttle.progress.Load(), "first 429 disarms progress")
// A success still inside the penalty window must not re-arm (would let a
// burst of in-flight successes start a fresh episode too early).
_, _ = f.shouldRetry(ctx, respOK, nil)
assert.False(t, f.throttle.progress.Load(), "success during penalty window does not re-arm")
// Once the penalty window has elapsed, a success re-arms for the next episode.
f.throttle.penaltyUntilNano.Store(time.Now().Add(-time.Second).UnixNano())
_, _ = f.shouldRetry(ctx, respOK, nil)
assert.True(t, f.throttle.progress.Load(), "success after penalty window re-arms")
}
// TestThrottleStateConcurrent drives the lock-free throttleState (shared across
// the shallow Fs copy) from many goroutines. Like the registry test, `go test
// -race` is the real assertion: it would flag any non-atomic access. The final
// flag is interleaving-dependent, so it is deliberately not asserted.
func TestThrottleStateConcurrent(t *testing.T) {
ctx := context.Background()
f := newTestFs()
resp429 := &http.Response{StatusCode: 429, Header: http.Header{"Retry-After": {"1"}}}
respOK := &http.Response{StatusCode: 200}
var wg sync.WaitGroup
for i := range 64 {
wg.Add(1)
go func(i int) {
defer wg.Done()
if i%2 == 0 {
_, _ = f.shouldRetry(ctx, resp429, assert.AnError)
} else {
_, _ = f.shouldRetry(ctx, respOK, nil)
}
}(i)
}
wg.Wait()
}