fix: Show notification fetch errors instead of JSON (#942)

Previous code showed any JSON-wrapped errors from notification fetches
as the JSON string, instead of the error message.

Fix this by switching to `ApiResult` and using the formatted error
message.

Fixes 937
This commit is contained in:
Nik Clayton 2024-09-25 13:49:43 +02:00 committed by GitHub
parent 561c26ad4d
commit 3f9ee1d9c8
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
2 changed files with 29 additions and 24 deletions

View File

@ -31,9 +31,12 @@ import app.pachli.core.network.retrofit.MastodonApi
import app.pachli.worker.NotificationWorker import app.pachli.worker.NotificationWorker
import com.github.michaelbull.result.Err import com.github.michaelbull.result.Err
import com.github.michaelbull.result.Ok import com.github.michaelbull.result.Ok
import com.github.michaelbull.result.onFailure
import com.github.michaelbull.result.onSuccess
import dagger.hilt.android.qualifiers.ApplicationContext import dagger.hilt.android.qualifiers.ApplicationContext
import java.time.Instant import java.time.Instant
import javax.inject.Inject import javax.inject.Inject
import kotlin.collections.set
import kotlin.math.min import kotlin.math.min
import kotlin.time.Duration.Companion.milliseconds import kotlin.time.Duration.Companion.milliseconds
import kotlinx.coroutines.delay import kotlinx.coroutines.delay
@ -182,34 +185,36 @@ class NotificationFetcher @Inject constructor(
while (minId != null) { while (minId != null) {
val now = Instant.now() val now = Instant.now()
Timber.d("Fetching notifications from server") Timber.d("Fetching notifications from server")
val response = mastodonApi.notificationsWithAuth( mastodonApi.notificationsWithAuth(
authHeader, authHeader,
account.domain, account.domain,
minId = minId, minId = minId,
) ).onSuccess { response ->
if (!response.isSuccessful) { val notifications = response.body
val error = response.errorBody()?.string()
Timber.e("Fetching notifications from server failed: %s", error)
NotificationConfig.lastFetchNewNotifications[account.fullName] = Pair(now, Err(error ?: "Unknown error"))
break
}
NotificationConfig.lastFetchNewNotifications[account.fullName] = Pair(now, Ok(Unit)) NotificationConfig.lastFetchNewNotifications[account.fullName] = Pair(now, Ok(Unit))
Timber.i( Timber.i(
"Fetching notifications from server succeeded, returned %d notifications", "Fetching notifications from server succeeded, returned %d notifications",
response.body()?.size, notifications.size,
) )
// Notifications are returned in the page in order, newest first, // Notifications are returned in the page in order, newest first,
// (https://github.com/mastodon/documentation/issues/1226), insert the // (https://github.com/mastodon/documentation/issues/1226), insert the
// new page at the head of the list. // new page at the head of the list.
response.body()?.let { addAll(0, it) } addAll(0, notifications)
// Get the previous page, which will be chronologically newer // Get the previous page, which will be chronologically newer
// notifications. If it doesn't exist this is null and the loop // notifications. If it doesn't exist this is null and the loop
// will exit. // will exit.
val links = Links.from(response.headers()["link"]) val links = Links.from(response.headers["link"])
minId = links.prev minId = links.prev
} }
.onFailure {
val error = it.fmt(context)
Timber.e("Fetching notifications from server failed: %s", error)
NotificationConfig.lastFetchNewNotifications[account.fullName] = Pair(now, Err(error))
return@buildList
}
}
} }
// Save the newest notification ID in the marker. // Save the newest notification ID in the marker.

View File

@ -190,7 +190,7 @@ interface MastodonApi {
@Header(DOMAIN_HEADER) domain: String, @Header(DOMAIN_HEADER) domain: String,
/** Return results immediately newer than this ID */ /** Return results immediately newer than this ID */
@Query("min_id") minId: String?, @Query("min_id") minId: String?,
): Response<List<Notification>> ): ApiResult<List<Notification>>
@POST("api/v1/notifications/clear") @POST("api/v1/notifications/clear")
suspend fun clearNotifications(): Response<ResponseBody> suspend fun clearNotifications(): Response<ResponseBody>