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
165 lines
6.1 KiB
Go
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()
|
|
}
|