From 12791406ca5ec1b7a82f62f6679e2d89c3cbc21b Mon Sep 17 00:00:00 2001 From: hayyaksi <193020925+hayaksi1@users.noreply.github.com> Date: Mon, 17 Aug 2026 16:14:15 +0300 Subject: [PATCH] Explain a rate limiting push gateway in the notification troubleshooter (#7448) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * 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 --- .../push/api/gateway/PushGatewayFailure.kt | 5 ++ .../pushgateway/PushGatewayNotifyRequest.kt | 43 ++++++++++----- .../impl/troubleshoot/PushLoopbackTest.kt | 7 +++ .../DefaultPushGatewayNotifyRequestTest.kt | 55 +++++++++++++++++++ .../impl/troubleshoot/PushLoopbackTestTest.kt | 19 +++++++ 5 files changed, 114 insertions(+), 15 deletions(-) diff --git a/libraries/push/api/src/main/kotlin/io/element/android/libraries/push/api/gateway/PushGatewayFailure.kt b/libraries/push/api/src/main/kotlin/io/element/android/libraries/push/api/gateway/PushGatewayFailure.kt index 975e10e134..3bf065550d 100644 --- a/libraries/push/api/src/main/kotlin/io/element/android/libraries/push/api/gateway/PushGatewayFailure.kt +++ b/libraries/push/api/src/main/kotlin/io/element/android/libraries/push/api/gateway/PushGatewayFailure.kt @@ -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() } diff --git a/libraries/push/impl/src/main/kotlin/io/element/android/libraries/push/impl/pushgateway/PushGatewayNotifyRequest.kt b/libraries/push/impl/src/main/kotlin/io/element/android/libraries/push/impl/pushgateway/PushGatewayNotifyRequest.kt index 6315285559..e3a7968909 100644 --- a/libraries/push/impl/src/main/kotlin/io/element/android/libraries/push/impl/pushgateway/PushGatewayNotifyRequest.kt +++ b/libraries/push/impl/src/main/kotlin/io/element/android/libraries/push/impl/pushgateway/PushGatewayNotifyRequest.kt @@ -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) diff --git a/libraries/push/impl/src/main/kotlin/io/element/android/libraries/push/impl/troubleshoot/PushLoopbackTest.kt b/libraries/push/impl/src/main/kotlin/io/element/android/libraries/push/impl/troubleshoot/PushLoopbackTest.kt index bc89d031a8..0277d7d2c8 100644 --- a/libraries/push/impl/src/main/kotlin/io/element/android/libraries/push/impl/troubleshoot/PushLoopbackTest.kt +++ b/libraries/push/impl/src/main/kotlin/io/element/android/libraries/push/impl/troubleshoot/PushLoopbackTest.kt @@ -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( diff --git a/libraries/push/impl/src/test/kotlin/io/element/android/libraries/push/impl/pushgateway/DefaultPushGatewayNotifyRequestTest.kt b/libraries/push/impl/src/test/kotlin/io/element/android/libraries/push/impl/pushgateway/DefaultPushGatewayNotifyRequestTest.kt index 47dc027b43..97f3bdc540 100644 --- a/libraries/push/impl/src/test/kotlin/io/element/android/libraries/push/impl/pushgateway/DefaultPushGatewayNotifyRequestTest.kt +++ b/libraries/push/impl/src/test/kotlin/io/element/android/libraries/push/impl/pushgateway/DefaultPushGatewayNotifyRequestTest.kt @@ -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(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(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( diff --git a/libraries/push/impl/src/test/kotlin/io/element/android/libraries/push/impl/troubleshoot/PushLoopbackTestTest.kt b/libraries/push/impl/src/test/kotlin/io/element/android/libraries/push/impl/troubleshoot/PushLoopbackTestTest.kt index f83fb660ce..447fdd7eb5 100644 --- a/libraries/push/impl/src/test/kotlin/io/element/android/libraries/push/impl/troubleshoot/PushLoopbackTestTest.kt +++ b/libraries/push/impl/src/test/kotlin/io/element/android/libraries/push/impl/troubleshoot/PushLoopbackTestTest.kt @@ -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.success(Unit) }