Stop removing failed upload to cloud storage remotes - fixes #559

We do remove a partially written file on local so we don't have
corrupted files lying around.

amazonclouddrive/amazonclouddrive_test.go

@@ -27,6 +27,7 @@ func TestFsListEmpty(t *testing.T)         { fstests.TestFsListEmpty(t) }
 func TestFsListDirEmpty(t *testing.T)      { fstests.TestFsListDirEmpty(t) }
 func TestFsNewObjectNotFound(t *testing.T) { fstests.TestFsNewObjectNotFound(t) }
 func TestFsPutFile1(t *testing.T)          { fstests.TestFsPutFile1(t) }
+func TestFsPutError(t *testing.T)          { fstests.TestFsPutError(t) }
 func TestFsPutFile2(t *testing.T)          { fstests.TestFsPutFile2(t) }
 func TestFsUpdateFile1(t *testing.T)       { fstests.TestFsUpdateFile1(t) }
 func TestFsListDirFile2(t *testing.T)      { fstests.TestFsListDirFile2(t) }

b2/b2_test.go

@@ -27,6 +27,7 @@ func TestFsListEmpty(t *testing.T)         { fstests.TestFsListEmpty(t) }
 func TestFsListDirEmpty(t *testing.T)      { fstests.TestFsListDirEmpty(t) }
 func TestFsNewObjectNotFound(t *testing.T) { fstests.TestFsNewObjectNotFound(t) }
 func TestFsPutFile1(t *testing.T)          { fstests.TestFsPutFile1(t) }
+func TestFsPutError(t *testing.T)          { fstests.TestFsPutError(t) }
 func TestFsPutFile2(t *testing.T)          { fstests.TestFsPutFile2(t) }
 func TestFsUpdateFile1(t *testing.T)       { fstests.TestFsUpdateFile1(t) }
 func TestFsListDirFile2(t *testing.T)      { fstests.TestFsListDirFile2(t) }

crypt/crypt2_test.go

@@ -28,6 +28,7 @@ func TestFsListEmpty2(t *testing.T)         { fstests.TestFsListEmpty(t) }
 func TestFsListDirEmpty2(t *testing.T)      { fstests.TestFsListDirEmpty(t) }
 func TestFsNewObjectNotFound2(t *testing.T) { fstests.TestFsNewObjectNotFound(t) }
 func TestFsPutFile12(t *testing.T)          { fstests.TestFsPutFile1(t) }
+func TestFsPutError2(t *testing.T)          { fstests.TestFsPutError(t) }
 func TestFsPutFile22(t *testing.T)          { fstests.TestFsPutFile2(t) }
 func TestFsUpdateFile12(t *testing.T)       { fstests.TestFsUpdateFile1(t) }
 func TestFsListDirFile22(t *testing.T)      { fstests.TestFsListDirFile2(t) }

crypt/crypt_test.go

@@ -28,6 +28,7 @@ func TestFsListEmpty(t *testing.T)         { fstests.TestFsListEmpty(t) }
 func TestFsListDirEmpty(t *testing.T)      { fstests.TestFsListDirEmpty(t) }
 func TestFsNewObjectNotFound(t *testing.T) { fstests.TestFsNewObjectNotFound(t) }
 func TestFsPutFile1(t *testing.T)          { fstests.TestFsPutFile1(t) }
+func TestFsPutError(t *testing.T)          { fstests.TestFsPutError(t) }
 func TestFsPutFile2(t *testing.T)          { fstests.TestFsPutFile2(t) }
 func TestFsUpdateFile1(t *testing.T)       { fstests.TestFsUpdateFile1(t) }
 func TestFsListDirFile2(t *testing.T)      { fstests.TestFsListDirFile2(t) }

drive/drive_test.go

@@ -27,6 +27,7 @@ func TestFsListEmpty(t *testing.T)         { fstests.TestFsListEmpty(t) }
 func TestFsListDirEmpty(t *testing.T)      { fstests.TestFsListDirEmpty(t) }
 func TestFsNewObjectNotFound(t *testing.T) { fstests.TestFsNewObjectNotFound(t) }
 func TestFsPutFile1(t *testing.T)          { fstests.TestFsPutFile1(t) }
+func TestFsPutError(t *testing.T)          { fstests.TestFsPutError(t) }
 func TestFsPutFile2(t *testing.T)          { fstests.TestFsPutFile2(t) }
 func TestFsUpdateFile1(t *testing.T)       { fstests.TestFsUpdateFile1(t) }
 func TestFsListDirFile2(t *testing.T)      { fstests.TestFsListDirFile2(t) }

dropbox/dropbox_test.go

@@ -27,6 +27,7 @@ func TestFsListEmpty(t *testing.T)         { fstests.TestFsListEmpty(t) }
 func TestFsListDirEmpty(t *testing.T)      { fstests.TestFsListDirEmpty(t) }
 func TestFsNewObjectNotFound(t *testing.T) { fstests.TestFsNewObjectNotFound(t) }
 func TestFsPutFile1(t *testing.T)          { fstests.TestFsPutFile1(t) }
