Optimistic / nonblocking message posting for chats
Merge request reports
Activity
74 74 }) 75 75 } else if (message.event === 'pleroma:chat_update') { 76 dispatch('addChatMessages', { 77 chatId: message.chatUpdate.id, 78 messages: [message.chatUpdate.lastMessage] 79 }) 80 dispatch('updateChat', { chat: message.chatUpdate }) 81 maybeShowChatNotification(store, message.chatUpdate) 76 setTimeout(() => { 77 dispatch('addChatMessages', { 78 chatId: message.chatUpdate.id, 79 messages: [message.chatUpdate.lastMessage] 80 }) 81 dispatch('updateChat', { chat: message.chatUpdate }) 82 maybeShowChatNotification(store, message.chatUpdate) 83 // Temporary band-aid to avoid duplicates when doing optimistic posting Yup, described the issue here #948 (comment 72332) (the WS event is delayed so that it arrives later then HTTP response).
I'd propose we extend the WS chat update event with something like the idempotency key to connect the fake message with the one coming from the WS event (which arrives earlier than the HTTP response). Another approach would be to save the
client_message_id, this also helps with handling errored messages on reconnect !1228 (comment 72596)Edited by Eugenijchanged this line in version 5 of the diff
Updated the branch, now it depends on this BE MR pleroma!3015 (merged) that adds the idempotency key to the
pleroma:chat_updateevent and so the hack with the artificial delay is no longer necessary.Edited by EugenijWithout the BE change, it works overall but the experience is quite frustrating: when the streaming is enabled, all the chat messages the user sends are duplicated if the idempotency key is not provided by BE. To mitigate that, we can use the artificial delay when receiving the user's own messages. This should cover the majority of the cases — the exception would be the BE HTTP response taking longer than the artificial delay.
Edited by Eugenij
now that I actually understand the problem, I think we should keep this temporary bandaid even with idempodency merged in now, just to support people who have upgraded FE to develop but haven't upgraded BE. We should keep it until we've had chat idempotency in stable releases for a while. The comment should have clear indication about why it's there and when we can get rid of it
added 9 commits
-
d357de57...da638940 - 7 commits from branch
pleroma:develop - 59b08582 - Optimistic message posting for chats
- 844674ad - Do not retry failed message on 4xx errors, count retries down to zero, use...
-
d357de57...da638940 - 7 commits from branch
W/r/t error handling, there is an issue with not knowing whether or not the message has been persisted during the connection loss.
E.g.,
- the user sends a message
- the connection loss occurs before the FE gets the response
- FE keeps the message marked as failed
- the connection is reestablished, the last messages are fetched, FE doesn't know whether or not the message that has been marked as failed is in that batch.
The simplest way is to remove the failed message on reconnect, but then we lose what the user has typed.
To avoid that, we can persist the
client_message_idin the DB and add it to the chat message model (I've noticed that slack probably is doing the same based on the message format it uses). Still, not entirely sure if supporting this rare case is worth the additional complexity.
mentioned in issue pleroma#2126 (closed)
added 1 commit
- 53f9d266 - Experimental client_message_id broadcast (requires the BE patch to work)
added 1 commit
- 972bbe3f - Experimental client_message_id broadcast (requires the BE patch to work)
added 20 commits
-
972bbe3f...0c0722a0 - 15 commits from branch
pleroma:develop - 21f2868a - Optimistic message posting for chats
- 73967496 - Do not retry failed message on 4xx errors, count retries down to zero, use...
- 7b49ac26 - Retry failed chat message only on 5xx error codes
- bbfc51df - Add the idempotency key to the chat message API call
- b0229319 - Experimental idempotency_key broadcast support (requires the BE patch to work)
Toggle commit list-
972bbe3f...0c0722a0 - 15 commits from branch
added 1 commit
- c2c790f0 - Experimental idempotency_key broadcast support (requires the BE patch to work)
added 1 commit
- a46a7d01 - Experimental idempotency_key broadcast support (requires the BE patch to work)
mentioned in merge request pleroma!3015 (merged)
added 1 commit
- d2983fa3 - Experimental idempotency_key broadcast support (requires the BE patch to work)
