protondrive: implement shouldRetry instead of always returning false
shouldRetry was a stub returning false unconditionally, which makes protondrive the only rclone backend that disables pacer-level retries entirely. Every other backend at minimum falls back to fserrors.ShouldRetry(err) so genuine transport-level transients (TCP resets, brief 5xx) get retried. - Use errors.As to unwrap proton.APIError instead of string matching - Retry transient storage block errors (Code=200501) - Retry server errors (5xx, except 503) - Skip 429 and 503 (handled by go-proton-api's resty retry layer via catchTooManyRequests / catchRetryAfter, which honours Retry-After) - Fall back to fserrors.ShouldRetry for non-API errors Co-authored-by: tomholford <tomholford@users.noreply.github.com>
This commit is contained in:
@@ -22,6 +22,7 @@ import (
|
||||
"github.com/rclone/rclone/fs/config/configmap"
|
||||
"github.com/rclone/rclone/fs/config/configstruct"
|
||||
"github.com/rclone/rclone/fs/config/obscure"
|
||||
"github.com/rclone/rclone/fs/fserrors"
|
||||
"github.com/rclone/rclone/fs/fshttp"
|
||||
"github.com/rclone/rclone/fs/hash"
|
||||
"github.com/rclone/rclone/lib/dircache"
|
||||
@@ -250,8 +251,31 @@ type Object struct {
|
||||
|
||||
// shouldRetry returns a boolean as to whether this err deserves to be
|
||||
// retried. It returns the err as a convenience
|
||||
//
|
||||
// 429 rate-limit and 503 responses are retried inside go-proton-api's resty
|
||||
// layer (see catchTooManyRequests / catchRetryAfter) using the Retry-After
|
||||
// header, so we do not retry them again here.
|
||||
func shouldRetry(ctx context.Context, err error) (bool, error) {
|
||||
return false, err
|
||||
if fserrors.ContextError(ctx, &err) {
|
||||
return false, err
|
||||
}
|
||||
if err == nil {
|
||||
return false, nil
|
||||
}
|
||||
var apiErr *proton.APIError
|
||||
if errors.As(err, &apiErr) {
|
||||
// Transient storage block error ("Please retry").
|
||||
if apiErr.Code == 200501 {
|
||||
fs.Debugf(nil, "Retrying storage block error: %v", err)
|
||||
return true, err
|
||||
}
|
||||
// Server errors. 503 is already handled by the SDK; retry the rest.
|
||||
if apiErr.Status >= 500 && apiErr.Status < 600 && apiErr.Status != 503 {
|
||||
return true, err
|
||||
}
|
||||
return false, err
|
||||
}
|
||||
return fserrors.ShouldRetry(err), err
|
||||
}
|
||||
|
||||
//------------------------------------------------------------------------------
|
||||
|
||||
@@ -1,8 +1,14 @@
|
||||
package protondrive
|
||||
|
||||
import (
|
||||
"context"
|
||||
"errors"
|
||||
"fmt"
|
||||
"regexp"
|
||||
"testing"
|
||||
|
||||
"github.com/rclone/go-proton-api"
|
||||
"github.com/stretchr/testify/assert"
|
||||
)
|
||||
|
||||
var protonDriveAppVersionPattern = regexp.MustCompile(`(?i)^external-drive(-[a-z_]+)+@[0-9]+\.[0-9]+\.[0-9]+(\.[0-9]+)?-((stable|beta|RC|alpha)(([.-]?\d+)*)?)?([.-]?dev)?(\+.*)?$`)
|
||||
@@ -53,3 +59,38 @@ func TestProtonDriveAppVersionFromRcloneVersion(t *testing.T) {
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
func TestShouldRetry(t *testing.T) {
|
||||
ctx := context.Background()
|
||||
cancelledCtx, cancel := context.WithCancel(ctx)
|
||||
cancel()
|
||||
|
||||
apiErr := func(status int, code proton.Code) error {
|
||||
return &proton.APIError{Status: status, Code: code, Message: "test"}
|
||||
}
|
||||
|
||||
for _, tc := range []struct {
|
||||
name string
|
||||
ctx context.Context
|
||||
err error
|
||||
wantRetry bool
|
||||
}{
|
||||
{"nil error", ctx, nil, false},
|
||||
{"cancelled context", cancelledCtx, errors.New("some error"), false},
|
||||
{"storage block error Code=200501", ctx, apiErr(422, 200501), true},
|
||||
{"server error Status=500", ctx, apiErr(500, 0), true},
|
||||
{"server error Status=502", ctx, apiErr(502, 0), true},
|
||||
{"server error Status=504", ctx, apiErr(504, 0), true},
|
||||
{"server error Status=503 (handled by SDK, not retried here)", ctx, apiErr(503, 0), false},
|
||||
{"rate limit Status=429 (handled by SDK, not retried here)", ctx, apiErr(429, 0), false},
|
||||
{"client error Status=400", ctx, apiErr(400, 0), false},
|
||||
{"client error Status=404", ctx, apiErr(404, 0), false},
|
||||
{"wrapped API error retried via errors.As", ctx, fmt.Errorf("wrapped: %w", &proton.APIError{Status: 500}), true},
|
||||
{"non-API error falls back to fserrors.ShouldRetry", ctx, errors.New("plain error"), false},
|
||||
} {
|
||||
t.Run(tc.name, func(t *testing.T) {
|
||||
gotRetry, _ := shouldRetry(tc.ctx, tc.err)
|
||||
assert.Equal(t, tc.wantRetry, gotRetry)
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user