From 8fec41c2ae8aa2324e00206097f52aeba4d7bd41 Mon Sep 17 00:00:00 2001 From: Nik Clayton Date: Sun, 11 Jun 2023 14:00:05 +0200 Subject: [PATCH] Send UI errors to a channel instead of a shared flow (#3652) In the previous code any errors that occured *before* a subscriber was listening to `uiError` would be dropped, so the user would be unware of them. By implementing as a channel these errors will be shown to the user, with an opportunity to retry the operation or report the error. --- .../notifications/NotificationsFragment.kt | 2 +- .../notifications/NotificationsViewModel.kt | 80 +++++++++++-------- 2 files changed, 46 insertions(+), 36 deletions(-) diff --git a/app/src/main/java/com/keylesspalace/tusky/components/notifications/NotificationsFragment.kt b/app/src/main/java/com/keylesspalace/tusky/components/notifications/NotificationsFragment.kt index 352e87ca..19d6edf9 100644 --- a/app/src/main/java/com/keylesspalace/tusky/components/notifications/NotificationsFragment.kt +++ b/app/src/main/java/com/keylesspalace/tusky/components/notifications/NotificationsFragment.kt @@ -267,7 +267,7 @@ class NotificationsFragment : Log.d(TAG, error.toString()) val message = getString( error.message, - error.exception.localizedMessage + error.throwable.localizedMessage ?: getString(R.string.ui_error_unknown) ) val snackbar = Snackbar.make( diff --git a/app/src/main/java/com/keylesspalace/tusky/components/notifications/NotificationsViewModel.kt b/app/src/main/java/com/keylesspalace/tusky/components/notifications/NotificationsViewModel.kt index ea0f2e8e..5eb54e10 100644 --- a/app/src/main/java/com/keylesspalace/tusky/components/notifications/NotificationsViewModel.kt +++ b/app/src/main/java/com/keylesspalace/tusky/components/notifications/NotificationsViewModel.kt @@ -25,6 +25,7 @@ import androidx.lifecycle.viewModelScope import androidx.paging.PagingData import androidx.paging.cachedIn import androidx.paging.map +import at.connyduck.calladapter.networkresult.getOrThrow import com.keylesspalace.tusky.R import com.keylesspalace.tusky.appstore.BlockEvent import com.keylesspalace.tusky.appstore.EventHub @@ -46,6 +47,7 @@ import com.keylesspalace.tusky.viewdata.NotificationViewData import com.keylesspalace.tusky.viewdata.StatusViewData import kotlinx.coroutines.ExperimentalCoroutinesApi import kotlinx.coroutines.FlowPreview +import kotlinx.coroutines.channels.Channel import kotlinx.coroutines.flow.Flow import kotlinx.coroutines.flow.MutableSharedFlow import kotlinx.coroutines.flow.MutableStateFlow @@ -60,6 +62,7 @@ import kotlinx.coroutines.flow.flatMapLatest import kotlinx.coroutines.flow.map import kotlinx.coroutines.flow.onEach import kotlinx.coroutines.flow.onStart +import kotlinx.coroutines.flow.receiveAsFlow import kotlinx.coroutines.flow.stateIn import kotlinx.coroutines.launch import kotlinx.coroutines.rx3.await @@ -220,7 +223,7 @@ sealed class StatusActionSuccess(open val action: StatusAction) : UiSuccess() { /** Errors from fallible view model actions that the UI will need to show */ sealed class UiError( /** The exception associated with the error */ - open val exception: Exception, + open val throwable: Throwable, /** String resource with an error message to show the user */ @StringRes val message: Int, @@ -228,50 +231,50 @@ sealed class UiError( /** The action that failed. Can be resent to retry the action */ open val action: UiAction? = null ) { - data class ClearNotifications(override val exception: Exception) : UiError( - exception, + data class ClearNotifications(override val throwable: Throwable) : UiError( + throwable, R.string.ui_error_clear_notifications ) data class Bookmark( - override val exception: Exception, + override val throwable: Throwable, override val action: StatusAction.Bookmark - ) : UiError(exception, R.string.ui_error_bookmark, action) + ) : UiError(throwable, R.string.ui_error_bookmark, action) data class Favourite( - override val exception: Exception, + override val throwable: Throwable, override val action: StatusAction.Favourite - ) : UiError(exception, R.string.ui_error_favourite, action) + ) : UiError(throwable, R.string.ui_error_favourite, action) data class Reblog( - override val exception: Exception, + override val throwable: Throwable, override val action: StatusAction.Reblog - ) : UiError(exception, R.string.ui_error_reblog, action) + ) : UiError(throwable, R.string.ui_error_reblog, action) data class VoteInPoll( - override val exception: Exception, + override val throwable: Throwable, override val action: StatusAction.VoteInPoll - ) : UiError(exception, R.string.ui_error_vote, action) + ) : UiError(throwable, R.string.ui_error_vote, action) data class AcceptFollowRequest( - override val exception: Exception, + override val throwable: Throwable, override val action: NotificationAction.AcceptFollowRequest - ) : UiError(exception, R.string.ui_error_accept_follow_request, action) + ) : UiError(throwable, R.string.ui_error_accept_follow_request, action) data class RejectFollowRequest( - override val exception: Exception, + override val throwable: Throwable, override val action: NotificationAction.RejectFollowRequest - ) : UiError(exception, R.string.ui_error_reject_follow_request, action) + ) : UiError(throwable, R.string.ui_error_reject_follow_request, action) companion object { - fun make(exception: Exception, action: FallibleUiAction) = when (action) { - is StatusAction.Bookmark -> Bookmark(exception, action) - is StatusAction.Favourite -> Favourite(exception, action) - is StatusAction.Reblog -> Reblog(exception, action) - is StatusAction.VoteInPoll -> VoteInPoll(exception, action) - is NotificationAction.AcceptFollowRequest -> AcceptFollowRequest(exception, action) - is NotificationAction.RejectFollowRequest -> RejectFollowRequest(exception, action) - FallibleUiAction.ClearNotifications -> ClearNotifications(exception) + fun make(throwable: Throwable, action: FallibleUiAction) = when (action) { + is StatusAction.Bookmark -> Bookmark(throwable, action) + is StatusAction.Favourite -> Favourite(throwable, action) + is StatusAction.Reblog -> Reblog(throwable, action) + is StatusAction.VoteInPoll -> VoteInPoll(throwable, action) + is NotificationAction.AcceptFollowRequest -> AcceptFollowRequest(throwable, action) + is NotificationAction.RejectFollowRequest -> RejectFollowRequest(throwable, action) + FallibleUiAction.ClearNotifications -> ClearNotifications(throwable) } } } @@ -298,14 +301,21 @@ class NotificationsViewModel @Inject constructor( private val uiAction = MutableSharedFlow() /** Flow of successful action results */ - // Note: These are a SharedFlow instead of a StateFlow because success or error state does not - // need to be retained. A message is shown once to a user and then dismissed. Re-collecting the - // flow (e.g., after a device orientation change) should not re-show the most recent success or - // error message, as it will be confusing to the user. + // Note: This is a SharedFlow instead of a StateFlow because success state does not need to be + // retained. A message is shown once to a user and then dismissed. Re-collecting the flow + // (e.g., after a device orientation change) should not re-show the most recent success + // message, as it will be confusing to the user. val uiSuccess = MutableSharedFlow() - /** Flow of transient errors for the UI to present */ - val uiError = MutableSharedFlow() + /** Channel for error results */ + // Errors are sent to a channel to ensure that any errors that occur *before* there are any + // subscribers are retained. If this was a SharedFlow any errors would be dropped, and if it + // was a StateFlow any errors would be retained, and there would need to be an explicit + // mechanism to dismiss them. + private val _uiErrorChannel = Channel() + + /** Expose UI errors as a flow */ + val uiError = _uiErrorChannel.receiveAsFlow() /** Accept UI actions in to actionStateFlow */ val accept: (UiAction) -> Unit = { action -> @@ -380,11 +390,11 @@ class NotificationsViewModel @Inject constructor( if (this.isSuccessful) { repository.invalidate() } else { - uiError.emit(UiError.make(HttpException(this), it)) + _uiErrorChannel.send(UiError.make(HttpException(this), it)) } } } catch (e: Exception) { - ifExpected(e) { uiError.emit(UiError.make(e, it)) } + ifExpected(e) { _uiErrorChannel.send(UiError.make(e, it)) } } } } @@ -403,7 +413,7 @@ class NotificationsViewModel @Inject constructor( } uiSuccess.emit(NotificationActionSuccess.from(action)) } catch (e: Exception) { - ifExpected(e) { uiError.emit(UiError.make(e, action)) } + ifExpected(e) { _uiErrorChannel.send(UiError.make(e, action)) } } } } @@ -436,10 +446,10 @@ class NotificationsViewModel @Inject constructor( action.poll.id, action.choices ) - } + }.getOrThrow() uiSuccess.emit(StatusActionSuccess.from(action)) - } catch (e: Exception) { - ifExpected(e) { uiError.emit(UiError.make(e, action)) } + } catch (t: Throwable) { + _uiErrorChannel.send(UiError.make(t, action)) } } }