Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

fix: Surface all exceptions to the user instead of crashing #565

Merged
merged 1 commit into from
Mar 24, 2024
Merged

fix: Surface all exceptions to the user instead of crashing #565

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
5 changes: 1 addition & 4 deletions app/src/main/java/app/pachli/components/account/media/AccountMediaRemoteMediator.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@ import androidx.paging.ExperimentalPagingApi
import androidx.paging.LoadType
import androidx.paging.PagingState
import androidx.paging.RemoteMediator
import app.pachli.components.timeline.util.ifExpected
import app.pachli.core.database.model.AccountEntity
import app.pachli.core.navigation.AttachmentViewData
import app.pachli.core.network.retrofit.MastodonApi
Expand Down Expand Up @@ -72,9 +71,7 @@ class AccountMediaRemoteMediator(
viewModel.currentSource?.invalidate()
return MediatorResult.Success(endOfPaginationReached = statuses.isEmpty())
} catch (e: Exception) {
return ifExpected(e) {
MediatorResult.Error(e)
}
return MediatorResult.Error(e)
}
}
}
5 changes: 2 additions & 3 deletions app/src/main/java/app/pachli/components/notifications/NotificationsViewModel.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,6 @@ import app.pachli.appstore.MuteConversationEvent
import app.pachli.appstore.MuteEvent
import app.pachli.components.timeline.FilterKind
import app.pachli.components.timeline.FiltersRepository
import app.pachli.components.timeline.util.ifExpected
import app.pachli.core.accounts.AccountManager
import app.pachli.core.network.model.Filter
import app.pachli.core.network.model.FilterContext
Expand Down Expand Up @@ -409,7 +408,7 @@ class NotificationsViewModel @Inject constructor(
}
}
} catch (e: Exception) {
ifExpected(e) { _uiErrorChannel.send(UiError.make(e, it)) }
_uiErrorChannel.send(UiError.make(e, it))
}
}
}
Expand All @@ -428,7 +427,7 @@ class NotificationsViewModel @Inject constructor(
}
uiSuccess.emit(NotificationActionSuccess.from(action))
} catch (e: Exception) {
ifExpected(e) { _uiErrorChannel.send(UiError.make(e, action)) }
_uiErrorChannel.send(UiError.make(e, action))
}
}
}
Expand Down
23 changes: 18 additions & 5 deletions app/src/main/java/app/pachli/components/timeline/TimelineFragment.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ import androidx.recyclerview.widget.SimpleItemAnimator
import androidx.swiperefreshlayout.widget.SwipeRefreshLayout.OnRefreshListener
import app.pachli.R
import app.pachli.adapter.StatusBaseViewHolder
import app.pachli.components.timeline.util.isExpected
import app.pachli.components.timeline.viewmodel.CachedTimelineViewModel
import app.pachli.components.timeline.viewmodel.InfallibleUiAction
import app.pachli.components.timeline.viewmodel.NetworkTimelineViewModel
Expand Down Expand Up @@ -431,20 +432,32 @@ class TimelineFragment :
// Show errors as a snackbar if there is existing content to show
// (either cached, or in the adapter), or as a full screen error
// otherwise.
//
// Expected errors can be retried, unexpected ones cannot
if (adapter.itemCount > 0) {
snackbar = Snackbar.make(
(activity as ActionButtonActivity).actionButton
?: binding.root,
message,
Snackbar.LENGTH_INDEFINITE,
)
.setAction(app.pachli.core.ui.R.string.action_retry) { adapter.retry() }
).apply {
if (error.isExpected()) {
setAction(app.pachli.core.ui.R.string.action_retry) { adapter.retry() }
}
}

