Skip to content

Commit

Permalink
fix: Show notification fetch errors instead of JSON (#942)
Browse files Browse the repository at this point in the history
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
  • Loading branch information
nikclayton committed Sep 25, 2024
1 parent 561c26a commit 3f9ee1d
Show file tree
Hide file tree
Showing 2 changed files with 29 additions and 24 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -31,9 +31,12 @@ import app.pachli.core.network.retrofit.MastodonApi
import app.pachli.worker.NotificationWorker
import com.github.michaelbull.result.Err
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 java.time.Instant
import javax.inject.Inject
import kotlin.collections.set
import kotlin.math.min
import kotlin.time.Duration.Companion.milliseconds
import kotlinx.coroutines.delay
Expand Down Expand Up @@ -182,33 +185,35 @@ class NotificationFetcher @Inject constructor(
while (minId != null) {
val now = Instant.now()
Timber.d("Fetching notifications from server")
val response = mastodonApi.notificationsWithAuth(
mastodonApi.notificationsWithAuth(
authHeader,
account.domain,
minId = minId,
)
if (!response.isSuccessful) {
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
).onSuccess { response ->
val notifications = response.body
NotificationConfig.lastFetchNewNotifications[account.fullName] = Pair(now, Ok(Unit))
Timber.i(
"Fetching notifications from server succeeded, returned %d notifications",
notifications.size,
)

// Notifications are returned in the page in order, newest first,
// (https://github.com/mastodon/documentation/issues/1226), insert the
// new page at the head of the list.
addAll(0, notifications)

// Get the previous page, which will be chronologically newer
// notifications. If it doesn't exist this is null and the loop
// will exit.
val links = Links.from(response.headers["link"])
minId = links.prev
}
NotificationConfig.lastFetchNewNotifications[account.fullName] = Pair(now, Ok(Unit))
Timber.i(
"Fetching notifications from server succeeded, returned %d notifications",
response.body()?.size,
)

// Notifications are returned in the page in order, newest first,
// (https://github.com/mastodon/documentation/issues/1226), insert the
// new page at the head of the list.
response.body()?.let { addAll(0, it) }

// Get the previous page, which will be chronologically newer
// notifications. If it doesn't exist this is null and the loop
// will exit.
val links = Links.from(response.headers()["link"])
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
}
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -190,7 +190,7 @@ interface MastodonApi {
@Header(DOMAIN_HEADER) domain: String,
/** Return results immediately newer than this ID */
@Query("min_id") minId: String?,
): Response<List<Notification>>
): ApiResult<List<Notification>>

@POST("api/v1/notifications/clear")
suspend fun clearNotifications(): Response<ResponseBody>
Expand Down

0 comments on commit 3f9ee1d

Please sign in to comment.