UnifiedPush: emit error when registration fails.

Note that I did not manage to have the method `onRegistrationFailed` invoked. If the network is not available for instance, unregistering the previous pusher will fail first.
This commit is contained in:
Benoit Marty 2025-11-13 12:34:40 +01:00
parent 6b7e8f72f5
commit f9c0b9e8bb
16 changed files with 208 additions and 23 deletions

View file

@ -12,6 +12,7 @@ import dev.zacsweers.metro.ContributesBinding
import io.element.android.libraries.core.extensions.flatMap
import io.element.android.libraries.core.log.logger.LoggerTag
import io.element.android.libraries.matrix.api.MatrixClientProvider
import io.element.android.libraries.push.api.PushService
import io.element.android.libraries.pushstore.api.clientsecret.PushClientSecret
import timber.log.Timber
@ -20,7 +21,7 @@ private val loggerTag = LoggerTag("UnifiedPushRemovedGatewayHandler", LoggerTag.
/**
* Handle new endpoint received from UnifiedPush. Will update the session matching the client secret.
*/
interface UnifiedPushRemovedGatewayHandler {
fun interface UnifiedPushRemovedGatewayHandler {
suspend fun handle(clientSecret: String): Result<Unit>
}
@ -29,9 +30,10 @@ class DefaultUnifiedPushRemovedGatewayHandler(
private val unregisterUnifiedPushUseCase: UnregisterUnifiedPushUseCase,
private val pushClientSecret: PushClientSecret,
private val matrixClientProvider: MatrixClientProvider,
private val pushService: PushService,
) : UnifiedPushRemovedGatewayHandler {
override suspend fun handle(clientSecret: String): Result<Unit> {
// Unregister the pusher for the session with this client secret, if is it using UnifiedPush.
// Unregister the pusher for the session with this client secret.
val userId = pushClientSecret.getUserIdFromSecret(clientSecret) ?: return Result.failure<Unit>(
IllegalStateException("Unable to retrieve session")
).also {
@ -39,15 +41,42 @@ class DefaultUnifiedPushRemovedGatewayHandler(
}
return matrixClientProvider
.getOrRestore(userId)
.onFailure {
Timber.tag(loggerTag.value).w(it, "Fails to restore client")
}
.flatMap { client ->
unregisterUnifiedPushUseCase.unregister(
matrixClient = client,
clientSecret = clientSecret,
unregisterUnifiedPush = false,
)
.onFailure {
Timber.tag(loggerTag.value).w(it, "Unable to unregister pusher")
}
.flatMap {
val pushProvider = pushService.getCurrentPushProvider(userId)
val distributor = pushProvider?.getCurrentDistributor(userId)
// Attempt to register again
if (pushProvider != null && distributor != null) {
pushService.registerWith(
client,
pushProvider,
distributor,
)
.onFailure {
Timber.tag(loggerTag.value).w(it, "Unable to register with current data")
}
} else {
Result.failure(IllegalStateException("Unable to register again"))
}
}
.onFailure {
// Let the user know
pushService.onServiceUnregistered(userId)
}
}
.onFailure {
Timber.tag(loggerTag.value).w(it, "Unable to unregister pusher")
Timber.tag(loggerTag.value).w(it, "Issue during pusher unregistration / re registration")
}
}
}

View file

@ -106,12 +106,14 @@ class VectorUnifiedPushMessagingReceiver : MessagingReceiver() {
*/
override fun onRegistrationFailed(context: Context, reason: FailedReason, instance: String) {
Timber.tag(loggerTag.value).e("onRegistrationFailed for $instance, reason: $reason")
/*
Toast.makeText(context, "Push service registration failed", Toast.LENGTH_SHORT).show()
val mode = BackgroundSyncMode.FDROID_BACKGROUND_SYNC_MODE_FOR_REALTIME
pushDataStore.setFdroidSyncBackgroundMode(mode)
guardServiceStarter.start()
*/
coroutineScope.launch {
endpointRegistrationHandler.registrationDone(
RegistrationResult(
clientSecret = instance,
result = Result.failure(Exception("Registration failed. Reason: $reason")),
)
)
}
}
/**

View file

@ -12,14 +12,21 @@ import io.element.android.libraries.matrix.api.MatrixClient
import io.element.android.tests.testutils.lambda.lambdaError
class FakeUnregisterUnifiedPushUseCase(
private val unregisterLambda: (MatrixClient, String) -> Result<Unit> = { _, _ -> lambdaError() },
private val cleanupLambda: (String) -> Unit = { lambdaError() },
private val unregisterLambda: (MatrixClient, String, Boolean) -> Result<Unit> = { _, _, _ -> lambdaError() },
private val cleanupLambda: (String, Boolean) -> Unit = { _, _ -> lambdaError() },
) : UnregisterUnifiedPushUseCase {
override suspend fun unregister(matrixClient: MatrixClient, clientSecret: String): Result<Unit> {
return unregisterLambda(matrixClient, clientSecret)
override suspend fun unregister(
matrixClient: MatrixClient,
clientSecret: String,
unregisterUnifiedPush: Boolean,
): Result<Unit> {
return unregisterLambda(matrixClient, clientSecret, unregisterUnifiedPush)
}
override fun cleanup(clientSecret: String) {
cleanupLambda(clientSecret)
override fun cleanup(
clientSecret: String,
unregisterUnifiedPush: Boolean,
) {
cleanupLambda(clientSecret, unregisterUnifiedPush)
}
}

View file

@ -118,7 +118,7 @@ class UnifiedPushProviderTest {
fun `unregister ok`() = runTest {
val matrixClient = FakeMatrixClient()
val getSecretForUserResultLambda = lambdaRecorder<SessionId, String> { A_SECRET }
val unregisterLambda = lambdaRecorder<MatrixClient, String, Result<Unit>> { _, _ -> Result.success(Unit) }
val unregisterLambda = lambdaRecorder<MatrixClient, String, Boolean, Result<Unit>> { _, _, _ -> Result.success(Unit) }
val unifiedPushProvider = createUnifiedPushProvider(
pushClientSecret = FakePushClientSecret(
getSecretForUserResult = getSecretForUserResultLambda,
@ -134,14 +134,14 @@ class UnifiedPushProviderTest {
.with(value(A_SESSION_ID))
unregisterLambda.assertions()
.isCalledOnce()
.with(value(matrixClient), value(A_SECRET))
.with(value(matrixClient), value(A_SECRET), value(true))
}
@Test
fun `unregister ko`() = runTest {
val matrixClient = FakeMatrixClient()
val getSecretForUserResultLambda = lambdaRecorder<SessionId, String> { A_SECRET }
val unregisterLambda = lambdaRecorder<MatrixClient, String, Result<Unit>> { _, _ -> Result.failure(AN_EXCEPTION) }
val unregisterLambda = lambdaRecorder<MatrixClient, String, Boolean, Result<Unit>> { _, _, _ -> Result.failure(AN_EXCEPTION) }
val unifiedPushProvider = createUnifiedPushProvider(
pushClientSecret = FakePushClientSecret(
getSecretForUserResult = getSecretForUserResultLambda,
@ -157,7 +157,7 @@ class UnifiedPushProviderTest {
.with(value(A_SESSION_ID))
unregisterLambda.assertions()
.isCalledOnce()
.with(value(matrixClient), value(A_SECRET))
.with(value(matrixClient), value(A_SECRET), value(true))
}
@Test
@ -230,7 +230,7 @@ class UnifiedPushProviderTest {
@Test
fun `onSessionDeleted should do the cleanup`() = runTest {
val cleanupLambda = lambdaRecorder<String, Unit> { }
val cleanupLambda = lambdaRecorder<String, Boolean, Unit> { _, _ -> }
val unifiedPushProvider = createUnifiedPushProvider(
pushClientSecret = FakePushClientSecret(
getSecretForUserResult = { A_SECRET }
@ -240,7 +240,7 @@ class UnifiedPushProviderTest {
),
)
unifiedPushProvider.onSessionDeleted(A_SESSION_ID)
cleanupLambda.assertions().isCalledOnce().with(value(A_SECRET))
cleanupLambda.assertions().isCalledOnce().with(value(A_SECRET), value(true))
}
private fun createUnifiedPushProvider(

View file

@ -23,6 +23,7 @@ import io.element.android.libraries.pushproviders.api.PushData
import io.element.android.libraries.pushproviders.api.PushHandler
import io.element.android.libraries.pushproviders.unifiedpush.registration.EndpointRegistrationHandler
import io.element.android.libraries.pushproviders.unifiedpush.registration.RegistrationResult
import io.element.android.tests.testutils.lambda.lambdaError
import io.element.android.tests.testutils.lambda.lambdaRecorder
import io.element.android.tests.testutils.lambda.value
import kotlinx.coroutines.ExperimentalCoroutinesApi
@ -51,10 +52,17 @@ class VectorUnifiedPushMessagingReceiverTest {
}
@Test
fun `onUnregistered does nothing`() = runTest {
fun `onUnregistered invokes the removedGatewayHandler`() = runTest {
val context = InstrumentationRegistry.getInstrumentation().context
val vectorUnifiedPushMessagingReceiver = createVectorUnifiedPushMessagingReceiver()
val handleResult = lambdaRecorder<String, Result<Unit>> {
Result.success(Unit)
}
val vectorUnifiedPushMessagingReceiver = createVectorUnifiedPushMessagingReceiver(
removedGatewayHandler = UnifiedPushRemovedGatewayHandler { handleResult(it) },
)
vectorUnifiedPushMessagingReceiver.onUnregistered(context, A_SECRET)
advanceUntilIdle()
handleResult.assertions().isCalledOnce().with(value(A_SECRET))
}
@Test
@ -199,6 +207,7 @@ class VectorUnifiedPushMessagingReceiverTest {
unifiedPushGatewayUrlResolver: UnifiedPushGatewayUrlResolver = FakeUnifiedPushGatewayUrlResolver(),
unifiedPushNewGatewayHandler: UnifiedPushNewGatewayHandler = FakeUnifiedPushNewGatewayHandler(),
endpointRegistrationHandler: EndpointRegistrationHandler = EndpointRegistrationHandler(),
removedGatewayHandler: UnifiedPushRemovedGatewayHandler = UnifiedPushRemovedGatewayHandler { lambdaError() },
): VectorUnifiedPushMessagingReceiver {
return VectorUnifiedPushMessagingReceiver().apply {
this.pushParser = unifiedPushParser
@ -208,6 +217,7 @@ class VectorUnifiedPushMessagingReceiverTest {
this.unifiedPushGatewayResolver = unifiedPushGatewayResolver
this.unifiedPushGatewayUrlResolver = unifiedPushGatewayUrlResolver
this.newGatewayHandler = unifiedPushNewGatewayHandler
this.removedGatewayHandler = removedGatewayHandler
this.endpointRegistrationHandler = endpointRegistrationHandler
this.coroutineScope = this@createVectorUnifiedPushMessagingReceiver
}