diff --git a/backend/protondrive/protondrive.go b/backend/protondrive/protondrive.go index 9b54fef4a..9188383cf 100644 --- a/backend/protondrive/protondrive.go +++ b/backend/protondrive/protondrive.go @@ -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 } //------------------------------------------------------------------------------ diff --git a/backend/protondrive/protondrive_internal_test.go b/backend/protondrive/protondrive_internal_test.go index 7c6751244..aaf2945ae 100644 --- a/backend/protondrive/protondrive_internal_test.go +++ b/backend/protondrive/protondrive_internal_test.go @@ -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) + }) + } +}