diff --git a/infra/event-sync/src/main/java/com/simprints/infra/eventsync/sync/up/tasks/EventUpSyncTask.kt b/infra/event-sync/src/main/java/com/simprints/infra/eventsync/sync/up/tasks/EventUpSyncTask.kt index bb7b4359e4..25f1d00be7 100644 --- a/infra/event-sync/src/main/java/com/simprints/infra/eventsync/sync/up/tasks/EventUpSyncTask.kt +++ b/infra/event-sync/src/main/java/com/simprints/infra/eventsync/sync/up/tasks/EventUpSyncTask.kt @@ -42,6 +42,7 @@ import com.simprints.infra.network.exceptions.BackendMaintenanceException import com.simprints.infra.network.exceptions.NetworkConnectionException import com.simprints.infra.network.exceptions.SyncCloudIntegrationException import com.simprints.infra.serialization.SimJson +import kotlinx.coroutines.CancellationException import kotlinx.coroutines.currentCoroutineContext import kotlinx.coroutines.flow.Flow import kotlinx.coroutines.flow.FlowCollector @@ -226,6 +227,7 @@ internal class EventUpSyncTask @Inject constructor( .let(eventFilter) .map { (scope, events) -> mapDomainEventScopeToApiUseCase(scope, events.orEmpty(), project) } val uploadedScopes = mutableListOf() + var isNetworkUnavailable = false scopesToUpload.takeIf { it.isNotEmpty() }?.apply { val requestId = UUID.randomUUID().toString() @@ -245,13 +247,23 @@ internal class EventUpSyncTask @Inject constructor( content = createUpSyncContent(this.size), ) uploadedScopes.addAll(this.map { it.id }) + } catch (ex: CancellationException) { + throw ex } catch (ex: Exception) { + isNetworkUnavailable = ex is NetworkConnectionException handleFailedRequest(requestId, ex, eventScope, requestStartTime) } } - Simber.d("Deleting ${uploadedScopes.size} session scopes", tag = SYNC) + Simber.d("Deleting ${uploadedScopes.size} $eventScopeTypeToUpload scopes", tag = SYNC) eventRepository.deleteEventScopes(uploadedScopes) + + // Without connectivity the upload will keep failing immediately on every iteration, + // spinning the loop indefinitely and logging a new failure event each time. Stop + // retrying this scope type until the next scheduled sync attempt instead. + if (isNetworkUnavailable) { + break + } } } diff --git a/infra/event-sync/src/test/java/com/simprints/infra/eventsync/sync/up/tasks/EventUpSyncTaskTest.kt b/infra/event-sync/src/test/java/com/simprints/infra/eventsync/sync/up/tasks/EventUpSyncTaskTest.kt index cb1953b3cc..ded45834db 100644 --- a/infra/event-sync/src/test/java/com/simprints/infra/eventsync/sync/up/tasks/EventUpSyncTaskTest.kt +++ b/infra/event-sync/src/test/java/com/simprints/infra/eventsync/sync/up/tasks/EventUpSyncTaskTest.kt @@ -45,6 +45,7 @@ import com.simprints.infra.network.exceptions.NetworkConnectionException import com.simprints.infra.network.exceptions.SyncCloudIntegrationException import io.mockk.* import io.mockk.impl.annotations.MockK +import kotlinx.coroutines.CancellationException import kotlinx.coroutines.flow.toList import kotlinx.coroutines.test.runTest import kotlinx.serialization.SerializationException @@ -437,6 +438,83 @@ internal class EventUpSyncTaskTest { coVerify(exactly = 0) { eventRepo.deleteEventScope(any()) } } + @Test + fun `when upload fails due to network issue should stop retrying instead of looping indefinitely`() = runTest { + setUpSyncKind(UpSynchronizationConfiguration.UpSynchronizationKind.ALL) + + // Scopes remain closed forever since the failed upload is never deleted, + // simulating a device that stays offline. + coEvery { eventRepo.getClosedEventScopesCount(EventScopeType.SESSION) } returns 1 + coEvery { eventRepo.getClosedEventScopes(any(), any()) } returns emptyList() + coEvery { eventRepo.getClosedEventScopes(EventScopeType.SESSION, any()) } returns listOf( + createSessionScope(GUID1), + ) + coEvery { + eventRepo.getEventsFromScope(GUID1) + } returns listOf(createEventWithSessionId(GUID1, GUID1)) + + coEvery { + eventRemoteDataSource.post(any(), any(), any()) + } throws NetworkConnectionException( + cause = Exception(), + ) + + eventUpSyncTask.upSync(operation, eventScope).toList() + + // Only a single attempt (and a single failure event) should be recorded per sync run, + // instead of spinning in a tight loop and logging duplicate events while offline. + coVerify(exactly = 1) { eventRemoteDataSource.post(any(), any(), any()) } + coVerify(exactly = 1) { eventRepo.addOrUpdateEvent(eventScope, any()) } + } + + @Test + fun `when upload is cancelled it should rethrow instead of treating it as a failed request`() = runTest { + setUpSyncKind(UpSynchronizationConfiguration.UpSynchronizationKind.ALL) + + coEvery { eventRepo.getClosedEventScopesCount(EventScopeType.SESSION) } returns 1 + coEvery { eventRepo.getClosedEventScopes(any(), any()) } returns emptyList() + coEvery { eventRepo.getClosedEventScopes(EventScopeType.SESSION, any()) } returns listOf( + createSessionScope(GUID1), + ) + coEvery { + eventRepo.getEventsFromScope(GUID1) + } returns listOf(createEventWithSessionId(GUID1, GUID1)) + + coEvery { + eventRemoteDataSource.post(any(), any(), any()) + } throws CancellationException("Upload was cancelled") + + eventUpSyncTask.upSync(operation, eventScope).toList() + + // The exception should propagate straight out of the request try/catch + coVerify(exactly = 0) { eventRepo.deleteEventScopes(any()) } + coVerify(exactly = 0) { eventRepo.addOrUpdateEvent(eventScope, any()) } + } + + @Test + fun `when upload is cancelled it should stop uploading remaining scope types`() = runTest { + setUpSyncKind(UpSynchronizationConfiguration.UpSynchronizationKind.ALL) + + coEvery { eventRepo.getClosedEventScopesCount(EventScopeType.SESSION) } returns 1 + coEvery { eventRepo.getClosedEventScopesCount(EventScopeType.UP_SYNC) } returns 1 + coEvery { eventRepo.getClosedEventScopesCount(EventScopeType.DOWN_SYNC) } returns 1 + coEvery { eventRepo.getClosedEventScopesCount(EventScopeType.SAMPLE_UP_SYNC) } returns 1 + coEvery { eventRepo.getClosedEventScopes(any(), any()) } returns listOf(createSessionScope(GUID1)) + coEvery { + eventRepo.getEventsFromScope(GUID1) + } returns listOf(createEventWithSessionId(GUID1, GUID1)) + + coEvery { + eventRemoteDataSource.post(any(), any(), any()) + } throws CancellationException("Upload was cancelled") + + eventUpSyncTask.upSync(operation, eventScope).toList() + + // Cancellation aborts the whole upSync flow immediately, so only the first + // scope type attempted its (single) request before the rest were abandoned. + coVerify(exactly = 1) { eventRemoteDataSource.post(any(), any(), any()) } + } + @Test fun `upload should not dump events when fetch fails`() = runTest { setUpSyncKind(UpSynchronizationConfiguration.UpSynchronizationKind.ALL)