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.
This commit is contained in:
Nik Clayton 2023-06-11 14:00:05 +02:00 committed by GitHub
parent 5e8a63a046
commit 8fec41c2ae
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 46 additions and 36 deletions

View File

@ -267,7 +267,7 @@ class NotificationsFragment :
Log.d(TAG, error.toString()) Log.d(TAG, error.toString())
val message = getString( val message = getString(
error.message, error.message,
error.exception.localizedMessage error.throwable.localizedMessage
?: getString(R.string.ui_error_unknown) ?: getString(R.string.ui_error_unknown)
) )
val snackbar = Snackbar.make( val snackbar = Snackbar.make(

View File

@ -25,6 +25,7 @@ import androidx.lifecycle.viewModelScope
import androidx.paging.PagingData import androidx.paging.PagingData
import androidx.paging.cachedIn import androidx.paging.cachedIn
import androidx.paging.map import androidx.paging.map
import at.connyduck.calladapter.networkresult.getOrThrow
import com.keylesspalace.tusky.R import com.keylesspalace.tusky.R
import com.keylesspalace.tusky.appstore.BlockEvent import com.keylesspalace.tusky.appstore.BlockEvent
import com.keylesspalace.tusky.appstore.EventHub import com.keylesspalace.tusky.appstore.EventHub
@ -46,6 +47,7 @@ import com.keylesspalace.tusky.viewdata.NotificationViewData
import com.keylesspalace.tusky.viewdata.StatusViewData import com.keylesspalace.tusky.viewdata.StatusViewData
import kotlinx.coroutines.ExperimentalCoroutinesApi import kotlinx.coroutines.ExperimentalCoroutinesApi
import kotlinx.coroutines.FlowPreview import kotlinx.coroutines.FlowPreview
import kotlinx.coroutines.channels.Channel
import kotlinx.coroutines.flow.Flow import kotlinx.coroutines.flow.Flow
import kotlinx.coroutines.flow.MutableSharedFlow import kotlinx.coroutines.flow.MutableSharedFlow
import kotlinx.coroutines.flow.MutableStateFlow import kotlinx.coroutines.flow.MutableStateFlow
@ -60,6 +62,7 @@ import kotlinx.coroutines.flow.flatMapLatest
import kotlinx.coroutines.flow.map import kotlinx.coroutines.flow.map
import kotlinx.coroutines.flow.onEach import kotlinx.coroutines.flow.onEach
import kotlinx.coroutines.flow.onStart import kotlinx.coroutines.flow.onStart
import kotlinx.coroutines.flow.receiveAsFlow
import kotlinx.coroutines.flow.stateIn import kotlinx.coroutines.flow.stateIn
import kotlinx.coroutines.launch import kotlinx.coroutines.launch
import kotlinx.coroutines.rx3.await 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 */ /** Errors from fallible view model actions that the UI will need to show */
sealed class UiError( sealed class UiError(
/** The exception associated with the error */ /** The exception associated with the error */
open val exception: Exception, open val throwable: Throwable,
/** String resource with an error message to show the user */ /** String resource with an error message to show the user */
@StringRes val message: Int, @StringRes val message: Int,
@ -228,50 +231,50 @@ sealed class UiError(
/** The action that failed. Can be resent to retry the action */ /** The action that failed. Can be resent to retry the action */
open val action: UiAction? = null open val action: UiAction? = null
) { ) {
data class ClearNotifications(override val exception: Exception) : UiError( data class ClearNotifications(override val throwable: Throwable) : UiError(
exception, throwable,
R.string.ui_error_clear_notifications R.string.ui_error_clear_notifications
) )
data class Bookmark( data class Bookmark(
override val exception: Exception, override val throwable: Throwable,
override val action: StatusAction.Bookmark override val action: StatusAction.Bookmark
) : UiError(exception, R.string.ui_error_bookmark, action) ) : UiError(throwable, R.string.ui_error_bookmark, action)
data class Favourite( data class Favourite(
override val exception: Exception, override val throwable: Throwable,
override val action: StatusAction.Favourite override val action: StatusAction.Favourite
) : UiError(exception, R.string.ui_error_favourite, action) ) : UiError(throwable, R.string.ui_error_favourite, action)
data class Reblog( data class Reblog(
override val exception: Exception, override val throwable: Throwable,
override val action: StatusAction.Reblog override val action: StatusAction.Reblog
) : UiError(exception, R.string.ui_error_reblog, action) ) : UiError(throwable, R.string.ui_error_reblog, action)
data class VoteInPoll( data class VoteInPoll(
override val exception: Exception, override val throwable: Throwable,
override val action: StatusAction.VoteInPoll override val action: StatusAction.VoteInPoll
) : UiError(exception, R.string.ui_error_vote, action) ) : UiError(throwable, R.string.ui_error_vote, action)
data class AcceptFollowRequest( data class AcceptFollowRequest(
override val exception: Exception, override val throwable: Throwable,
override val action: NotificationAction.AcceptFollowRequest 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( data class RejectFollowRequest(
override val exception: Exception, override val throwable: Throwable,
override val action: NotificationAction.RejectFollowRequest 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 { companion object {
fun make(exception: Exception, action: FallibleUiAction) = when (action) { fun make(throwable: Throwable, action: FallibleUiAction) = when (action) {
is StatusAction.Bookmark -> Bookmark(exception, action) is StatusAction.Bookmark -> Bookmark(throwable, action)
is StatusAction.Favourite -> Favourite(exception, action) is StatusAction.Favourite -> Favourite(throwable, action)
is StatusAction.Reblog -> Reblog(exception, action) is StatusAction.Reblog -> Reblog(throwable, action)
is StatusAction.VoteInPoll -> VoteInPoll(exception, action) is StatusAction.VoteInPoll -> VoteInPoll(throwable, action)
is NotificationAction.AcceptFollowRequest -> AcceptFollowRequest(exception, action) is NotificationAction.AcceptFollowRequest -> AcceptFollowRequest(throwable, action)
is NotificationAction.RejectFollowRequest -> RejectFollowRequest(exception, action) is NotificationAction.RejectFollowRequest -> RejectFollowRequest(throwable, action)
FallibleUiAction.ClearNotifications -> ClearNotifications(exception) FallibleUiAction.ClearNotifications -> ClearNotifications(throwable)
} }
} }
} }
@ -298,14 +301,21 @@ class NotificationsViewModel @Inject constructor(
private val uiAction = MutableSharedFlow<UiAction>() private val uiAction = MutableSharedFlow<UiAction>()
/** Flow of successful action results */ /** Flow of successful action results */
// Note: These are a SharedFlow instead of a StateFlow because success or error state does not // Note: This is a SharedFlow instead of a StateFlow because success state does not need to be
// need to be retained. A message is shown once to a user and then dismissed. Re-collecting the // retained. A message is shown once to a user and then dismissed. Re-collecting the flow
// flow (e.g., after a device orientation change) should not re-show the most recent success or // (e.g., after a device orientation change) should not re-show the most recent success
// error message, as it will be confusing to the user. // message, as it will be confusing to the user.
val uiSuccess = MutableSharedFlow<UiSuccess>() val uiSuccess = MutableSharedFlow<UiSuccess>()
/** Flow of transient errors for the UI to present */ /** Channel for error results */
val uiError = MutableSharedFlow<UiError>() // 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<UiError>()
/** Expose UI errors as a flow */
val uiError = _uiErrorChannel.receiveAsFlow()
/** Accept UI actions in to actionStateFlow */ /** Accept UI actions in to actionStateFlow */
val accept: (UiAction) -> Unit = { action -> val accept: (UiAction) -> Unit = { action ->
@ -380,11 +390,11 @@ class NotificationsViewModel @Inject constructor(
if (this.isSuccessful) { if (this.isSuccessful) {
repository.invalidate() repository.invalidate()
} else { } else {
uiError.emit(UiError.make(HttpException(this), it)) _uiErrorChannel.send(UiError.make(HttpException(this), it))
} }
} }
} catch (e: Exception) { } 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)) uiSuccess.emit(NotificationActionSuccess.from(action))
} catch (e: Exception) { } 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.poll.id,
action.choices action.choices
) )
} }.getOrThrow()
uiSuccess.emit(StatusActionSuccess.from(action)) uiSuccess.emit(StatusActionSuccess.from(action))
} catch (e: Exception) { } catch (t: Throwable) {
ifExpected(e) { uiError.emit(UiError.make(e, action)) } _uiErrorChannel.send(UiError.make(t, action))
} }
} }
} }