+func TestFsPutError(t *testing.T)          { fstests.TestFsPutError(t) }
 func TestFsPutFile2(t *testing.T)          { fstests.TestFsPutFile2(t) }
 func TestFsUpdateFile1(t *testing.T)       { fstests.TestFsUpdateFile1(t) }
 func TestFsListDirFile2(t *testing.T)      { fstests.TestFsListDirFile2(t) }

fs/operations.go

@@ -265,11 +265,6 @@ func Copy(f Fs, dst, src Object) (err error) {
 		// Retry if err returned a retry error
 		if IsRetryError(err) || ShouldRetry(err) {
 			Debug(src, "Received error: %v - low level retry %d/%d", err, tries, maxTries)
-			if removeFailedCopy(dst) {
-				// If we removed dst, then nil it out and note we are not updating
-				dst = nil
-				doUpdate = false
-			}
 			continue
 		}
 		// otherwise finish
@@ -278,7 +273,6 @@ func Copy(f Fs, dst, src Object) (err error) {
 	if err != nil {
 		Stats.Error()
 		ErrorLog(src, "Failed to copy: %v", err)
-		removeFailedCopy(dst)
 		return err
 	}
 

fstest/fstests/fstests.go

@@ -7,6 +7,7 @@ package fstests
 
 import (
 	"bytes"
+	"errors"
 	"flag"
 	"fmt"
 	"io"
@@ -235,6 +236,37 @@ func TestFsPutFile1(t *testing.T) {
 	file1Contents = testPut(t, &file1)
 }
 
+type errorReader struct {
+	err error
+}
+
+func (er errorReader) Read(p []byte) (n int, err error) {
+	return 0, er.err
+}
+
+// TestFsPutError tests uploading a file where there is an error
+//
+// It makes sure that aborting a file half way through does not create
+// a file on the remote.
+func TestFsPutError(t *testing.T) {
+	skipIfNotOk(t)
+
+	// Read 50 bytes then produce an error
+	contents := fstest.RandomString(50)
+	buf := bytes.NewBufferString(contents)
+	er := &errorReader{errors.New("potato")}
+	in := io.MultiReader(buf, er)
+
+	obji := fs.NewStaticObjectInfo(file2.Path, file2.ModTime, 100, true, nil, nil)
+	obj, err := remote.Put(in, obji)
+	// assert.Nil(t, obj) - FIXME some remotes return the object even on nil
+	assert.NotNil(t, err)
+
+	obj, err = remote.NewObject(file2.Path)
+	assert.Nil(t, obj)
+	assert.Equal(t, fs.ErrorObjectNotFound, err)
+}
+
 // TestFsPutFile2 tests putting a file into a subdirectory
 func TestFsPutFile2(t *testing.T) {
 	skipIfNotOk(t)

googlecloudstorage/googlecloudstorage_test.go

@@ -27,6 +27,7 @@ func TestFsListEmpty(t *testing.T)         { fstests.TestFsListEmpty(t) }
 func TestFsListDirEmpty(t *testing.T)      { fstests.TestFsListDirEmpty(t) }
 func TestFsNewObjectNotFound(t *testing.T) { fstests.TestFsNewObjectNotFound(t) }
 func TestFsPutFile1(t *testing.T)          { fstests.TestFsPutFile1(t) }
+func TestFsPutError(t *testing.T)          { fstests.TestFsPutError(t) }
 func TestFsPutFile2(t *testing.T)          { fstests.TestFsPutFile2(t) }
 func TestFsUpdateFile1(t *testing.T)       { fstests.TestFsUpdateFile1(t) }
 func TestFsListDirFile2(t *testing.T)      { fstests.TestFsListDirFile2(t) }

hubic/hubic_test.go

@@ -27,6 +27,7 @@ func TestFsListEmpty(t *testing.T)         { fstests.TestFsListEmpty(t) }
 func TestFsListDirEmpty(t *testing.T)      { fstests.TestFsListDirEmpty(t) }
 func TestFsNewObjectNotFound(t *testing.T) { fstests.TestFsNewObjectNotFound(t) }
 func TestFsPutFile1(t *testing.T)          { fstests.TestFsPutFile1(t) }
+func TestFsPutError(t *testing.T)          { fstests.TestFsPutError(t) }
 func TestFsPutFile2(t *testing.T)          { fstests.TestFsPutFile2(t) }
 func TestFsUpdateFile1(t *testing.T)       { fstests.TestFsUpdateFile1(t) }
 func TestFsListDirFile2(t *testing.T)      { fstests.TestFsListDirFile2(t) }

local/local.go

@@ -665,12 +665,16 @@ func (o *Object) Update(in io.Reader, src fs.ObjectInfo) error {
 	in = io.TeeReader(in, hash)
 
 	_, err = io.Copy(out, in)
-	outErr := out.Close()
+	closeErr := out.Close()
-	if err != nil {
+	if err == nil {
-		return err
+		err = closeErr
 	}
-	if outErr != nil {
+	if err != nil {
-		return outErr
+		fs.Debug(o, "Removing partially written file on error: %v", err)
+		if removeErr := os.Remove(o.path); removeErr != nil {
+			fs.ErrorLog(o, "Failed to remove partially written file: %v", removeErr)
+		}
+		return err
 	}
 
 	// All successful so update the hashes

local/local_test.go

@@ -27,6 +27,7 @@ func TestFsListEmpty(t *testing.T)         { fstests.TestFsListEmpty(t) }
 func TestFsListDirEmpty(t *testing.T)      { fstests.TestFsListDirEmpty(t) }
 func TestFsNewObjectNotFound(t *testing.T) { fstests.TestFsNewObjectNotFound(t) }
 func TestFsPutFile1(t *testing.T)          { fstests.TestFsPutFile1(t) }
+func TestFsPutError(t *testing.T)          { fstests.TestFsPutError(t) }
 func TestFsPutFile2(t *testing.T)          { fstests.TestFsPutFile2(t) }
 func TestFsUpdateFile1(t *testing.T)       { fstests.TestFsUpdateFile1(t) }
 func TestFsListDirFile2(t *testing.T)      { fstests.TestFsListDirFile2(t) }

onedrive/onedrive_test.go

@@ -27,6 +27,7 @@ func TestFsListEmpty(t *testing.T)         { fstests.TestFsListEmpty(t) }
 func TestFsListDirEmpty(t *testing.T)      { fstests.TestFsListDirEmpty(t) }
 func TestFsNewObjectNotFound(t *testing.T) { fstests.TestFsNewObjectNotFound(t) }
 func TestFsPutFile1(t *testing.T)          { fstests.TestFsPutFile1(t) }
+func TestFsPutError(t *testing.T)          { fstests.TestFsPutError(t) }
 func TestFsPutFile2(t *testing.T)          { fstests.TestFsPutFile2(t) }
 func TestFsUpdateFile1(t *testing.T)       { fstests.TestFsUpdateFile1(t) }
 func TestFsListDirFile2(t *testing.T)      { fstests.TestFsListDirFile2(t) }

s3/s3_test.go

@@ -27,6 +27,7 @@ func TestFsListEmpty(t *testing.T)         { fstests.TestFsListEmpty(t) }
 func TestFsListDirEmpty(t *testing.T)      { fstests.TestFsListDirEmpty(t) }
 func TestFsNewObjectNotFound(t *testing.T) { fstests.TestFsNewObjectNotFound(t) }
 func TestFsPutFile1(t *testing.T)          { fstests.TestFsPutFile1(t) }
+func TestFsPutError(t *testing.T)          { fstests.TestFsPutError(t) }
 func TestFsPutFile2(t *testing.T)          { fstests.TestFsPutFile2(t) }
 func TestFsUpdateFile1(t *testing.T)       { fstests.TestFsUpdateFile1(t) }
 func TestFsListDirFile2(t *testing.T)      { fstests.TestFsListDirFile2(t) }

swift/swift_test.go

@@ -27,6 +27,7 @@ func TestFsListEmpty(t *testing.T)         { fstests.TestFsListEmpty(t) }
 func TestFsListDirEmpty(t *testing.T)      { fstests.TestFsListDirEmpty(t) }
 func TestFsNewObjectNotFound(t *testing.T) { fstests.TestFsNewObjectNotFound(t) }
 func TestFsPutFile1(t *testing.T)          { fstests.TestFsPutFile1(t) }
+func TestFsPutError(t *testing.T)          { fstests.TestFsPutError(t) }
 func TestFsPutFile2(t *testing.T)          { fstests.TestFsPutFile2(t) }
 func TestFsUpdateFile1(t *testing.T)       { fstests.TestFsUpdateFile1(t) }
 func TestFsListDirFile2(t *testing.T)      { fstests.TestFsListDirFile2(t) }

yandex/yandex_test.go

@@ -27,6 +27,7 @@ func TestFsListEmpty(t *testing.T)         { fstests.TestFsListEmpty(t) }
 func TestFsListDirEmpty(t *testing.T)      { fstests.TestFsListDirEmpty(t) }
 func TestFsNewObjectNotFound(t *testing.T) { fstests.TestFsNewObjectNotFound(t) }
 func TestFsPutFile1(t *testing.T)          { fstests.TestFsPutFile1(t) }
+func TestFsPutError(t *testing.T)          { fstests.TestFsPutError(t) }
 func TestFsPutFile2(t *testing.T)          { fstests.TestFsPutFile2(t) }
 func TestFsUpdateFile1(t *testing.T)       { fstests.TestFsUpdateFile1(t) }
 func TestFsListDirFile2(t *testing.T)      { fstests.TestFsListDirFile2(t) }