snackbar!!.show()
} else {
binding.statusView.setup(error) {
snackbar?.dismiss()
adapter.retry()
val callback: ((v: View) -> Unit)? = if (error.isExpected()) {
{
snackbar?.dismiss()
adapter.retry()
}
} else {
null
}

binding.statusView.setup(error, callback)
binding.statusView.show()
binding.recyclerView.hide()
}
Expand Down
8 changes: 4 additions & 4 deletions app/src/main/java/app/pachli/components/timeline/viewmodel/CachedTimelineRemoteMediator.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,6 @@ import app.pachli.core.network.model.Links
import app.pachli.core.network.model.Status
import app.pachli.core.network.retrofit.MastodonApi
import com.squareup.moshi.Moshi
import java.io.IOException
import kotlinx.coroutines.async
import kotlinx.coroutines.coroutineScope
import okhttp3.Headers
Expand Down Expand Up @@ -78,6 +77,7 @@ class CachedTimelineRemoteMediator(
Timber.d("Loading from item: %s", statusId)
getInitialPage(statusId, state.config.pageSize)
}

LoadType.APPEND -> {
val rke = remoteKeyDao.remoteKeyForKind(
activeAccount.id,
Expand All @@ -87,6 +87,7 @@ class CachedTimelineRemoteMediator(
Timber.d("Loading from remoteKey: %s", rke)
api.homeTimeline(maxId = rke.key, limit = state.config.pageSize)
}

LoadType.PREPEND -> {
val rke = remoteKeyDao.remoteKeyForKind(
activeAccount.id,
Expand Down Expand Up @@ -168,9 +169,8 @@ class CachedTimelineRemoteMediator(
}

return MediatorResult.Success(endOfPaginationReached = false)
} catch (e: IOException) {
MediatorResult.Error(e)
} catch (e: HttpException) {
} catch (e: Exception) {
Timber.e(e, "Error loading, LoadType = %s", loadType)
MediatorResult.Error(e)
}
}
Expand Down
5 changes: 2 additions & 3 deletions app/src/main/java/app/pachli/components/timeline/viewmodel/NetworkTimelineRemoteMediator.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -126,9 +126,8 @@ class NetworkTimelineRemoteMediator(
}

return MediatorResult.Success(endOfPaginationReached = endOfPaginationReached)
} catch (e: IOException) {
MediatorResult.Error(e)
} catch (e: HttpException) {
} catch (e: Exception) {
Timber.e(e, "Error loading, LoadType = %s", loadType)
MediatorResult.Error(e)
}
}
Expand Down
3 changes: 1 addition & 2 deletions app/src/main/java/app/pachli/components/timeline/viewmodel/TimelineViewModel.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,6 @@ import app.pachli.appstore.StatusEditedEvent
import app.pachli.appstore.UnfollowEvent
import app.pachli.components.timeline.FilterKind
import app.pachli.components.timeline.FiltersRepository
import app.pachli.components.timeline.util.ifExpected
import app.pachli.core.accounts.AccountManager
import app.pachli.core.network.model.Filter
import app.pachli.core.network.model.FilterContext
Expand Down Expand Up @@ -361,7 +360,7 @@ abstract class TimelineViewModel(
}.getOrThrow()
uiSuccess.emit(StatusActionSuccess.from(action))
} catch (e: Exception) {
ifExpected(e) { _uiErrorChannel.send(UiError.make(e, action)) }
_uiErrorChannel.send(UiError.make(e, action))
}
}
}
Expand Down
3 changes: 3 additions & 0 deletions core/ui/build.gradle.kts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,9 @@ dependencies {
// Uses HttpException from Retrofit
implementation(projects.core.network)

// Uses JsonDataException from Moshi
implementation(libs.moshi)

// Some views inherit from AndroidX views
implementation(libs.bundles.androidx)

Expand Down
2 changes: 2 additions & 0 deletions core/ui/src/main/kotlin/app/pachli/core/ui/extensions/ThrowableExtensions.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ package app.pachli.core.ui.extensions
import android.content.Context
import app.pachli.core.network.extensions.getServerErrorMessage
import app.pachli.core.ui.R
import com.squareup.moshi.JsonDataException
import java.io.IOException
import retrofit2.HttpException

Expand All @@ -40,5 +41,6 @@ fun Throwable.getDrawableRes(): Int = when (this) {
fun Throwable.getErrorString(context: Context): String = getServerErrorMessage() ?: when (this) {
is IOException -> context.getString(R.string.error_network_fmt, this.message)
is HttpException -> if (this.code() == 404) context.getString(R.string.error_404_not_found_fmt, this.message) else context.getString(R.string.error_generic_fmt, this.message)
is JsonDataException -> context.getString(R.string.error_json_data_fmt, this.message)
else -> context.getString(R.string.error_generic_fmt, this.message)
}
1 change: 1 addition & 0 deletions core/ui/src/main/res/values/strings.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
<string name="error_network_fmt">A network error occurred: %s</string>
<string name="error_generic_fmt">An error occurred: %s</string>
<string name="error_404_not_found_fmt">Your server does not support this feature: %1$s</string>
<string name="error_json_data_fmt">Your server returned an invalid response: %1$s</string>
<string name="error_generic">An error occurred.</string>
<string name="message_empty">Nothing here.</string>
<string name="action_retry">Retry</string>
Expand Down