From a4972de505afc1cbef45a3aa338a4df0ad905b86 Mon Sep 17 00:00:00 2001 From: Nick Craig-Wood Date: Mon, 20 Jul 2026 17:21:33 +0100 Subject: [PATCH] shade: retry server errors instead of failing the transfer The pacer callbacks only retried 429 responses, so transient server errors like 502 Bad Gateway failed transfers immediately. Retry the standard retryable status codes and transport errors everywhere, make chunk upload retries resend the whole chunk, and fix a potential nil pointer dereference when logging a bad token response. --- backend/shade/shade.go | 41 +++++++++++++++++++++++++++++------------ backend/shade/upload.go | 29 +++++++++-------------------- 2 files changed, 38 insertions(+), 32 deletions(-) diff --git a/backend/shade/shade.go b/backend/shade/shade.go index dc8d1c64e..eacbdc37b 100644 --- a/backend/shade/shade.go +++ b/backend/shade/shade.go @@ -22,6 +22,7 @@ import ( "github.com/rclone/rclone/fs/config" "github.com/rclone/rclone/fs/config/configmap" "github.com/rclone/rclone/fs/config/configstruct" + "github.com/rclone/rclone/fs/fserrors" "github.com/rclone/rclone/fs/fshttp" "github.com/rclone/rclone/fs/hash" "github.com/rclone/rclone/fs/object" @@ -42,6 +43,25 @@ const ( maxUploadParts = 10000 // maximum allowed number of parts in a multipart upload ) +// retryErrorCodes is a slice of error codes that we will retry +var retryErrorCodes = []int{ + 429, // Too Many Requests. + 500, // Internal Server Error + 502, // Bad Gateway + 503, // Service Unavailable + 504, // Gateway Timeout + 509, // Bandwidth Limit Exceeded +} + +// shouldRetry returns a boolean as to whether this resp and err +// deserve to be retried. It returns the err as a convenience +func shouldRetry(ctx context.Context, resp *http.Response, err error) (bool, error) { + if fserrors.ContextError(ctx, &err) { + return false, err + } + return fserrors.ShouldRetry(err) || fserrors.ShouldRetryHTTP(resp, retryErrorCodes), err +} + // Register with Fs func init() { fs.Register(&fs.RegInfo{ @@ -126,14 +146,14 @@ func (f *Fs) refreshJWTToken(ctx context.Context) (string, error) { res, err := f.apiSrv.Call(ctx, &opts) if err != nil { fs.Debugf(f, "Token request failed: %v", err) - return false, err + return shouldRetry(ctx, res, err) } defer fs.CheckClose(res.Body, &err) if res.StatusCode != http.StatusOK { fs.Debugf(f, "Token request failed with code: %d", res.StatusCode) - return res.StatusCode == http.StatusTooManyRequests, fmt.Errorf("failed to get ShadeFS token, status: %d", res.StatusCode) + return fserrors.ShouldRetryHTTP(res, retryErrorCodes), fmt.Errorf("failed to get ShadeFS token, status: %d", res.StatusCode) } // Read token directly as plain text @@ -202,10 +222,7 @@ func (f *Fs) callAPI(ctx context.Context, method, path string, response any) (*h } else { res, err = f.srv.Call(ctx, &opts) } - if err != nil { - return res != nil && res.StatusCode == http.StatusTooManyRequests, err - } - return false, nil + return shouldRetry(ctx, res, err) }) return res, err } @@ -334,11 +351,11 @@ func (f *Fs) Move(ctx context.Context, src fs.Object, remote string) (fs.Object, err = o.fs.pacer.Call(func() (bool, error) { resp, err := f.srv.Call(ctx, &opts) - if err != nil && resp.StatusCode == http.StatusBadRequest { + if err != nil && resp != nil && resp.StatusCode == http.StatusBadRequest { fs.Debugf(f, "Bad token from server: %v", token) } - return resp != nil && resp.StatusCode == http.StatusTooManyRequests, err + return shouldRetry(ctx, resp, err) }) if err != nil { return nil, err @@ -860,9 +877,9 @@ func (o *Object) Open(ctx context.Context, options ...fs.OpenOption) (io.ReadClo } res, err = client.Do(req) if err != nil { - return false, err + return shouldRetry(ctx, nil, err) } - return res.StatusCode == http.StatusTooManyRequests, nil + return fserrors.ShouldRetryHTTP(res, retryErrorCodes), nil }) if err != nil { @@ -899,12 +916,12 @@ func (o *Object) Open(ctx context.Context, options ...fs.OpenOption) (io.ReadClo err = o.fs.pacer.Call(func() (bool, error) { downloadRes, err = client.Call(ctx, &opts) if err != nil { - return false, err + return shouldRetry(ctx, downloadRes, err) } if downloadRes == nil { return false, fmt.Errorf("failed to fetch presigned URL") } - return downloadRes.StatusCode == http.StatusTooManyRequests, nil + return fserrors.ShouldRetryHTTP(downloadRes, retryErrorCodes), nil }) if err != nil { diff --git a/backend/shade/upload.go b/backend/shade/upload.go index 0419f42d1..89f62bac2 100644 --- a/backend/shade/upload.go +++ b/backend/shade/upload.go @@ -17,6 +17,7 @@ import ( "github.com/rclone/rclone/backend/shade/api" "github.com/rclone/rclone/fs" "github.com/rclone/rclone/fs/chunksize" + "github.com/rclone/rclone/fs/fserrors" "github.com/rclone/rclone/lib/multipart" "github.com/rclone/rclone/lib/rest" ) @@ -125,10 +126,7 @@ func (f *Fs) OpenChunkWriter(ctx context.Context, remote string, src fs.ObjectIn err = o.fs.pacer.Call(func() (bool, error) { res, err := o.fs.srv.CallJSON(ctx, &opts, reqBody, &initResp) - if err != nil { - return res != nil && res.StatusCode == http.StatusTooManyRequests, err - } - return false, nil + return shouldRetry(ctx, res, err) }) if err != nil { @@ -183,10 +181,7 @@ func (s *shadeChunkWriter) WriteChunk(ctx context.Context, chunkNumber int, read err = s.f.pacer.Call(func() (bool, error) { res, err := s.f.srv.CallJSON(ctx, &partOpts, nil, &partURL) - if err != nil { - return res != nil && res.StatusCode == http.StatusTooManyRequests, err - } - return false, nil + return shouldRetry(ctx, res, err) }) if err != nil { @@ -195,7 +190,6 @@ func (s *shadeChunkWriter) WriteChunk(ctx context.Context, chunkNumber int, read opts := rest.Opts{ Method: "PUT", RootURL: partURL.URL, - Body: &chunk, ContentType: "", ContentLength: &n, } @@ -208,11 +202,10 @@ func (s *shadeChunkWriter) WriteChunk(ctx context.Context, chunkNumber int, read } err = s.f.pacer.Call(func() (bool, error) { + // Use a fresh reader for each attempt so retries resend the whole chunk + opts.Body = bytes.NewReader(chunk.Bytes()) uploadRes, err = s.f.srv.Call(ctx, &opts) - if err != nil { - return uploadRes != nil && uploadRes.StatusCode == http.StatusTooManyRequests, err - } - return false, nil + return shouldRetry(ctx, uploadRes, err) }) if err != nil { @@ -276,17 +269,13 @@ func (s *shadeChunkWriter) Close(ctx context.Context) error { err = s.f.pacer.Call(func() (bool, error) { res, err := s.f.srv.CallJSON(ctx, &completeOpts, completeBody, &response) - if err != nil && res == nil { - return false, err - } - - if res.StatusCode == http.StatusTooManyRequests { - return true, err // Retry on 429 + if err != nil { + return shouldRetry(ctx, res, err) } if res.StatusCode != http.StatusOK && res.StatusCode != http.StatusCreated { body, _ := io.ReadAll(res.Body) - return false, fmt.Errorf("complete multipart failed with status %d: %s", res.StatusCode, string(body)) + return fserrors.ShouldRetryHTTP(res, retryErrorCodes), fmt.Errorf("complete multipart failed with status %d: %s", res.StatusCode, string(body)) } return false, nil