move lock release to last

google · Dec 26, 2023 · dc2378b · dc2378b
1 parent d204682
commit dc2378b
Show file tree

Hide file tree

Showing 2 changed files with 29 additions and 21 deletions.
diff --git a/engine/src/main/java/com/google/android/fhir/sync/FhirSyncWorker.kt b/engine/src/main/java/com/google/android/fhir/sync/FhirSyncWorker.kt
@@ -61,7 +61,7 @@ abstract class FhirSyncWorker(appContext: Context, workerParams: WorkerParameter
   internal open fun getDataSource() = FhirEngineProvider.getDataSource(applicationContext)
 
   override suspend fun doWork(): Result {
-    if (!lock.tryLock()) {
+    if (!syncWorkerLock.tryLock()) {
       return Result.retry()
     }
     val dataSource =
@@ -104,28 +104,36 @@ abstract class FhirSyncWorker(appContext: Context, workerParams: WorkerParameter
         }
       }
 
-    val result = synchronizer.synchronize()
-    val output = buildWorkData(result)
+    try {
+      val result = synchronizer.synchronize()
+      val output = buildWorkData(result)
 
-    // await/join is needed to collect states completely
-    kotlin.runCatching { job.join() }.onFailure(Timber::w)
+      // await/join is needed to collect states completely
+      kotlin.runCatching { job.join() }.onFailure(Timber::w)
 
-    setProgress(output)
+      setProgress(output)
 
-    Timber.d("Received result from worker $result and sending output $output")
+      Timber.d("Received result from worker $result and sending output $output")
 
-    lock.unlock()
-
-    return when (result) {
-      is SyncJobStatus.Finished -> Result.success(output)
-      else -> {
-        /**
-         * In case of failure, we can check if its worth retrying and do retry based on
-         * [RetryConfiguration.maxRetries] set by user.
-         */
-        val retries = inputData.getInt(MAX_RETRIES_ALLOWED, 0)
-        if (retries > runAttemptCount) Result.retry() else Result.failure(output)
+      return when (result) {
+        is SyncJobStatus.Finished -> Result.success(output)
+        else -> {
+          /**
+           * In case of failure, we can check if its worth retrying and do retry based on
+           * [RetryConfiguration.maxRetries] set by user.
+           */
+          val retries = inputData.getInt(MAX_RETRIES_ALLOWED, 0)
+          if (retries > runAttemptCount) Result.retry() else Result.failure(output)
+        }
       }
+    } catch (exception: Exception) {
+      setProgress(
+        buildWorkData(SyncJobStatus.Failed(listOf(ResourceSyncException(null, exception)))),
+      )
+      Timber.e("FhirSyncWorker encountered an exception: $exception")
+      return Result.retry()
+    } finally {
+      syncWorkerLock.unlock()
     }
   }
 
@@ -156,8 +164,8 @@ abstract class FhirSyncWorker(appContext: Context, workerParams: WorkerParameter
   }
 
   companion object {
-    private val lock = ReentrantLock()
+    private val syncWorkerLock = ReentrantLock()
 
-    fun isLocked() = lock.isLocked
+    fun isLocked() = syncWorkerLock.isLocked
   }
 }
diff --git a/engine/src/main/java/com/google/android/fhir/sync/FhirSynchronizer.kt b/engine/src/main/java/com/google/android/fhir/sync/FhirSynchronizer.kt
@@ -42,7 +42,7 @@ private sealed class SyncResult {
   data class Error(val exceptions: List<ResourceSyncException>) : SyncResult()
 }
 
-data class ResourceSyncException(val resourceType: ResourceType, val exception: Exception)
+data class ResourceSyncException(val resourceType: ResourceType?, val exception: Exception)
 
 internal data class UploadConfiguration(
   val uploader: Uploader,