Show better errors when loading notifications fails (#3448)

* Show better errors with notification loading fails

The errors are returned as a JSON object, parse it, and show the error
message it contains.

Handle the cases where there might be no error message, or the JSON may be
malformed.

Add tests.

Fixes #3445

* Lint
This commit is contained in:
Nik Clayton 2023-03-18 10:25:41 +01:00 committed by GitHub
parent 84188ed10f
commit 81f725667e
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
4 changed files with 192 additions and 3 deletions

View file

@ -20,6 +20,7 @@ package com.keylesspalace.tusky.components.notifications
import android.util.Log import android.util.Log
import androidx.paging.PagingSource import androidx.paging.PagingSource
import androidx.paging.PagingState import androidx.paging.PagingState
import com.google.gson.Gson
import com.keylesspalace.tusky.entity.Notification import com.keylesspalace.tusky.entity.Notification
import com.keylesspalace.tusky.network.MastodonApi import com.keylesspalace.tusky.network.MastodonApi
import com.keylesspalace.tusky.util.HttpHeaderLink import com.keylesspalace.tusky.util.HttpHeaderLink
@ -35,6 +36,7 @@ data class Links(val next: String?, val prev: String?)
/** [PagingSource] for Mastodon Notifications, identified by the Notification ID */ /** [PagingSource] for Mastodon Notifications, identified by the Notification ID */
class NotificationsPagingSource @Inject constructor( class NotificationsPagingSource @Inject constructor(
private val mastodonApi: MastodonApi, private val mastodonApi: MastodonApi,
private val gson: Gson,
private val notificationFilter: Set<Notification.Type> private val notificationFilter: Set<Notification.Type>
) : PagingSource<String, Notification>() { ) : PagingSource<String, Notification>() {
override suspend fun load(params: LoadParams<String>): LoadResult<String, Notification> { override suspend fun load(params: LoadParams<String>): LoadResult<String, Notification> {
@ -58,7 +60,23 @@ class NotificationsPagingSource @Inject constructor(
} }
if (!response.isSuccessful) { if (!response.isSuccessful) {
return LoadResult.Error(Throwable(response.errorBody()?.string())) val code = response.code()
val msg = response.errorBody()?.string()?.let { errorBody ->
if (errorBody.isBlank()) return@let "no reason given"
val error = try {
gson.fromJson(errorBody, com.keylesspalace.tusky.entity.Error::class.java)
} catch (e: Exception) {
return@let "$errorBody ($e)"
}
when (val desc = error.error_description) {
null -> error.error
else -> "${error.error}: $desc"
}
} ?: "no reason given"
return LoadResult.Error(Throwable("HTTP $code: $msg"))
} }
val links = getPageLinks(response.headers()["link"]) val links = getPageLinks(response.headers()["link"])

View file

@ -23,6 +23,7 @@ import androidx.paging.Pager
import androidx.paging.PagingConfig import androidx.paging.PagingConfig
import androidx.paging.PagingData import androidx.paging.PagingData
import androidx.paging.PagingSource import androidx.paging.PagingSource
import com.google.gson.Gson
import com.keylesspalace.tusky.entity.Notification import com.keylesspalace.tusky.entity.Notification
import com.keylesspalace.tusky.network.MastodonApi import com.keylesspalace.tusky.network.MastodonApi
import kotlinx.coroutines.flow.Flow import kotlinx.coroutines.flow.Flow
@ -31,7 +32,8 @@ import retrofit2.Response
import javax.inject.Inject import javax.inject.Inject
class NotificationsRepository @Inject constructor( class NotificationsRepository @Inject constructor(
private val mastodonApi: MastodonApi private val mastodonApi: MastodonApi,
private val gson: Gson
) { ) {
private var factory: InvalidatingPagingSourceFactory<String, Notification>? = null private var factory: InvalidatingPagingSourceFactory<String, Notification>? = null
@ -47,7 +49,7 @@ class NotificationsRepository @Inject constructor(
Log.d(TAG, "getNotificationsStream(), filtering: $filter") Log.d(TAG, "getNotificationsStream(), filtering: $filter")
factory = InvalidatingPagingSourceFactory { factory = InvalidatingPagingSourceFactory {
NotificationsPagingSource(mastodonApi, filter) NotificationsPagingSource(mastodonApi, gson, filter)
} }
return Pager( return Pager(

View file

@ -0,0 +1,24 @@
/*
* Copyright 2023 Tusky Contributors
*
* This file is a part of Tusky.
*
* This program is free software; you can redistribute it and/or modify it under the terms of the
* GNU General Public License as published by the Free Software Foundation; either version 3 of the
* License, or (at your option) any later version.
*
* Tusky is distributed in the hope that it will be useful, but WITHOUT ANY WARRANTY; without even
* the implied warranty of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU General
* Public License for more details.
*
* You should have received a copy of the GNU General Public License along with Tusky; if not,
* see <http://www.gnu.org/licenses>.
*/
package com.keylesspalace.tusky.entity
/** @see [Error](https://docs.joinmastodon.org/entities/Error/) */
data class Error(
val error: String,
val error_description: String?
)

View file

@ -0,0 +1,145 @@
/*
* Copyright 2023 Tusky Contributors
*
* This file is a part of Tusky.
*
* This program is free software; you can redistribute it and/or modify it under the terms of the
* GNU General Public License as published by the Free Software Foundation; either version 3 of the
* License, or (at your option) any later version.
*
* Tusky is distributed in the hope that it will be useful, but WITHOUT ANY WARRANTY; without even
* the implied warranty of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU General
* Public License for more details.
*
* You should have received a copy of the GNU General Public License along with Tusky; if not,
* see <http://www.gnu.org/licenses>.
*/
package com.keylesspalace.tusky.components.notifications
import androidx.paging.PagingSource
import androidx.test.ext.junit.runners.AndroidJUnit4
import com.google.gson.Gson
import com.keylesspalace.tusky.entity.Notification
import com.keylesspalace.tusky.network.MastodonApi
import kotlinx.coroutines.ExperimentalCoroutinesApi
import kotlinx.coroutines.test.runTest
import okhttp3.ResponseBody.Companion.toResponseBody
import org.junit.Assert.assertEquals
import org.junit.Assert.assertTrue
import org.junit.Test
import org.junit.runner.RunWith
import org.mockito.kotlin.any
import org.mockito.kotlin.anyOrNull
import org.mockito.kotlin.doReturn
import org.mockito.kotlin.mock
import org.robolectric.annotation.Config
import retrofit2.Response
@Config(sdk = [28])
@RunWith(AndroidJUnit4::class)
@OptIn(ExperimentalCoroutinesApi::class)
class NotificationsPagingSourceTest {
@Test
fun `load() returns error message on HTTP error`() = runTest {
// Given
val jsonError = "{error: 'This is an error'}".toResponseBody()
val mockApi: MastodonApi = mock {
onBlocking { notifications(anyOrNull(), anyOrNull(), anyOrNull(), anyOrNull()) } doReturn Response.error(429, jsonError)
onBlocking { notification(any()) } doReturn Response.error(429, jsonError)
}
val filter = emptySet<Notification.Type>()
val gson = Gson()
val pagingSource = NotificationsPagingSource(mockApi, gson, filter)
val loadingParams = PagingSource.LoadParams.Refresh("0", 5, false)
// When
val loadResult = pagingSource.load(loadingParams)
// Then
assertTrue(loadResult is PagingSource.LoadResult.Error)
assertEquals(
"HTTP 429: This is an error",
(loadResult as PagingSource.LoadResult.Error).throwable.message
)
}
// As previous, but with `error_description` field as well.
@Test
fun `load() returns extended error message on HTTP error`() = runTest {
// Given
val jsonError = "{error: 'This is an error', error_description: 'Description of the error'}".toResponseBody()
val mockApi: MastodonApi = mock {
onBlocking { notifications(anyOrNull(), anyOrNull(), anyOrNull(), anyOrNull()) } doReturn Response.error(429, jsonError)
onBlocking { notification(any()) } doReturn Response.error(429, jsonError)
}
val filter = emptySet<Notification.Type>()
val gson = Gson()
val pagingSource = NotificationsPagingSource(mockApi, gson, filter)
val loadingParams = PagingSource.LoadParams.Refresh("0", 5, false)
// When
val loadResult = pagingSource.load(loadingParams)
// Then
assertTrue(loadResult is PagingSource.LoadResult.Error)
assertEquals(
"HTTP 429: This is an error: Description of the error",
(loadResult as PagingSource.LoadResult.Error).throwable.message
)
}
// As previous, but no error JSON, so expect default response
@Test
fun `load() returns default error message on empty HTTP error`() = runTest {
// Given
val jsonError = "{}".toResponseBody()
val mockApi: MastodonApi = mock {
onBlocking { notifications(anyOrNull(), anyOrNull(), anyOrNull(), anyOrNull()) } doReturn Response.error(429, jsonError)
onBlocking { notification(any()) } doReturn Response.error(429, jsonError)
}
val filter = emptySet<Notification.Type>()
val gson = Gson()
val pagingSource = NotificationsPagingSource(mockApi, gson, filter)
val loadingParams = PagingSource.LoadParams.Refresh("0", 5, false)
// When
val loadResult = pagingSource.load(loadingParams)
// Then
assertTrue(loadResult is PagingSource.LoadResult.Error)
assertEquals(
"HTTP 429: no reason given",
(loadResult as PagingSource.LoadResult.Error).throwable.message
)
}
// As previous, but malformed JSON, so expect response with enough information to troubleshoot
@Test
fun `load() returns useful error message on malformed HTTP error`() = runTest {
// Given
val jsonError = "{'malformedjson}".toResponseBody()
val mockApi: MastodonApi = mock {
onBlocking { notifications(anyOrNull(), anyOrNull(), anyOrNull(), anyOrNull()) } doReturn Response.error(429, jsonError)
onBlocking { notification(any()) } doReturn Response.error(429, jsonError)
}
val filter = emptySet<Notification.Type>()
val gson = Gson()
val pagingSource = NotificationsPagingSource(mockApi, gson, filter)
val loadingParams = PagingSource.LoadParams.Refresh("0", 5, false)
// When
val loadResult = pagingSource.load(loadingParams)
// Then
assertTrue(loadResult is PagingSource.LoadResult.Error)
assertEquals(
"HTTP 429: {'malformedjson} (com.google.gson.JsonSyntaxException: com.google.gson.stream.MalformedJsonException: Unterminated string at line 1 column 17 path \$.)",
(loadResult as PagingSource.LoadResult.Error).throwable.message
)
}
}