Merge pull request #17229 from nextcloud/backport/17191/stable-34.0.x

[stable-34.0.x] fix(file-upload-worker): better error handling

Author: alperozturk96

app/src/main/java/com/nextcloud/client/jobs/upload/FileUploadWorker.kt

@@ -26,6 +26,7 @@ import com.nextcloud.client.network.ConnectivityService
 import com.nextcloud.client.preferences.AppPreferences
 import com.nextcloud.utils.ForegroundServiceHelper
 import com.nextcloud.utils.extensions.getPercent
+import com.nextcloud.utils.extensions.isNonRetryable
 import com.nextcloud.utils.extensions.toFile
 import com.owncloud.android.R
 import com.owncloud.android.datamodel.ForegroundServiceType
@@ -34,6 +35,7 @@ import com.owncloud.android.datamodel.SyncedFolderProvider
 import com.owncloud.android.datamodel.ThumbnailsCacheManager
 import com.owncloud.android.datamodel.UploadsStorageManager
 import com.owncloud.android.db.OCUpload
+import com.owncloud.android.db.UploadResult
 import com.owncloud.android.lib.common.OwnCloudAccount
 import com.owncloud.android.lib.common.OwnCloudClient
 import com.owncloud.android.lib.common.OwnCloudClientManagerFactory
