From 2d6d802da4c597d667bb83d82cb02111077d5b16 Mon Sep 17 00:00:00 2001 From: Brandon McAnsh Date: Fri, 26 Jun 2026 16:24:50 -0400 Subject: [PATCH] fix(chat): prevent offline message send from hanging and creating empty bubbles MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Restructure SharedFlow event handlers to use fire-and-forget pattern (viewModelScope.launch) for network calls, preventing backpressure from blocking all event delivery when any gRPC call hangs offline. Capture text and clear input synchronously on Main thread via flowOn before launching the send coroutine. Add 30s gRPC deadline to sendMessage so pending bubbles transition to FAILED status instead of hanging indefinitely. Add tap-to-retry flow for failed messages with in-place status update (FAILED→SENDING) to avoid UI jitter. Guard against blank message sends at both VM and coordinator layers. Signed-off-by: Brandon McAnsh --- .../core/src/main/res/values/strings.xml | 3 + .../app/messenger/internal/ChatViewModel.kt | 96 ++++++++++++------- .../internal/screens/MessengerScreen.kt | 8 ++ .../screens/components/MessageList.kt | 5 + .../screens/components/ReceiptLabel.kt | 17 +++- .../flipcash/shared/chat/ChatCoordinator.kt | 4 + .../internal/delegates/MessagingDelegate.kt | 20 ++++ .../sources/ChatMessageDataSource.kt | 10 ++ .../sources/mapper/chat/ChatEntityMapper.kt | 3 + .../internal/network/api/ChatMessagingApi.kt | 4 +- 10 files changed, 132 insertions(+), 38 deletions(-) diff --git a/apps/flipcash/core/src/main/res/values/strings.xml b/apps/flipcash/core/src/main/res/values/strings.xml index 92787dc3e..3331722f7 100644 --- a/apps/flipcash/core/src/main/res/values/strings.xml +++ b/apps/flipcash/core/src/main/res/values/strings.xml @@ -785,7 +785,10 @@ Delivered Read + Not Sent Yesterday + Message Not Sent + Your message could not be delivered. Would you like to try again? Today Send Money To Your Friends diff --git a/apps/flipcash/features/messenger/src/main/kotlin/com/flipcash/app/messenger/internal/ChatViewModel.kt b/apps/flipcash/features/messenger/src/main/kotlin/com/flipcash/app/messenger/internal/ChatViewModel.kt index 0e88ad4bc..1476998d3 100644 --- a/apps/flipcash/features/messenger/src/main/kotlin/com/flipcash/app/messenger/internal/ChatViewModel.kt +++ b/apps/flipcash/features/messenger/src/main/kotlin/com/flipcash/app/messenger/internal/ChatViewModel.kt @@ -65,6 +65,8 @@ import kotlinx.coroutines.flow.mapNotNull import kotlinx.coroutines.flow.onEach import kotlinx.coroutines.flow.stateIn import kotlinx.coroutines.flow.transformLatest +import kotlinx.coroutines.flow.flowOn +import kotlinx.coroutines.flow.mapNotNull import kotlinx.coroutines.launch import javax.inject.Inject import kotlin.math.min @@ -135,6 +137,7 @@ internal class ChatViewModel @Inject constructor( data object ResolveFailed : Event data object SendMessage : Event + data class RetryMessage(val pendingId: String?, val content: MessageContent) : Event data class NavigateToAmountEntry(val contact: DeviceContact) : Event data object PresentDepositOptions : Event @@ -269,7 +272,7 @@ internal class ChatViewModel @Inject constructor( if (chatId != null) { dispatchEvent(Event.ChatFound(chatId)) chatCoordinator.setActiveChatId(chatId) - chatCoordinator.loadMessages(chatId) + viewModelScope.launch { chatCoordinator.loadMessages(chatId) } chatCoordinator.dismissNotifications(chatId) } @@ -293,26 +296,24 @@ internal class ChatViewModel @Inject constructor( // Resolve owner authority for sending cash eventFlow .filterIsInstance() - .map { it.contact } - .map { - contactCoordinator.resolve(it.e164) - }.onResult( - onSuccess = { - dispatchEvent(Event.ResolveCompleted(it)) - }, - onError = { - dispatchEvent(Event.ResolveFailed) + .onEach { event -> + viewModelScope.launch { + contactCoordinator.resolve(event.contact.e164) + .onSuccess { dispatchEvent(Event.ResolveCompleted(it)) } + .onFailure { dispatchEvent(Event.ResolveFailed) } } - ).launchIn(viewModelScope) + }.launchIn(viewModelScope) // Re-resolve the contact from the device (e.g. after adding via system contacts) eventFlow .filterIsInstance() .mapNotNull { stateFlow.value.chattingWith?.e164 } .onEach { e164 -> - val refreshed = contactCoordinator.refreshContact(e164) - if (refreshed != null) { - dispatchEvent(Event.OnContactFound(refreshed)) + viewModelScope.launch { + val refreshed = contactCoordinator.refreshContact(e164) + if (refreshed != null) { + dispatchEvent(Event.OnContactFound(refreshed)) + } } } .launchIn(viewModelScope) @@ -341,7 +342,7 @@ internal class ChatViewModel @Inject constructor( .filterIsInstance() .onEach { event -> val chatId = stateFlow.value.chatId ?: return@onEach - chatCoordinator.advanceReadPointer(chatId, event.messageId) + viewModelScope.launch { chatCoordinator.advanceReadPointer(chatId, event.messageId) } } .launchIn(viewModelScope) } @@ -418,32 +419,31 @@ internal class ChatViewModel @Inject constructor( } .launchIn(viewModelScope) - // Notify server of typing state changes + // Notify server of typing state changes (fire-and-forget to avoid + // blocking SharedFlow emission when the gRPC call hangs offline) eventFlow.filterIsInstance() .mapNotNull { stateFlow.value.chatId } - .onEach { chatCoordinator.notifyTyping(it, TypingState.STARTED_TYPING) } + .onEach { viewModelScope.launch { chatCoordinator.notifyTyping(it, TypingState.STARTED_TYPING) } } .launchIn(viewModelScope) eventFlow.filterIsInstance() .mapNotNull { stateFlow.value.chatId } - .onEach { chatCoordinator.notifyTyping(it, TypingState.STILL_TYPING) } + .onEach { viewModelScope.launch { chatCoordinator.notifyTyping(it, TypingState.STILL_TYPING) } } .launchIn(viewModelScope) eventFlow.filterIsInstance() .mapNotNull { stateFlow.value.chatId } - .onEach { chatCoordinator.notifyTyping(it, TypingState.STOPPED_TYPING) } + .onEach { viewModelScope.launch { chatCoordinator.notifyTyping(it, TypingState.STOPPED_TYPING) } } .launchIn(viewModelScope) // Observe typing indicators once chatId is known - stateFlow.map { it.chatId } - .filterNotNull() + stateFlow.mapNotNull { it.chatId } .flatMapLatest { chatId -> chatCoordinator.observeTypingIndicators(chatId) } .onEach { typists -> dispatchEvent(Event.TypistsUpdated(typists)) } .launchIn(viewModelScope) // Enable typing notifications once a payment has been exchanged - stateFlow.map { it.chatId } - .filterNotNull() + stateFlow.mapNotNull { it.chatId } .distinctUntilChanged() .flatMapLatest { chatId -> chatCoordinator.observeMessages(chatId) @@ -459,20 +459,45 @@ internal class ChatViewModel @Inject constructor( private fun initSendHandlers() { // Send text message eventFlow.filterIsInstance() - .map { stateFlow.value.chatInputState } - .mapNotNull { textInput -> - val textToSend = textInput.text.toString() - val chatId = stateFlow.value.chatId ?: return@mapNotNull null + .onEach { + val textToSend = stateFlow.value.chatInputState.text.toString() + val chatId = stateFlow.value.chatId ?: return@onEach + if (textToSend.isBlank()) return@onEach + stateFlow.value.chatInputState.clearText() - chatCoordinator.sendMessage(chatId, textToSend) - }.onResult( - onSuccess = { - trace("message sent successfully") - }, - onError = { - trace("message failed to send - ${it.localizedMessage}") + + viewModelScope.launch { + chatCoordinator.sendMessage(chatId, textToSend) + .onSuccess { trace("message sent successfully") } + .onFailure { trace("message failed to send - ${it.localizedMessage}") } } - ) + } + .flowOn(Dispatchers.Main.immediate) + .launchIn(viewModelScope) + + // Retry a failed message + eventFlow.filterIsInstance() + .onEach { (pendingClientIdHex, content) -> + val chatId = stateFlow.value.chatId ?: return@onEach + val pendingId = pendingClientIdHex ?: return@onEach + + BottomBarManager.showInfo( + title = resources.getString(R.string.title_messageNotSent), + message = resources.getString(R.string.description_messageNotSent), + actions = listOf( + BottomBarAction( + text = resources.getString(R.string.action_retry), + ) { + viewModelScope.launch { + chatCoordinator.retryMessage(chatId, pendingId, listOf(content)) + .onSuccess { trace("retry message sent successfully") } + .onFailure { trace("retry message failed - ${it.localizedMessage}") } + } + }, + ), + showCancel = true, + ) + } .launchIn(viewModelScope) // confirmation of amount and checks @@ -687,6 +712,7 @@ internal class ChatViewModel @Inject constructor( state.copy(resolveState = ResolveState.Failed) } is Event.SendMessage -> { state -> state } + is Event.RetryMessage -> { state -> state } is Event.NavigateToAmountEntry -> { state -> state.copy(sendProgress = LoadingSuccessState()) } is Event.PresentDepositOptions -> { state -> state } is Event.OpenScreen -> { state -> state } diff --git a/apps/flipcash/features/messenger/src/main/kotlin/com/flipcash/app/messenger/internal/screens/MessengerScreen.kt b/apps/flipcash/features/messenger/src/main/kotlin/com/flipcash/app/messenger/internal/screens/MessengerScreen.kt index aa300dc33..0829eb398 100644 --- a/apps/flipcash/features/messenger/src/main/kotlin/com/flipcash/app/messenger/internal/screens/MessengerScreen.kt +++ b/apps/flipcash/features/messenger/src/main/kotlin/com/flipcash/app/messenger/internal/screens/MessengerScreen.kt @@ -74,6 +74,7 @@ internal fun MessengerScreen(viewModel: ChatViewModel) { val navigator = LocalCodeNavigator.current val hazeState = rememberHazeState() + val keyboard = rememberKeyboardController() ChatInputScaffold( topBar = { ChatTopBar(navigator, state.chattingWith) }, @@ -101,6 +102,13 @@ internal fun MessengerScreen(viewModel: ChatViewModel) { onAdvanceReadPointer = { messageId -> viewModel.dispatchEvent(ChatViewModel.Event.AdvanceReadPointer(messageId)) }, + onRetryMessage = { bubble -> + keyboard.hideIfVisible { + viewModel.dispatchEvent( + ChatViewModel.Event.RetryMessage(bubble.pendingClientIdHex, bubble.content) + ) + } + }, onRefreshContact = { viewModel.dispatchEvent(ChatViewModel.Event.RefreshContact) }, diff --git a/apps/flipcash/features/messenger/src/main/kotlin/com/flipcash/app/messenger/internal/screens/components/MessageList.kt b/apps/flipcash/features/messenger/src/main/kotlin/com/flipcash/app/messenger/internal/screens/components/MessageList.kt index 5a419e901..5cf28c1bb 100644 --- a/apps/flipcash/features/messenger/src/main/kotlin/com/flipcash/app/messenger/internal/screens/components/MessageList.kt +++ b/apps/flipcash/features/messenger/src/main/kotlin/com/flipcash/app/messenger/internal/screens/components/MessageList.kt @@ -61,6 +61,7 @@ internal fun MessageList( separatorConfig: SeparatorConfig, otherReadPointer: MessagePointer? = null, onAdvanceReadPointer: ((Long) -> Unit)? = null, + onRetryMessage: ((ChatListItem.ContentBubble) -> Unit)? = null, onRefreshContact: () -> Unit = {}, ) { val keyboard = rememberKeyboardController() @@ -215,6 +216,9 @@ internal fun MessageList( status = effectiveStatus, readPointer = otherReadPointer, animateEntrance = wasSending, + onRetryFailed = if (effectiveStatus == ReceiptStatus.FAILED) { + { onRetryMessage?.invoke(item) } + } else null, ) } } @@ -401,6 +405,7 @@ private fun shouldShowReceiptLabel( ): Boolean { if (!item.isFromSelf) return false val status = effectiveReceiptStatus(item, otherReadPointer) ?: return false + if (status == ReceiptStatus.FAILED) return true if (status != ReceiptStatus.SENT && status != ReceiptStatus.READ) return false // index - 1 is the item below (newer) in reverseLayout diff --git a/apps/flipcash/features/messenger/src/main/kotlin/com/flipcash/app/messenger/internal/screens/components/ReceiptLabel.kt b/apps/flipcash/features/messenger/src/main/kotlin/com/flipcash/app/messenger/internal/screens/components/ReceiptLabel.kt index 5e1ec78b4..456ddd1a1 100644 --- a/apps/flipcash/features/messenger/src/main/kotlin/com/flipcash/app/messenger/internal/screens/components/ReceiptLabel.kt +++ b/apps/flipcash/features/messenger/src/main/kotlin/com/flipcash/app/messenger/internal/screens/components/ReceiptLabel.kt @@ -12,6 +12,7 @@ import androidx.compose.animation.scaleIn import androidx.compose.animation.scaleOut import androidx.compose.animation.shrinkVertically import androidx.compose.animation.togetherWith +import androidx.compose.foundation.clickable import androidx.compose.foundation.layout.Arrangement import androidx.compose.foundation.layout.Box import androidx.compose.foundation.layout.Column @@ -53,6 +54,7 @@ internal fun ReceiptLabel( readPointer: MessagePointer?, modifier: Modifier = Modifier, animateEntrance: Boolean = false, + onRetryFailed: (() -> Unit)? = null, ) { // iOS: "Delivered" hides instantly on send, then appears after 700ms with // scale(0.95)+opacity spring (duration: 0.4, bounce: 0.12). @@ -103,6 +105,7 @@ internal fun ReceiptLabel( val text = when (animatedStatus) { ReceiptStatus.SENT -> stringResource(R.string.label_chatReceipt_delivered) ReceiptStatus.READ -> stringResource(R.string.label_chatReceipt_read) + ReceiptStatus.FAILED -> stringResource(R.string.label_chatReceipt_notSent) else -> return@AnimatedContent } @@ -110,14 +113,24 @@ internal fun ReceiptLabel( readPointer?.timestamp?.let { formatReadTimestamp(it) } ?: "" Row( - horizontalArrangement = Arrangement.spacedBy(CodeTheme.dimens.grid.x1)) { + modifier = if (animatedStatus == ReceiptStatus.FAILED && onRetryFailed != null) { + Modifier.clickable(onClick = onRetryFailed) + } else { + Modifier + }, + horizontalArrangement = Arrangement.spacedBy(CodeTheme.dimens.grid.x1), + ) { Text( modifier = Modifier.alignByBaseline(), text = text, style = CodeTheme.typography.caption.copy( fontWeight = FontWeight.Bold, ), - color = CodeTheme.colors.textSecondary, + color = if (animatedStatus == ReceiptStatus.FAILED) { + CodeTheme.colors.error + } else { + CodeTheme.colors.textSecondary + }, ) if (animatedStatus == ReceiptStatus.READ && readAtFormatted.isNotEmpty()) { diff --git a/apps/flipcash/shared/chat/src/main/kotlin/com/flipcash/shared/chat/ChatCoordinator.kt b/apps/flipcash/shared/chat/src/main/kotlin/com/flipcash/shared/chat/ChatCoordinator.kt index e34eec357..0660397ef 100644 --- a/apps/flipcash/shared/chat/src/main/kotlin/com/flipcash/shared/chat/ChatCoordinator.kt +++ b/apps/flipcash/shared/chat/src/main/kotlin/com/flipcash/shared/chat/ChatCoordinator.kt @@ -8,6 +8,7 @@ import com.flipcash.app.core.contacts.DeviceContact import com.flipcash.services.models.chat.ChatId import com.flipcash.services.models.chat.ChatMember import com.flipcash.services.models.chat.ChatMessage +import com.flipcash.services.models.chat.MessageContent import com.flipcash.services.models.chat.MessagePointer import com.flipcash.services.models.chat.ReactionSummary import com.flipcash.services.models.chat.TypingState @@ -87,6 +88,9 @@ interface MessagingOperations { /** Sends a text message to [chatId]. Returns the server-confirmed [ChatMessage]. */ suspend fun sendMessage(chatId: ChatId, content: String): Result + /** Retries a failed pending message: resets to SENDING and re-sends to the server. */ + suspend fun retryMessage(chatId: ChatId, pendingClientIdHex: String, content: List): Result + /** Advances the local and remote read pointer for [chatId] to [messageId]. */ suspend fun advanceReadPointer(chatId: ChatId, messageId: Long): Result diff --git a/apps/flipcash/shared/chat/src/main/kotlin/com/flipcash/shared/chat/internal/delegates/MessagingDelegate.kt b/apps/flipcash/shared/chat/src/main/kotlin/com/flipcash/shared/chat/internal/delegates/MessagingDelegate.kt index 589e639b6..84678139d 100644 --- a/apps/flipcash/shared/chat/src/main/kotlin/com/flipcash/shared/chat/internal/delegates/MessagingDelegate.kt +++ b/apps/flipcash/shared/chat/src/main/kotlin/com/flipcash/shared/chat/internal/delegates/MessagingDelegate.kt @@ -141,6 +141,10 @@ class MessagingDelegate @Inject constructor( } override suspend fun sendMessage(chatId: ChatId, content: String): Result { + if (content.isBlank()) { + return Result.failure(IllegalArgumentException("Cannot send a blank message")) + } + val senderId = userManager.accountId ?: return Result.failure(IllegalStateException("Cannot send message without an account")) @@ -164,6 +168,22 @@ class MessagingDelegate @Inject constructor( } } + override suspend fun retryMessage(chatId: ChatId, pendingClientIdHex: String, content: List): Result { + val clientMessageId = messageDataSource.retryPending(chatId, pendingClientIdHex) + + return messagingController.sendMessage(chatId, content, clientMessageId) + .onSuccess { serverMessage -> + messageDataSource.confirmPending(chatId, clientMessageId, serverMessage) + advanceReadPointer(chatId, serverMessage.messageId) + + metadataDataSource.updateLastMessageId(chatId, serverMessage.messageId) + metadataDataSource.updateLastActivity(chatId, serverMessage.timestamp.toEpochMilliseconds()) + } + .onFailure { + messageDataSource.failPending(chatId, clientMessageId) + } + } + override suspend fun advanceReadPointer(chatId: ChatId, messageId: Long): Result { val selfId = userManager.accountId ?: return Result.failure( IllegalStateException("No account") diff --git a/apps/flipcash/shared/persistence/sources/src/main/kotlin/com/flipcash/app/persistence/sources/ChatMessageDataSource.kt b/apps/flipcash/shared/persistence/sources/src/main/kotlin/com/flipcash/app/persistence/sources/ChatMessageDataSource.kt index ec690e5a1..c8405b0da 100644 --- a/apps/flipcash/shared/persistence/sources/src/main/kotlin/com/flipcash/app/persistence/sources/ChatMessageDataSource.kt +++ b/apps/flipcash/shared/persistence/sources/src/main/kotlin/com/flipcash/app/persistence/sources/ChatMessageDataSource.kt @@ -162,6 +162,16 @@ class ChatMessageDataSource @Inject constructor( ) } + suspend fun retryPending(chatId: ChatId, pendingClientIdHex: String): ClientMessageId { + val clientMessageId = mapper.clientMessageIdFromHex(pendingClientIdHex) + db?.chatMessageDao()?.updatePendingStatus( + mapper.chatIdHex(chatId), + pendingClientIdHex, + MessageStatus.SENDING, + ) + return clientMessageId + } + fun toChatMessage(entity: ChatMessageEntity): ChatMessage { val message = mapper.toMessage(entity) val selfId = userManager.accountId diff --git a/apps/flipcash/shared/persistence/sources/src/main/kotlin/com/flipcash/app/persistence/sources/mapper/chat/ChatEntityMapper.kt b/apps/flipcash/shared/persistence/sources/src/main/kotlin/com/flipcash/app/persistence/sources/mapper/chat/ChatEntityMapper.kt index 079ab7d59..f87a62b9d 100644 --- a/apps/flipcash/shared/persistence/sources/src/main/kotlin/com/flipcash/app/persistence/sources/mapper/chat/ChatEntityMapper.kt +++ b/apps/flipcash/shared/persistence/sources/src/main/kotlin/com/flipcash/app/persistence/sources/mapper/chat/ChatEntityMapper.kt @@ -169,6 +169,9 @@ class ChatEntityMapper @Inject constructor() { fun clientMessageIdHex(clientMessageId: ClientMessageId): String = clientMessageId.bytes.toList().hexEncodedString() + fun clientMessageIdFromHex(hex: String): ClientMessageId = + ClientMessageId(hex.hexToByteArray()) + fun pointerToJson(pointer: MessagePointer): String { return kotlinx.serialization.json.Json.encodeToString( listOf(pointer.toSerialized()) diff --git a/services/flipcash/src/main/kotlin/com/flipcash/services/internal/network/api/ChatMessagingApi.kt b/services/flipcash/src/main/kotlin/com/flipcash/services/internal/network/api/ChatMessagingApi.kt index 35987db50..684d51224 100644 --- a/services/flipcash/src/main/kotlin/com/flipcash/services/internal/network/api/ChatMessagingApi.kt +++ b/services/flipcash/src/main/kotlin/com/flipcash/services/internal/network/api/ChatMessagingApi.kt @@ -29,6 +29,7 @@ import dev.bmcreations.protovalidate.orThrow import io.grpc.ManagedChannel import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.withContext +import java.util.concurrent.TimeUnit import javax.inject.Inject import javax.inject.Singleton @@ -114,7 +115,8 @@ internal class ChatMessagingApi @Inject constructor( request.validate().orThrow() return withContext(Dispatchers.IO) { - api.sendMessage(request) + api.withDeadlineAfter(30, TimeUnit.SECONDS) + .sendMessage(request) } }