mirror of
https://github.com/Novaloop-AG/element-x-android.git
synced 2026-10-31 16:22:49 +00:00
Explain a rate limiting push gateway in the notification troubleshooter (#7448)
* Explain a rate limiting push gateway in the notification troubleshooter The push loop back test printed whatever HTTP status the gateway answered, so a user throttled by the public ntfy.sh gateway was told "Error: HTTP 507", which reads as a server fault rather than as a quota they can wait out or move away from. Map the two statuses that mean the notification was throttled onto a typed failure and give the troubleshooter a message that says what to do. * Import strings from localazy, remove `temporary.xml` in `:libraries:push:impl` --------- Co-authored-by: Jorge Martín <jorgem@element.io>
This commit is contained in:
+5
@@ -10,4 +10,9 @@ package io.element.android.libraries.push.api.gateway
|
||||
|
||||
sealed class PushGatewayFailure : Exception() {
|
||||
class PusherRejected : PushGatewayFailure()
|
||||
|
||||
/**
|
||||
* The gateway refused the notification because too many were sent to it.
|
||||
*/
|
||||
class RateLimited : PushGatewayFailure()
|
||||
}
|
||||
|
||||
+28
-15
@@ -12,6 +12,7 @@ import dev.zacsweers.metro.ContributesBinding
|
||||
import io.element.android.libraries.matrix.api.core.EventId
|
||||
import io.element.android.libraries.matrix.api.core.RoomId
|
||||
import io.element.android.libraries.push.api.gateway.PushGatewayFailure
|
||||
import retrofit2.HttpException
|
||||
|
||||
interface PushGatewayNotifyRequest {
|
||||
data class Params(
|
||||
@@ -33,26 +34,38 @@ class DefaultPushGatewayNotifyRequest(
|
||||
val pushGatewayApi = pushGatewayApiFactory.create(
|
||||
params.url.substringBefore(PushGatewayConfig.URI_PUSH_GATEWAY_PREFIX_PATH)
|
||||
)
|
||||
val response = pushGatewayApi.notify(
|
||||
PushGatewayNotifyBody(
|
||||
PushGatewayNotification(
|
||||
eventId = params.eventId.value,
|
||||
roomId = params.roomId.value,
|
||||
devices = listOf(
|
||||
PushGatewayDevice(
|
||||
params.appId,
|
||||
params.pushKey,
|
||||
PusherData(mapOf(
|
||||
"cs" to "A_FAKE_SECRET",
|
||||
))
|
||||
)
|
||||
),
|
||||
val response = try {
|
||||
pushGatewayApi.notify(
|
||||
PushGatewayNotifyBody(
|
||||
PushGatewayNotification(
|
||||
eventId = params.eventId.value,
|
||||
roomId = params.roomId.value,
|
||||
devices = listOf(
|
||||
PushGatewayDevice(
|
||||
params.appId,
|
||||
params.pushKey,
|
||||
PusherData(mapOf(
|
||||
"cs" to "A_FAKE_SECRET",
|
||||
))
|
||||
)
|
||||
),
|
||||
)
|
||||
)
|
||||
)
|
||||
)
|
||||
} catch (httpException: HttpException) {
|
||||
if (httpException.code() in RATE_LIMITED_STATUS_CODES) {
|
||||
throw PushGatewayFailure.RateLimited()
|
||||
}
|
||||
throw httpException
|
||||
}
|
||||
|
||||
if (response.rejectedPushKeys.contains(params.pushKey)) {
|
||||
throw PushGatewayFailure.PusherRejected()
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* 429 is the standard answer, ntfy answers 507 when a device is over its message quota.
|
||||
*/
|
||||
private val RATE_LIMITED_STATUS_CODES = setOf(429, 507)
|
||||
|
||||
+7
@@ -62,6 +62,13 @@ class PushLoopbackTest(
|
||||
)
|
||||
job.cancel()
|
||||
return
|
||||
} catch (_: PushGatewayFailure.RateLimited) {
|
||||
delegate.updateState(
|
||||
description = stringProvider.getString(R.string.troubleshoot_notifications_test_push_loop_back_failure_rate_limit),
|
||||
status = NotificationTroubleshootTestState.Status.Failure()
|
||||
)
|
||||
job.cancel()
|
||||
return
|
||||
} catch (e: Exception) {
|
||||
Timber.e(e, "Failed to test push")
|
||||
delegate.updateState(
|
||||
|
||||
+55
@@ -12,8 +12,11 @@ import com.google.common.truth.Truth.assertThat
|
||||
import io.element.android.libraries.push.api.gateway.PushGatewayFailure
|
||||
import io.element.android.libraries.push.impl.test.DefaultTestPush
|
||||
import kotlinx.coroutines.test.runTest
|
||||
import okhttp3.ResponseBody.Companion.toResponseBody
|
||||
import org.junit.Assert.assertThrows
|
||||
import org.junit.Test
|
||||
import retrofit2.HttpException
|
||||
import retrofit2.Response
|
||||
|
||||
class DefaultPushGatewayNotifyRequestTest {
|
||||
@Test
|
||||
@@ -64,6 +67,58 @@ class DefaultPushGatewayNotifyRequestTest {
|
||||
assertThat(factory.baseUrlParameter).isEqualTo("aUrl")
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `notify with a rate limiting status should throw expected Exception`() {
|
||||
listOf(429, 507).forEach { statusCode ->
|
||||
val factory = FakePushGatewayApiFactory(
|
||||
notifyResponse = {
|
||||
throw HttpException(Response.error<Unit>(statusCode, "".toResponseBody()))
|
||||
}
|
||||
)
|
||||
val pushGatewayNotifyRequest = DefaultPushGatewayNotifyRequest(
|
||||
pushGatewayApiFactory = factory,
|
||||
)
|
||||
assertThrows(PushGatewayFailure.RateLimited::class.java) {
|
||||
runTest {
|
||||
pushGatewayNotifyRequest.execute(
|
||||
PushGatewayNotifyRequest.Params(
|
||||
url = "aUrl",
|
||||
appId = "anAppId",
|
||||
pushKey = "aPushKey",
|
||||
eventId = DefaultTestPush.TEST_EVENT_ID,
|
||||
roomId = DefaultTestPush.TEST_ROOM_ID,
|
||||
)
|
||||
)
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `notify with another http error should not be mapped to a rate limit`() {
|
||||
val factory = FakePushGatewayApiFactory(
|
||||
notifyResponse = {
|
||||
throw HttpException(Response.error<Unit>(500, "".toResponseBody()))
|
||||
}
|
||||
)
|
||||
val pushGatewayNotifyRequest = DefaultPushGatewayNotifyRequest(
|
||||
pushGatewayApiFactory = factory,
|
||||
)
|
||||
assertThrows(HttpException::class.java) {
|
||||
runTest {
|
||||
pushGatewayNotifyRequest.execute(
|
||||
PushGatewayNotifyRequest.Params(
|
||||
url = "aUrl",
|
||||
appId = "anAppId",
|
||||
pushKey = "aPushKey",
|
||||
eventId = DefaultTestPush.TEST_EVENT_ID,
|
||||
roomId = DefaultTestPush.TEST_ROOM_ID,
|
||||
)
|
||||
)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `notify with rejected push key should throw expected Exception`() {
|
||||
val factory = FakePushGatewayApiFactory(
|
||||
|
||||
+19
@@ -59,6 +59,25 @@ class PushLoopbackTestTest {
|
||||
}
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `test PushLoopbackTest RateLimited error`() = runTest {
|
||||
val sut = createPushLoopbackTest(
|
||||
pushService = FakePushService(
|
||||
testPushBlock = {
|
||||
throw PushGatewayFailure.RateLimited()
|
||||
}
|
||||
),
|
||||
stringProvider = FakeStringProvider(A_FAILURE_REASON),
|
||||
)
|
||||
sut.runAndTestState {
|
||||
assertThat(awaitItem().status).isEqualTo(NotificationTroubleshootTestState.Status.Idle(true))
|
||||
assertThat(awaitItem().status).isEqualTo(NotificationTroubleshootTestState.Status.InProgress)
|
||||
val lastItem = awaitItem()
|
||||
assertThat(lastItem.status).isEqualTo(NotificationTroubleshootTestState.Status.Failure())
|
||||
assertThat(lastItem.description).isEqualTo(A_FAILURE_REASON)
|
||||
}
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `test PushLoopbackTest PusherRejected error with quick fix`() = runTest {
|
||||
val rotateTokenLambda = lambdaRecorder<Result<Unit>> { Result.success(Unit) }
|
||||
|
||||
Reference in New Issue
Block a user