@@ -204,6 +206,18 @@ class FileUploadWorker(
             .setSilent(true)
             .build()
 
+    private enum class UploadFilesResult {
+        Success,
+        Error,
+        Retry;
+
+        fun toWorkerResult(): Result = when (this) {
+            Success -> Result.success()
+            Error -> Result.failure()
+            Retry -> Result.retry()
+        }
+    }
+
     @Suppress("ReturnCount", "LongMethod", "DEPRECATION")
     private suspend fun uploadFiles(): Result = withContext(Dispatchers.IO) {
         val accountName = inputData.getString(ACCOUNT)
@@ -246,6 +260,7 @@ class FileUploadWorker(
         val client = OwnCloudClientManagerFactory.getDefaultSingleton().getClientFor(ocAccount, context)
         val syncFolderHelper = SyncFolderHelper(context)
         val syncedFolders = syncedFolderProvider.syncedFolders
+        var uploadFilesResult = UploadFilesResult.Success
 
         for ((index, upload) in uploads.withIndex()) {
             ensureActive()
@@ -288,20 +303,33 @@ class FileUploadWorker(
             )
 
             val result = withContext(Dispatchers.IO) {
-                upload(upload, operation, user, client)
+                upload(operation, user, client)
             }
             activeOperations.remove(upload.uploadId)
 
+            // check quota first
             if (result.code == ResultCode.QUOTA_EXCEEDED) {
                 Log_OC.w(TAG, "Quota exceeded, stopping uploads")
                 notificationManager.showQuotaExceedNotification(operation)
                 break
             }
 
+            // check upload result for worker
+            val uploadResult = UploadResult.fromOperationResult(result)
+            if (!result.isSuccess) {
+                Log_OC.e(TAG, "upload failed for ${upload.remotePath}: ${result.code}")
+                if (uploadResult.isNonRetryable()) {
+                    uploadFilesResult = UploadFilesResult.Error
+                } else if (uploadFilesResult != UploadFilesResult.Error) {
+                    // only set retry if any other not failed before
+                    uploadFilesResult = UploadFilesResult.Retry
+                }
+            }
+
             sendUploadFinishEvent(totalUploadSize, currentUploadIndex, operation, result)
         }
 
-        return@withContext Result.success()
+        return@withContext uploadFilesResult.toWorkerResult()
     }
 
     @Suppress("ReturnCount")
@@ -356,21 +384,24 @@ class FileUploadWorker(
 
     @Suppress("TooGenericExceptionCaught", "DEPRECATION")
     private suspend fun upload(
-        upload: OCUpload,
         operation: UploadFileOperation,
         user: User,
         client: OwnCloudClient
     ): RemoteOperationResult<Any?> = withContext(Dispatchers.IO) {
-        lateinit var result: RemoteOperationResult<Any?>
+        var result: RemoteOperationResult<Any?>
 
         try {
             val storageManager = operation.storageManager
-            result = operation.execute(client)
-            val task = ThumbnailsCacheManager.ThumbnailGenerationTask(storageManager, user)
-            val file = File(operation.originalStoragePath)
-            val remoteId: String? = operation.file.remoteId
-            task.execute(ThumbnailsCacheManager.ThumbnailGenerationTaskObject(file, remoteId))
             fileUploadEventBroadcaster.sendUploadStarted(operation, context)
+            result = operation.execute(client)
+
+            // only generate a thumbnail if the upload actually succeeded
+            if (result.isSuccess) {
+                val task = ThumbnailsCacheManager.ThumbnailGenerationTask(storageManager, user)
+                val file = File(operation.originalStoragePath)
+                val remoteId: String? = operation.file.remoteId
+                task.execute(ThumbnailsCacheManager.ThumbnailGenerationTaskObject(file, remoteId))
+            }
         } catch (e: Exception) {
             Log_OC.e(TAG, "Error uploading", e)
             result = RemoteOperationResult(e)
@@ -405,7 +436,7 @@ class FileUploadWorker(
     private var lastUpdateTime = 0L
 
     /**
-     * Receives from [com.owncloud.android.operations.UploadFileOperation.normalUpload]
+     * Receives from [UploadFileOperation.normalUpload]
      */
     @Suppress("MagicNumber")
     override fun onTransferProgress(

gradle/verification-metadata.xml

@@ -20804,6 +20804,14 @@
             <sha256 value="9ee93f81089c2f4b3841a7efa5c77f845bf13811d5d6a340d35cb960537cfb10" origin="Generated by Gradle" reason="Artifact is not signed"/>
          </artifact>
       </component>
+      <component group="com.github.nextcloud" name="android-library" version="0282f675c8f11af9f1da91436f1fa95abe2333fe">
+         <artifact name="android-library-0282f675c8f11af9f1da91436f1fa95abe2333fe.aar">
+            <sha256 value="bddfb0de83c7ff95220cc804bf6e2ccfa3634ccecce7b0e34403f84f311d0187" origin="Generated by Gradle" reason="Artifact is not signed"/>
+         </artifact>
+         <artifact name="android-library-0282f675c8f11af9f1da91436f1fa95abe2333fe.module">
+            <sha256 value="c0cbe0bb8321a22135cd376242ef6ee39615ad984214934dd703187b75b78801" origin="Generated by Gradle" reason="Artifact is not signed"/>
+         </artifact>
+      </component>
       <component group="com.github.nextcloud" name="android-library" version="0303be50f9">
          <artifact name="android-library-0303be50f9.aar">
             <sha256 value="d10959e216de68719efc582c5698b7e78385a53900672cb024ecc36111be6d2f" origin="Generated by Gradle" reason="Artifact is not signed"/>
@@ -20932,6 +20940,14 @@
             <sha256 value="324ae2a4961a86e623e9112db4db616b589526834cf37a709ca932c865f4843b" origin="Generated by Gradle"/>
          </artifact>
       </component>
+      <component group="com.github.nextcloud" name="android-library" version="1bdcdd40eb">
+         <artifact name="android-library-1bdcdd40eb.aar">
+            <sha256 value="dea88ca0af50d654bb20c44165a0a09d92cca3e984b03f286d63cd9de5e16340" origin="Generated by Gradle" reason="Artifact is not signed"/>
+         </artifact>
+         <artifact name="android-library-1bdcdd40eb.module">
+            <sha256 value="e36ca21dec5c8da36d526c4f463b0f47cabe3aa7967f57463208f24ce128e3b8" origin="Generated by Gradle" reason="Artifact is not signed"/>
+         </artifact>
+      </component>
       <component group="com.github.nextcloud" name="android-library" version="1c847678482c739d0b92e27a445d2f5f168f7d33">
          <artifact name="android-library-1c847678482c739d0b92e27a445d2f5f168f7d33.aar">
             <sha256 value="2fbe1710b99ab094a4a0c7d1fb6cb32241dfc78188af994fd1ea6e214a754d84" origin="Generated by Gradle" reason="Artifact is not signed"/>
@@ -21388,6 +21404,14 @@
             <sha256 value="12e69eebf36959a9fc9c4ae61343ec17e74f5af581accee3a14a6b8b4adacc54" origin="Generated by Gradle" reason="Artifact is not signed"/>
          </artifact>
       </component>
+      <component group="com.github.nextcloud" name="android-library" version="64bab56dce">
+         <artifact name="android-library-64bab56dce.aar">
+            <sha256 value="57510727181131abcd62696b4de737b5769df4de9276dc868335f8caf65aa547" origin="Generated by Gradle" reason="Artifact is not signed"/>
+         </artifact>
+         <artifact name="android-library-64bab56dce.module">
+            <sha256 value="9c2fa4cf6b957dd26184d88ba952fa6ae1aacdc3b959fd36c7773edd7041eda4" origin="Generated by Gradle" reason="Artifact is not signed"/>
+         </artifact>
+      </component>
       <component group="com.github.nextcloud" name="android-library" version="67f7bf5eb832b5fa24f06d9b84db0e21d4c8638c">
          <artifact name="android-library-67f7bf5eb832b5fa24f06d9b84db0e21d4c8638c.aar">
             <sha256 value="c253a126ca3c32dd5b373ab1c64668e4603305b3113b052fc0fc5e3c833a913c" origin="Generated by Gradle" reason="Artifact is not signed"/>