Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

hitting 'resend' on queued messages can still send in the wrong order #18942

Open
ara4n opened this issue Sep 8, 2021 · 3 comments · Fixed by matrix-org/matrix-js-sdk#3131
Open
Assignees
Labels
A-Timeline O-Uncommon Most users are unlikely to come across this or unexpected workflow S-Major Severely degrades major functionality or product features, with no satisfactory workaround T-Defect Team: App Z-Chronic

Comments

@ara4n
Copy link
Member

ara4n commented Sep 8, 2021

i'm on bad connectivity on a train. i sent 4 messages in a row. the first and fourth(?!) entered error state; the 2nd and 3rd were still pending. I hit retry to resend the failed ones; they end up sending in order 4,2,3,1. It feels like we should serialise the message sending so that the ordering is preserved and ensure that if one message gets stuck, it blocks subsequent ones, to avoid this miserable failure mode

@dbkr dbkr added O-Uncommon Most users are unlikely to come across this or unexpected workflow S-Minor Impairs non-critical functionality or suitable workarounds exist T-Defect labels Sep 8, 2021
@ara4n
Copy link
Member Author

ara4n commented Jul 18, 2022

see also #22885

@Johennes
Copy link
Contributor

Johennes commented Jan 20, 2023

I'm upgrading this to S-Major. For one thing, this is part of what we've identified as "Chronic Issues". For another, sending messages appears to be critical functionality to me and manually re-queuing them in the correct order doesn't feel like a satisfactory workaround to me (especially since by the time you realize they were send in the wrong order, the damage is already done).

@Johennes Johennes added S-Major Severely degrades major functionality or product features, with no satisfactory workaround Team: App Z-Chronic and removed S-Minor Impairs non-critical functionality or suitable workarounds exist labels Jan 20, 2023
su-ex added a commit to SchildiChat/matrix-js-sdk that referenced this issue Feb 28, 2023
* Element-R: implement encryption of outgoing events ([\matrix-org#3122](matrix-org#3122)).
* Poll model - page /relations results ([\matrix-org#3073](matrix-org#3073)). Contributed by @kerryarchibald.
* Poll model - validate end events ([\matrix-org#3072](matrix-org#3072)). Contributed by @kerryarchibald.
* Handle optional last_known_event_id property in m.predecessor ([\matrix-org#3119](matrix-org#3119)). Contributed by @andybalaam.
* Add support for stable identifier for fixed MAC in SAS verification ([\matrix-org#3101](matrix-org#3101)).
* Provide eventId as well as roomId from Room.findPredecessor ([\matrix-org#3095](matrix-org#3095)). Contributed by @andybalaam.
* MSC3946 Dynamic room predecessors ([\matrix-org#3042](matrix-org#3042)). Contributed by @andybalaam.
* Poll model ([\matrix-org#3036](matrix-org#3036)). Contributed by @kerryarchibald.
* Remove video tracks on video mute without renegotiating ([\matrix-org#3091](matrix-org#3091)).
* Introduces a backwards-compatible API change. `MegolmEncrypter#prepareToEncrypt`'s return type has changed from `void` to `() => void`. ([\matrix-org#3035](matrix-org#3035)). Contributed by @clarkf.
* Stop the ICE disconnected timer on call terminate ([\matrix-org#3147](matrix-org#3147)).
* Clear notifications when we can infer read status from receipts ([\matrix-org#3139](matrix-org#3139)). Fixes element-hq/element-web#23991.
* Messages sent out of order after one message fails ([\matrix-org#3131](matrix-org#3131)). Fixes element-hq/element-web#22885 and element-hq/element-web#18942. Contributed by @justjanne.
* Element-R: fix a bug which prevented encryption working after a reload ([\matrix-org#3126](matrix-org#3126)).
* Element-R: Fix invite processing ([\matrix-org#3121](matrix-org#3121)).
* Don't throw with no `opponentDeviceInfo` ([\matrix-org#3107](matrix-org#3107)).
* Remove flaky megolm test ([\matrix-org#3098](matrix-org#3098)). Contributed by @clarkf.
* Fix "verifyLinks" functionality of getRoomUpgradeHistory ([\matrix-org#3089](matrix-org#3089)). Contributed by @andybalaam.
su-ex added a commit to SchildiChat/matrix-js-sdk that referenced this issue Mar 15, 2023
* Add easy way to determine if the decryption failure is due to "DecryptionError: The sender has disabled encrypting to unverified devices." ([\matrix-org#3167](matrix-org#3167)). Contributed by @florianduros.
* Polls: expose end event id on poll model ([\matrix-org#3160](matrix-org#3160)). Contributed by @kerryarchibald.
* Polls: count undecryptable poll relations ([\matrix-org#3163](matrix-org#3163)). Contributed by @kerryarchibald.
* Fix spec compliance issue around encrypted `m.relates_to` ([\matrix-org#3178](matrix-org#3178)).
* Fix reactions in threads sometimes causing stuck notifications ([\matrix-org#3146](matrix-org#3146)). Fixes element-hq/element-web#24000. Contributed by @justjanne.
* Better type guard parseTopicContent ([\matrix-org#3165](matrix-org#3165)). Fixes matrix-org/element-web-rageshakes#20177 and matrix-org/element-web-rageshakes#20178.
* Fix a bug where events in encrypted rooms would sometimes erroneously increment the total unread counter after being processed locally. ([\matrix-org#3130](matrix-org#3130)). Fixes element-hq/element-web#24448. Contributed by @Half-Shot.
* Stop the ICE disconnected timer on call terminate ([\matrix-org#3147](matrix-org#3147)).
* Clear notifications when we can infer read status from receipts ([\matrix-org#3139](matrix-org#3139)). Fixes element-hq/element-web#23991.
* Messages sent out of order after one message fails ([\matrix-org#3131](matrix-org#3131)). Fixes element-hq/element-web#22885 and element-hq/element-web#18942. Contributed by @justjanne.
@ara4n
Copy link
Member Author

ara4n commented Dec 16, 2023

This has regressed. While suffering #26778, I ended up with two unsent messages which failed in 'red' state, and then three following messages which continued to try to send... and eventually successfully sent. Therefore the message order eventually ended up being 3,4,5,1,2 rather than 1,2,3,4,5.

@ara4n ara4n reopened this Dec 16, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-Timeline O-Uncommon Most users are unlikely to come across this or unexpected workflow S-Major Severely degrades major functionality or product features, with no satisfactory workaround T-Defect Team: App Z-Chronic
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants