Repository: rclone/rclone
Pull request #3954 (2020-02-11 16:39:36)

backend/s3: Added 500 as retryErrorCode
The error code 500 Internal Error indicates that Amazon S3 is unable to handle the request at that time. The error code 503 Slow Down typically indicates that the requests to the S3 bucket are very high, exceeding the request rates described in Request Rate and Performance Guidelines.

Because Amazon S3 is a distributed service, a very small percentage of 5xx errors are expected during normal use of the service. All requests that return 5xx errors from Amazon S3 can and should be retried, so we recommend that applications making requests to Amazon S3 have a fault-tolerance mechanism to recover from these errors.

https://aws.amazon.com/premiumsupport/knowledge-center/http-5xx-errors-s3/


#### What is the purpose of this change?

Register 500 as retryErrorCode in s3 backend.

#### Checklist

- [x] I have read the [contribution guidelines](https://github.com/rclone/rclone/blob/master/CONTRIBUTING.md#submitting-a-pull-request).
- [x] All commit messages are in [house style](https://github.com/rclone/rclone/blob/master/CONTRIBUTING.md#commit-messages).
- [x] I'm done, this Pull Request is ready for review :-)



Patch:
diff --git a/backend/cache/directory.go b/backend/cache/directory.go
index c95377712..2f03eb938 100644
--- a/backend/cache/directory.go
+++ b/backend/cache/directory.go
@@ -101,15 +101,6 @@ func (d *Directory) abs() string {
 	return cleanPath(path.Join(d.Dir, d.Name))
 }
 
-// parentRemote returns the absolute path parent remote
-func (d *Directory) parentRemote() string {
-	absPath := d.abs()
-	if absPath == "" {
-		return ""
-	}
-	return cleanPath(path.Dir(absPath))
-}
-
 // ModTime returns the cached ModTime
 func (d *Directory) ModTime(ctx context.Context) time.Time {
 	return time.Unix(0, d.CacheModTime)
diff --git a/backend/cache/storage_persistent.go b/backend/cache/storage_persistent.go
index 6221616f2..239dca02f 100644
--- a/backend/cache/storage_persistent.go
+++ b/backend/cache/storage_persistent.go
@@ -767,31 +767,6 @@ func (b *Persistent) iterateBuckets(buk *bolt.Bucket, bucketFn func(name string)
 	return err
 }
 
-func (b *Persistent) dumpRoot() string {
-	var itBuckets func(buk *bolt.Bucket) map[string]interface{}
-
-	itBuckets = func(buk *bolt.Bucket) map[string]interface{} {
-		m := make(map[string]interface{})
-		c := buk.Cursor()
-		for k, v := c.First(); k != nil; k, v = c.Next() {
-			if v == nil {
-				buk2 := buk.Bucket(k)
-				m[string(k)] = itBuckets(buk2)
-			} else {
-				m[string(k)] = "-"
-			}
-		}
-		return m
-	}
-	var mm map[string]interface{}
-	_ = b.db.View(func(tx *bolt.Tx) error {
-		mm = itBuckets(tx.Bucket([]byte(RootBucket)))
-		return nil
-	})
-	raw, _ := json.MarshalIndent(mm, "", "  ")
-	return string(raw)
-}
-
 // addPendingUpload adds a new file to the pending queue of uploads
 func (b *Persistent) addPendingUpload(destPath string, started bool) error {
 	return b.db.Update(func(tx *bolt.Tx) error {
diff --git a/backend/s3/s3.go b/backend/s3/s3.go
index 07bc8780b..94462b4bc 100644
--- a/backend/s3/s3.go
+++ b/backend/s3/s3.go
@@ -951,7 +951,7 @@ func (f *Fs) Features() *fs.Features {
 // retryErrorCodes is a slice of error codes that we will retry
 // See: https://docs.aws.amazon.com/AmazonS3/latest/API/ErrorResponses.html
 var retryErrorCodes = []int{
-	// 409, // Conflict - various states that could be resolved on a retry
+	500, // Internal Server Error - "We encountered an internal error. Please try again."
 	503, // Service Unavailable/Slow Down - "Reduce your request rate"
 }
 
@@ -2165,6 +2165,12 @@ func (o *Object) uploadMultipart(ctx context.Context, req *s3.PutObjectInput, si
 			buf = make([]byte, partSize)
 		}
 
+		// Fail fast, in case an errgroup managed function returns an error
+		// gCtx is cancelled. There is no point in uploading all the other parts.
+		if gCtx.Err() != nil {
+			break
+		}
+
 		// Read the chunk
 		var n int
 		n, err = readers.ReadFill(in, buf) // this can never return 0, nil
diff --git a/docs/content/authors.md b/docs/content/authors.md
index 2056736fe..c4ab001c6 100644
--- a/docs/content/authors.md
+++ b/docs/content/authors.md
@@ -337,3 +337,4 @@ Contributors
   * Dave Koston <dave.koston@stackpath.com>
   * Durval Menezes <DurvalMenezes@users.noreply.github.com>
   * Tim Gallant <me@timgallant.us>
+  * Frederick Zhang <frederick888@tsundere.moe>
diff --git a/docs/content/onedrive.md b/docs/content/onedrive.md
index dc49e3580..c00be83fe 100644
--- a/docs/content/onedrive.md
+++ b/docs/content/onedrive.md
@@ -129,7 +129,7 @@ If you are having problems with them (E.g., seeing a lot of throttling), you can
 Client ID and Key by following the steps below:
 
 1. Open https://portal.azure.com/#blade/Microsoft_AAD_RegisteredApps/ApplicationsListBlade, then click `New registration`.
-2. Enter a name for your app, choose account type `Any Azure AD directory - Multitenant`, select `Web` in `Redirect URI` Enter `http://localhost:53682/` and click Register. Copy and keep the `Application (client) ID` under the app name for later use.
+2. Enter a name for your app, choose account type `Accounts in any organizational directory (Any Azure AD directory - Multitenant) and personal Microsoft accounts (e.g. Skype, Xbox)`, select `Web` in `Redirect URI` Enter `http://localhost:53682/` and click Register. Copy and keep the `Application (client) ID` under the app name for later use.
 3. Under `manage` select `Certificates & secrets`, click `New client secret`. Copy and keep that secret for later use.
 4. Under `manage` select `API permissions`, click `Add a permission` and select `Microsoft Graph` then select `delegated permissions`.
 5. Search and select the follwing permssions: `Files.Read`, `Files.ReadWrite`, `Files.Read.All`, `Files.ReadWrite.All`, `offline_access`, `User.Read`. Once selected click `Add permissions` at the bottom.


Test patch:
diff --git a/backend/cache/cache_internal_test.go b/backend/cache/cache_internal_test.go
index d183f2a23..b6ee2a057 100644
--- a/backend/cache/cache_internal_test.go
+++ b/backend/cache/cache_internal_test.go
@@ -17,7 +17,6 @@ import (
 	"path/filepath"
 	"runtime"
 	"runtime/debug"
-	"strconv"
 	"strings"
 	"testing"
 	"time"
@@ -1139,23 +1138,6 @@ func (r *run) randomReader(t *testing.T, size int64) io.ReadCloser {
 	return f
 }
 
-func (r *run) writeRemoteRandomBytes(t *testing.T, f fs.Fs, p string, size int64) string {
-	remote := path.Join(p, strconv.Itoa(rand.Int())+".bin")
-	// create some rand test data
-	testData := randStringBytes(int(size))
-
-	r.writeRemoteBytes(t, f, remote, testData)
-	return remote
-}
-
-func (r *run) writeObjectRandomBytes(t *testing.T, f fs.Fs, p string, size int64) fs.Object {
-	remote := path.Join(p, strconv.Itoa(rand.Int())+".bin")
-	// create some rand test data
-	testData := randStringBytes(int(size))
-
-	return r.writeObjectBytes(t, f, remote, testData)
-}
-
 func (r *run) writeRemoteString(t *testing.T, f fs.Fs, remote, content string) {
 	r.writeRemoteBytes(t, f, remote, []byte(content))
 }
@@ -1344,26 +1326,6 @@ func (r *run) list(t *testing.T, f fs.Fs, remote string) ([]interface{}, error)
 	return l, err
 }
 
-func (r *run) listPath(t *testing.T, f fs.Fs, remote string) []string {
-	var err error
-	var l []string
-	if r.useMount {
-		var list []os.FileInfo
-		list, err = ioutil.ReadDir(path.Join(r.mntDir, remote))
-		for _, ll := range list {
-			l = append(l, ll.Name())
-		}
-	} else {
-		var list fs.DirEntries
-		list, err = f.List(context.Background(), remote)
-		for _, ll := range list {
-			l = append(l, ll.Remote())
-		}
-	}
-	require.NoError(t, err)
-	return l
-}
-
 func (r *run) copyFile(t *testing.T, f fs.Fs, src, dst string) error {
 	in, err := os.Open(src)
 	if err != nil {
diff --git a/vfs/cache_test.go b/vfs/cache_test.go
index d6affea3c..65112e990 100644
--- a/vfs/cache_test.go
+++ b/vfs/cache_test.go
@@ -333,7 +333,7 @@ func TestCacheOpenMkdir(t *testing.T) {
 	assert.Equal(t, []string(nil), itemAsString(c))
 
 	// test directory does not exist
-	fi, err = os.Stat(filepath.Dir(p))
+	_, err = os.Stat(filepath.Dir(p))
 	require.True(t, os.IsNotExist(err))
 }
 
diff --git a/vfs/dir_test.go b/vfs/dir_test.go
index 61fe85715..51dcefd50 100644
--- a/vfs/dir_test.go
+++ b/vfs/dir_test.go
@@ -238,7 +238,7 @@ func TestDirStat(t *testing.T) {
 	assert.Equal(t, int64(14), node.Size())
 	assert.Equal(t, "file1", node.Name())
 
-	node, err = dir.Stat("not found")
+	_, err = dir.Stat("not found")
 	assert.Equal(t, ENOENT, err)
 }
 
@@ -293,7 +293,7 @@ func TestDirOpen(t *testing.T) {
 	assert.True(t, ok)
 	require.NoError(t, fd.Close())
 
-	fd, err = dir.Open(os.O_WRONLY)
+	_, err = dir.Open(os.O_WRONLY)
 	assert.Equal(t, EPERM, err)
 }
 
@@ -405,7 +405,7 @@ func TestDirRemove(t *testing.T) {
 	require.NoError(t, err)
 
 	// check directory is not there
-	node, err = vfs.Stat("dir")
+	_, err = vfs.Stat("dir")
 	assert.Equal(t, ENOENT, err)
 
 	// check the vfs
diff --git a/vfs/file_test.go b/vfs/file_test.go
index 7a07c9486..a36b5888e 100644
--- a/vfs/file_test.go
+++ b/vfs/file_test.go
@@ -238,7 +238,7 @@ func TestFileOpen(t *testing.T) {
 	_, ok = fd.(*WriteFileHandle)
 	assert.True(t, ok)
 
-	fd, err = file.Open(3)
+	_, err = file.Open(3)
 	assert.Equal(t, EPERM, err)
 }
 
diff --git a/vfs/vfs_test.go b/vfs/vfs_test.go
index bc2a5a3cd..ed3e2faf0 100644
--- a/vfs/vfs_test.go
+++ b/vfs/vfs_test.go
@@ -152,16 +152,16 @@ func TestVFSStat(t *testing.T) {
 	assert.True(t, node.IsFile())
 	assert.Equal(t, "file2", node.Name())
 
-	node, err = vfs.Stat("not found")
+	_, err = vfs.Stat("not found")
 	assert.Equal(t, os.ErrNotExist, err)
 
-	node, err = vfs.Stat("dir/not found")
+	_, err = vfs.Stat("dir/not found")
 	assert.Equal(t, os.ErrNotExist, err)
 
-	node, err = vfs.Stat("not found/not found")
+	_, err = vfs.Stat("not found/not found")
 	assert.Equal(t, os.ErrNotExist, err)
 
-	node, err = vfs.Stat("file1/under a file")
+	_, err = vfs.Stat("file1/under a file")
 	assert.Equal(t, os.ErrNotExist, err)
 }