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.
This commit is contained in:
+29
-12
@@ -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 {
|
||||
|
||||
+9
-20
@@ -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
|
||||
|
||||
Reference in New Issue
Block a user