backend/protondrive: fix shouldRetry to enable pacer retry logic #9080
+67
−1
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Fix broken shouldRetry function that always returned false, completely disabling the pacer's retry mechanism. - Check context cancellation via fserrors.ContextError - Retry transient storage block errors (Code=200501) - Retry rate limiting (429) and server errors (5xx) - Fall back to fserrors.ShouldRetry for network-level errors
Add table-driven tests covering: - nil error handling - context cancellation - Proton-specific retriable errors (Code=200501) - rate limiting (429) and server errors (5xx) - non-retriable client errors (400, 404)
Member
|
Looking good so far - carry on please :-) |
Author
Thank you, just did a manual test and it worked as expected. Marked as ready for review 🤞 Next going to take a crack at the upstream issue in the Proton API Bridge |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
Implement
shouldRetry, replacing an alwaysfalsestubProblem
The
shouldRetryfunction was a stub returningfalseunconditionally, preventing the pacer from retrying transient errors. Users see failures like:422 POST .../storage/blocks: Operation failed: Please retry (Code=200501)These should be automatically retried but aren't.
See also:
https://forum.rclone.org/t/rclone-uploads-failing-to-proton-drive/52545
Changes
shouldRetryfollowing pattern from other backends (box, dropbox)fserrors.ContextErrorfserrors.ShouldRetryfor network-level errorsTest plan
golangci-lintpassesgo buildandgo testpassManual test results
Tested with real Proton Drive account. The fix correctly detects and retries Code=200501 errors:
Note: Uploads still fail due to a separate orphaned draft issue (Code=2500/2501) in the upstream Proton-API-Bridge library. This is a known limitation tracked at henrybear327/Proton-API-Bridge#16 and requires ClientUID support to resolve. The
shouldRetryfix in this PR works as intended.Notes
Reviewed
shouldRetryimplementations across other backends (swift, dropbox, ftp, putio, filescom, box). This implementation follows the established pattern: context check first, backend-specific error detection, then fallback tofserrors.ShouldRetry. String-based error matching mirrors dropbox's approach, which is appropriate when the underlying SDK doesn't expose typed errors.