fix: Ensure coroutine cancellations propograte, rethrow CancellationException (#1231)

Previous code had legacy `try ... catch` blocks that could catch all
exceptions, including `CancellationException`, thrown if the job of a
suspending function is cancelled.

Indiscriminately catching those can interfere with cancellation, so use
`currentCoroutineContext().ensureActive()` to rethrow the exception if
the job has been cancelled.
This commit is contained in:
Nik Clayton 2025-01-24 18:04:46 +01:00 committed by GitHub
parent 8ad1a37991
commit 5f67f9938c
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
15 changed files with 51 additions and 0 deletions

View File

@ -23,6 +23,8 @@ import androidx.paging.RemoteMediator
import app.pachli.core.database.model.AccountEntity
import app.pachli.core.navigation.AttachmentViewData
import app.pachli.core.network.retrofit.MastodonApi
import kotlinx.coroutines.currentCoroutineContext
import kotlinx.coroutines.ensureActive
import retrofit2.HttpException
@OptIn(ExperimentalPagingApi::class)
@ -71,6 +73,7 @@ class AccountMediaRemoteMediator(
viewModel.currentSource?.invalidate()
return MediatorResult.Success(endOfPaginationReached = statuses.isEmpty())
} catch (e: Exception) {
currentCoroutineContext().ensureActive()
return MediatorResult.Error(e)
}
}

View File

@ -10,6 +10,8 @@ import app.pachli.core.database.di.TransactionProvider
import app.pachli.core.database.model.ConversationEntity
import app.pachli.core.network.model.HttpHeaderLink
import app.pachli.core.network.retrofit.MastodonApi
import kotlinx.coroutines.currentCoroutineContext
import kotlinx.coroutines.ensureActive
import retrofit2.HttpException
@OptIn(ExperimentalPagingApi::class)
@ -73,6 +75,7 @@ class ConversationsRemoteMediator(
}
return MediatorResult.Success(endOfPaginationReached = nextKey == null)
} catch (e: Exception) {
currentCoroutineContext().ensureActive()
return MediatorResult.Error(e)
}
}

View File

@ -36,6 +36,8 @@ import app.pachli.usecase.TimelineCases
import at.connyduck.calladapter.networkresult.fold
import dagger.hilt.android.lifecycle.HiltViewModel
import javax.inject.Inject
import kotlinx.coroutines.currentCoroutineContext
import kotlinx.coroutines.ensureActive
import kotlinx.coroutines.flow.SharingStarted
import kotlinx.coroutines.flow.filter
import kotlinx.coroutines.flow.filterIsInstance
@ -181,6 +183,7 @@ class ConversationsViewModel @Inject constructor(
accountId = accountManager.activeAccount!!.id,
)
} catch (e: Exception) {
currentCoroutineContext().ensureActive()
Timber.w(e, "failed to delete conversation")
}
}
@ -197,6 +200,7 @@ class ConversationsViewModel @Inject constructor(
muted,
)
} catch (e: Exception) {
currentCoroutineContext().ensureActive()
Timber.w(e, "failed to mute conversation")
}
}

View File

@ -7,6 +7,8 @@ import androidx.paging.RemoteMediator
import app.pachli.core.network.model.HashTag
import app.pachli.core.network.model.HttpHeaderLink
import app.pachli.core.network.retrofit.MastodonApi
import kotlinx.coroutines.currentCoroutineContext
import kotlinx.coroutines.ensureActive
import retrofit2.HttpException
import retrofit2.Response
@ -25,6 +27,7 @@ class FollowedTagsRemoteMediator(
return applyResponse(response)
} catch (e: Exception) {
currentCoroutineContext().ensureActive()
MediatorResult.Error(e)
}
}

View File

@ -23,6 +23,8 @@ import com.google.android.material.divider.MaterialDividerItemDecoration
import com.google.android.material.snackbar.Snackbar
import dagger.hilt.android.AndroidEntryPoint
import javax.inject.Inject
import kotlinx.coroutines.currentCoroutineContext
import kotlinx.coroutines.ensureActive
import kotlinx.coroutines.launch
import timber.log.Timber
@ -110,6 +112,7 @@ class InstanceListFragment :
onFetchInstancesFailure(Exception(response.message()))
}
} catch (e: Exception) {
currentCoroutineContext().ensureActive()
onFetchInstancesFailure(e)
}
}

View File

@ -43,7 +43,9 @@ import javax.inject.Inject
import kotlin.collections.set
import kotlin.math.min
import kotlin.time.Duration.Companion.milliseconds
import kotlinx.coroutines.currentCoroutineContext
import kotlinx.coroutines.delay
import kotlinx.coroutines.ensureActive
import kotlinx.coroutines.flow.first
import kotlinx.coroutines.flow.take
import timber.log.Timber
@ -144,6 +146,7 @@ class NotificationFetcher @Inject constructor(
entity,
)
} catch (e: Exception) {
currentCoroutineContext().ensureActive()
Timber.e(e, "Error while fetching notifications")
}
}
@ -248,6 +251,7 @@ class NotificationFetcher @Inject constructor(
Timber.d("Fetched marker for %s: %s", account.fullName, notificationMarker)
notificationMarker
} catch (e: Exception) {
currentCoroutineContext().ensureActive()
Timber.e(e, "Failed to fetch marker")
null
}

View File

@ -62,6 +62,8 @@ import dagger.assisted.AssistedInject
import dagger.hilt.android.lifecycle.HiltViewModel
import kotlinx.coroutines.ExperimentalCoroutinesApi
import kotlinx.coroutines.channels.Channel
import kotlinx.coroutines.currentCoroutineContext
import kotlinx.coroutines.ensureActive
import kotlinx.coroutines.flow.Flow
import kotlinx.coroutines.flow.MutableSharedFlow
import kotlinx.coroutines.flow.SharingStarted
@ -462,6 +464,7 @@ class NotificationsViewModel @AssistedInject constructor(
}
Ok(NotificationActionSuccess.from(action))
} catch (e: Exception) {
currentCoroutineContext().ensureActive()
Err(UiError.make(e, action))
}
_uiResult.send(result)
@ -583,6 +586,7 @@ class NotificationsViewModel @AssistedInject constructor(
if (!isSuccessful) _uiResult.send(Err(UiError.make(HttpException(this), action)))
}
} catch (e: Exception) {
currentCoroutineContext().ensureActive()
_uiResult.send(Err(UiError.make(e, action)))
}
}

View File

@ -22,6 +22,8 @@ import app.pachli.core.network.model.Status
import app.pachli.core.network.retrofit.MastodonApi
import kotlinx.coroutines.Dispatchers
import kotlinx.coroutines.async
import kotlinx.coroutines.currentCoroutineContext
import kotlinx.coroutines.ensureActive
import kotlinx.coroutines.withContext
import timber.log.Timber
@ -69,6 +71,7 @@ class StatusesPagingSource(
nextKey = result.lastOrNull()?.id,
)
} catch (e: Exception) {
currentCoroutineContext().ensureActive()
Timber.w(e, "failed to load statuses")
return LoadResult.Error(e)
}

View File

@ -34,6 +34,8 @@ import app.pachli.core.network.model.Status
import app.pachli.core.network.retrofit.MastodonApi
import kotlinx.coroutines.async
import kotlinx.coroutines.coroutineScope
import kotlinx.coroutines.currentCoroutineContext
import kotlinx.coroutines.ensureActive
import okhttp3.Headers
import retrofit2.HttpException
import retrofit2.Response
@ -159,6 +161,7 @@ class CachedTimelineRemoteMediator(
return MediatorResult.Success(endOfPaginationReached = false)
} catch (e: Exception) {
currentCoroutineContext().ensureActive()
Timber.e(e, "Error loading, LoadType = %s", loadType)
MediatorResult.Error(e)
}

View File

@ -28,6 +28,8 @@ import app.pachli.core.model.Timeline
import app.pachli.core.network.model.Status
import app.pachli.core.network.retrofit.MastodonApi
import java.io.IOException
import kotlinx.coroutines.currentCoroutineContext
import kotlinx.coroutines.ensureActive
import retrofit2.HttpException
import retrofit2.Response
import timber.log.Timber
@ -97,6 +99,7 @@ class NetworkTimelineRemoteMediator(
return MediatorResult.Success(endOfPaginationReached = endOfPaginationReached)
} catch (e: Exception) {
currentCoroutineContext().ensureActive()
Timber.e(e, "Error loading, LoadType = %s", loadType)
MediatorResult.Error(e)
}

View File

@ -62,6 +62,8 @@ import com.github.michaelbull.result.Err
import com.github.michaelbull.result.Ok
import com.github.michaelbull.result.Result
import kotlinx.coroutines.channels.Channel
import kotlinx.coroutines.currentCoroutineContext
import kotlinx.coroutines.ensureActive
import kotlinx.coroutines.flow.Flow
import kotlinx.coroutines.flow.MutableSharedFlow
import kotlinx.coroutines.flow.SharingStarted
@ -375,6 +377,7 @@ abstract class TimelineViewModel<T : Any>(
// Result<_, _> instead of NetworkResult.
_uiResult.send(Ok(StatusActionSuccess.from(action)))
} catch (e: Exception) {
currentCoroutineContext().ensureActive()
_uiResult.send(Err(UiError.make(e, action)))
}
}

View File

@ -53,6 +53,8 @@ import javax.inject.Inject
import kotlinx.coroutines.Job
import kotlinx.coroutines.async
import kotlinx.coroutines.channels.BufferOverflow
import kotlinx.coroutines.currentCoroutineContext
import kotlinx.coroutines.ensureActive
import kotlinx.coroutines.flow.Flow
import kotlinx.coroutines.flow.MutableSharedFlow
import kotlinx.coroutines.flow.MutableStateFlow
@ -277,6 +279,7 @@ class ViewThreadViewModel @Inject constructor(
try {
timelineCases.reblog(status.actionableId, reblog).getOrThrow()
} catch (t: Exception) {
currentCoroutineContext().ensureActive()
ifExpected(t) {
Timber.d(t, "Failed to reblog status: %s", status.actionableId)
}
@ -287,6 +290,7 @@ class ViewThreadViewModel @Inject constructor(
try {
timelineCases.favourite(status.actionableId, favorite).getOrThrow()
} catch (t: Exception) {
currentCoroutineContext().ensureActive()
ifExpected(t) {
Timber.d(t, "Failed to favourite status: %s ", status.actionableId)
}
@ -297,6 +301,7 @@ class ViewThreadViewModel @Inject constructor(
try {
timelineCases.bookmark(status.actionableId, bookmark).getOrThrow()
} catch (t: Exception) {
currentCoroutineContext().ensureActive()
ifExpected(t) {
Timber.d(t, "Failed to bookmark status: %s", status.actionableId)
}
@ -312,6 +317,7 @@ class ViewThreadViewModel @Inject constructor(
try {
timelineCases.voteInPoll(status.actionableId, poll.id, choices).getOrThrow()
} catch (t: Exception) {
currentCoroutineContext().ensureActive()
ifExpected(t) {
Timber.d(t, "Failed to vote in poll: %s", status.actionableId)
}

View File

@ -50,7 +50,9 @@ import kotlinx.coroutines.CoroutineScope
import kotlinx.coroutines.Dispatchers
import kotlinx.coroutines.Job
import kotlinx.coroutines.SupervisorJob
import kotlinx.coroutines.currentCoroutineContext
import kotlinx.coroutines.delay
import kotlinx.coroutines.ensureActive
import kotlinx.coroutines.launch
import kotlinx.coroutines.runBlocking
import kotlinx.parcelize.Parcelize
@ -179,6 +181,7 @@ class SendStatusService : Service() {
mediaCheckRetries++
}
} catch (e: Exception) {
currentCoroutineContext().ensureActive()
Timber.w(e, "failed getting media status")
retrySending(statusId)
return@launch

View File

@ -31,6 +31,8 @@ import dagger.assisted.Assisted
import dagger.assisted.AssistedInject
import java.time.Instant
import kotlin.time.Duration.Companion.hours
import kotlinx.coroutines.currentCoroutineContext
import kotlinx.coroutines.ensureActive
import timber.log.Timber
/** Prune the database cache of old statuses. */
@ -49,6 +51,7 @@ class PruneLogEntryEntityWorker @AssistedInject constructor(
logEntryDao.prune(oldest)
Result.success()
} catch (e: Exception) {
currentCoroutineContext().ensureActive()
Timber.e(e, "error in PruneLogEntryEntityWorker.doWork")
Result.failure()
}

View File

@ -44,6 +44,8 @@ import app.pachli.core.network.model.TimelineAccount
import app.pachli.core.network.retrofit.MastodonApi
import kotlinx.coroutines.async
import kotlinx.coroutines.coroutineScope
import kotlinx.coroutines.currentCoroutineContext
import kotlinx.coroutines.ensureActive
import okhttp3.Headers
import retrofit2.HttpException
import retrofit2.Response
@ -156,6 +158,7 @@ class NotificationsRemoteMediator(
MediatorResult.Success(endOfPaginationReached = false)
} catch (e: Exception) {
currentCoroutineContext().ensureActive()
Timber.e(e, "error loading, loadtype = %s", loadType)
MediatorResult.Error(e)
}