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
This commit is contained in:
committed by
Nick Craig-Wood
parent
037340b1b5
commit
171e86369a
@@ -2,6 +2,7 @@ package zoho
|
||||
|
||||
import (
|
||||
"context"
|
||||
"errors"
|
||||
"net/http"
|
||||
"sync"
|
||||
"testing"
|
||||
@@ -73,6 +74,16 @@ func TestShouldRetry(t *testing.T) {
|
||||
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()
|
||||
@@ -83,6 +94,26 @@ func TestShouldRetry(t *testing.T) {
|
||||
})
|
||||
}
|
||||
|
||||
// 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.
|
||||
|
||||
+31
-2
@@ -663,12 +663,20 @@ func (f *Fs) logThrottle(wait time.Duration, err error) {
|
||||
}
|
||||
}
|
||||
|
||||
// isMissingResourceErr reports whether resp/err is Zoho's 401 "R008 Unauthorized
|
||||
// access" - returned (not a 404) for a resource id (folder or file) that was
|
||||
// deleted or never existed. A freshly refreshed token still gets it, so it is a
|
||||
// missing resource, not a token problem.
|
||||
func isMissingResourceErr(resp *http.Response, err error) bool {
|
||||
return resp != nil && resp.StatusCode == 401 && err != nil && strings.Contains(err.Error(), "R008")
|
||||
}
|
||||
|
||||
// shouldRetry reports whether the given resp and err deserve to be retried.
|
||||
//
|
||||
// A 429 is honoured via the Retry-After header (falling back to 60s plus a
|
||||
// margin) and starts or continues a throttling episode; expired OAuth tokens
|
||||
// are retried, missing OAuth scopes abort, and standard HTTP retry conditions
|
||||
// are also handled.
|
||||
// are also handled. A missing folder (R008) is not retried.
|
||||
//
|
||||
// Returns whether to retry, and the err as a convenience.
|
||||
func (f *Fs) shouldRetry(ctx context.Context, resp *http.Response, err error) (bool, error) {
|
||||
@@ -694,6 +702,11 @@ func (f *Fs) shouldRetry(ctx context.Context, resp *http.Response, err error) (b
|
||||
authRetry = true
|
||||
fs.Debugf(nil, "Should retry: %v", err)
|
||||
}
|
||||
|
||||
// A missing resource never reappears, and retrying escalates to a 429 F7008.
|
||||
if isMissingResourceErr(resp, err) {
|
||||
return false, err
|
||||
}
|
||||
if resp != nil && resp.StatusCode == 429 {
|
||||
// Zoho's listing API is heavily rate limited and tells us how long to
|
||||
// wait in the Retry-After header. Honour it so we don't retry too early
|
||||
@@ -772,6 +785,16 @@ func (f *Fs) readMetaDataForPath(ctx context.Context, path string) (info *api.It
|
||||
return false
|
||||
})
|
||||
if err != nil {
|
||||
if err == fs.ErrorDirNotFound {
|
||||
// The cached parent directory id is stale: its folder was deleted, so
|
||||
// listing it returned R008 (mapped to ErrorDirNotFound). Flush the stale
|
||||
// entry so a later create re-resolves (and recreates) the parent, and
|
||||
// report the object as not found - it cannot exist if its parent is gone.
|
||||
parent := strings.TrimSuffix(path[:len(path)-len(leaf)], "/")
|
||||
fs.Debugf(f, "readMetaDataForPath %q: parent %q stale (R008), flushing dircache", path, parent)
|
||||
f.dirCache.FlushDir(parent)
|
||||
return nil, fs.ErrorObjectNotFound
|
||||
}
|
||||
return nil, err
|
||||
}
|
||||
if !found {
|
||||
@@ -1000,7 +1023,13 @@ OUTER:
|
||||
return f.shouldRetry(ctx, resp, err)
|
||||
})
|
||||
if err != nil {
|
||||
return found, fmt.Errorf("couldn't list files: %w", err)
|
||||
// Surface a missing folder as directory-not-found so dircache/the VFS
|
||||
// treat it as gone instead of hard-failing on a stale id.
|
||||
if isMissingResourceErr(resp, err) {
|
||||
fs.Debugf(f, "listAll %q: R008 unauthorized - treating as directory not found", dirID)
|
||||
return false, fs.ErrorDirNotFound
|
||||
}
|
||||
return false, fmt.Errorf("couldn't list files: %w", err)
|
||||
}
|
||||
if len(result.Items) == 0 {
|
||||
break
|
||||
|
||||
Reference in New Issue
Block a user