Fix/ses 4678 repeated sogs messages #1612
Conversation
| } | ||
|
|
||
| val response = getResponseBody(request, signRequest = true) | ||
| val reaction: AddReactionResponse = MessagingModuleConfiguration.shared.json.decodeFromStream(response.inputStream()) |
There was a problem hiding this comment.
I think we should close the input stream, even it's bytearray's input stream (same for the deleteReaction)
| val reaction: AddReactionResponse = MessagingModuleConfiguration.shared.json.decodeFromStream(response.inputStream()) | |
| val reaction: AddReactionResponse = response.inputStream().use( MessagingModuleConfiguration.shared.json::decodeFromStream) |
| @JvmStatic | ||
| fun deleteMessage(serverID: Long, room: String, server: String): Promise<Unit, Exception> { | ||
| suspend fun deleteMessage(serverID: Long, room: String, server: String) { |
There was a problem hiding this comment.
Can probably get rid of @JvmStatic: java is not going to be able to use this anyway
| } catch (exception: Exception) { | ||
| handleFailure(exception) | ||
| handleFailedMessageSend(message, exception) | ||
| throw exception |
There was a problem hiding this comment.
I think there might be an issue here: if we are using coroutine, we want to be careful about CancellationException: it's a special exception used by coroutine to interrupt the flow. In most cases, we should not interpret it as an error, and should rethrow it right away
| } catch (exception: Exception) { | |
| handleFailure(exception) | |
| handleFailedMessageSend(message, exception) | |
| throw exception | |
| } catch (exception: Exception) { | |
| if (exception !is CancellationException) handleFailedMessageSend(message, exception) | |
| throw exception |
| OpenGroupApi.addReaction( | ||
| room = recipient.address.room, | ||
| server = recipient.address.serverUrl, | ||
| messageId = messageServerId, | ||
| emoji = emoji | ||
| ) |
There was a problem hiding this comment.
Need try catch otherwise it will crash the app on error (Promise will silently ignore it but we can't with coroutine)
There was a problem hiding this comment.
This will apply to all fire-and-forget cases
| scope.launch { | ||
| try { | ||
| callManager.onNewOffer(sdp, callId, address) | ||
| } catch (e: Exception) { |
There was a problem hiding this comment.
Need special handling for CancellationException
| viewModel.blindedPublicKey, | ||
| isResync = true | ||
| ) | ||
| runCatching { |
There was a problem hiding this comment.
Should this be inside the forEach?
There was a problem hiding this comment.
Yeah, no reason to fail all is one doesn't go through
| OpenGroupApi.getDefaultServerCapabilities().map { | ||
| OpenGroupApi.getDefaultRoomsIfNeeded() | ||
| runCatching { | ||
| OpenGroupApi.getDefaultServerCapabilities().map { |
There was a problem hiding this comment.
Interesting. map was previously used on a Promise and now it's used for the List, so now it will run getDefaultRoomsIfNeededmultiple times
There was a problem hiding this comment.
True, this can happen sequentially
2 participants
SES-4678 - Community message duplication. This fixes an issues with the ix of Promises and coroutines. We want Promises out of the app, so this removes a bunch of them around the OpenGroupAPI and related classes
This also fixes an issue with removing reactions on other people's messages. There was confusion as to what the author is: The author of the reaction or of the message on which the reaction is added.
This also fixes a layout issue wit the reaction rail under the message, that would sometime appear in the middle