From a56a7adb46d831b53df9c695b0a3e769623b54fd Mon Sep 17 00:00:00 2001 From: Adam Brown Date: Mon, 15 Aug 2022 16:36:53 +0100 Subject: [PATCH 1/3] including the worker failure message in the logs and including throwable class name --- .../MultipleEventSendingDispatcherWorker.kt | 4 ++-- .../worker/SessionSafeCoroutineWorker.kt | 18 ++++++++++-------- 2 files changed, 12 insertions(+), 10 deletions(-) diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/room/send/MultipleEventSendingDispatcherWorker.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/room/send/MultipleEventSendingDispatcherWorker.kt index 2afca6e554..801ff0ec79 100644 --- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/room/send/MultipleEventSendingDispatcherWorker.kt +++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/room/send/MultipleEventSendingDispatcherWorker.kt @@ -53,7 +53,7 @@ internal class MultipleEventSendingDispatcherWorker(context: Context, params: Wo @Inject lateinit var timelineSendEventWorkCommon: TimelineSendEventWorkCommon @Inject lateinit var localEchoRepository: LocalEchoRepository - override fun doOnError(params: Params): Result { + override fun doOnError(params: Params, failureMessage: String): Result { params.localEchoIds.forEach { localEchoIds -> localEchoRepository.updateSendState( eventId = localEchoIds.eventId, @@ -63,7 +63,7 @@ internal class MultipleEventSendingDispatcherWorker(context: Context, params: Wo ) } - return super.doOnError(params) + return super.doOnError(params, failureMessage) } override fun injectWith(injector: SessionComponent) { diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/worker/SessionSafeCoroutineWorker.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/worker/SessionSafeCoroutineWorker.kt index 030f51428b..b98b61c9f0 100644 --- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/worker/SessionSafeCoroutineWorker.kt +++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/worker/SessionSafeCoroutineWorker.kt @@ -55,14 +55,16 @@ internal abstract class SessionSafeCoroutineWorker( // Make sure to inject before handling error as you may need some dependencies to process them. injectWith(sessionComponent) - if (params.lastFailureMessage != null) { - // Forward error to the next workers - doOnError(params) - } else { - doSafeWork(params) + + when (val lastFailureMessage = params.lastFailureMessage) { + null -> doSafeWork(params) + else -> { + // Forward error to the next workers + doOnError(params, lastFailureMessage) + } } } catch (throwable: Throwable) { - buildErrorResult(params, throwable.localizedMessage ?: "error") + buildErrorResult(params, "${throwable::class.java.name}: ${throwable.localizedMessage ?: "N/A error message"}") } } @@ -89,10 +91,10 @@ internal abstract class SessionSafeCoroutineWorker( * This is called when the input parameters are correct, but contain an error from the previous worker. */ @CallSuper - open fun doOnError(params: PARAM): Result { + open fun doOnError(params: PARAM, failureMessage: String): Result { // Forward the error return Result.success(inputData) - .also { Timber.e("Work cancelled due to input error from parent") } + .also { Timber.e("Work cancelled due to input error from parent: $failureMessage") } } companion object { From 1fd1a4e82436f502b33bda2d3be8f642898689ce Mon Sep 17 00:00:00 2001 From: Adam Brown Date: Wed, 17 Aug 2022 15:22:28 +0100 Subject: [PATCH 2/3] fixing SyncWorker becoming stuck in failure state on uncaught exception - the sync worker makes use of the CoroutineWorker which does not stop when the work completes, this means we often append to the existing worker. When appending by default the previous worker result payload is merged with (or in our case overwrites) the input data instead, meaning any failure state is set and kept until the worker stops, which in turns causes the sync worker to never sync - the fix is to make use of an input merge that always favour the request input data instead of the previous worker results --- .../matrix/android/sdk/internal/session/sync/job/SyncWorker.kt | 2 ++ 1 file changed, 2 insertions(+) diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/sync/job/SyncWorker.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/sync/job/SyncWorker.kt index 0cc7944d58..a04bc74628 100644 --- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/sync/job/SyncWorker.kt +++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/sync/job/SyncWorker.kt @@ -30,6 +30,7 @@ import org.matrix.android.sdk.internal.session.sync.SyncTask import org.matrix.android.sdk.internal.worker.SessionSafeCoroutineWorker import org.matrix.android.sdk.internal.worker.SessionWorkerParams import org.matrix.android.sdk.internal.worker.WorkerParamsFactory +import org.matrix.android.sdk.internal.worker.startChain import timber.log.Timber import java.util.concurrent.TimeUnit import javax.inject.Inject @@ -136,6 +137,7 @@ internal class SyncWorker(context: Context, workerParameters: WorkerParameters, .setConstraints(WorkManagerProvider.workConstraints) .setBackoffCriteria(BackoffPolicy.LINEAR, WorkManagerProvider.BACKOFF_DELAY_MILLIS, TimeUnit.MILLISECONDS) .setInputData(data) + .startChain(true) .build() workManagerProvider.workManager .enqueueUniqueWork(BG_SYNC_WORK_NAME, ExistingWorkPolicy.APPEND_OR_REPLACE, workRequest) From a3a76d1c351c24c7fa2663d879236532e285d996 Mon Sep 17 00:00:00 2001 From: Adam Brown Date: Wed, 17 Aug 2022 15:24:05 +0100 Subject: [PATCH 3/3] adding changelog entry --- changelog.d/6836.bugfix | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelog.d/6836.bugfix diff --git a/changelog.d/6836.bugfix b/changelog.d/6836.bugfix new file mode 100644 index 0000000000..6fbcc35001 --- /dev/null +++ b/changelog.d/6836.bugfix @@ -0,0 +1 @@ +Fixes uncaught exceptions in the SyncWorker to cause the worker to become stuck in the failure state