diff --git a/backend/zoho/throttle_test.go b/backend/zoho/throttle_test.go index b73ab4187..445f45940 100644 --- a/backend/zoho/throttle_test.go +++ b/backend/zoho/throttle_test.go @@ -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. diff --git a/backend/zoho/zoho.go b/backend/zoho/zoho.go index 453fde539..4d4f26ac8 100644 --- a/backend/zoho/zoho.go +++ b/backend/zoho/zoho.go @@ -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