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

fix: send calling message in order [WPB-10051] #2920

Merged

Conversation

vitorhugods
Copy link
Member

Manually cherry picked from #2915:

BugWPB-10051 [Android] join call button stuck even after the call have ended


PR Submission Checklist for internal contributors

  • The PR Title

    • conforms to the style of semantic commits messages¹ supported in Wire's Github Workflow²
    • contains a reference JIRA issue number like SQPIT-764
    • answers the question: If merged, this PR will: ... ³
  • The PR Description

    • is free of optional paragraphs and you have filled the relevant parts to the best of your ability

What's new in this PR?

Issues

Calling messages are not always sent in order. Leading to issues like "Join" button getting stuck on Android.

Causes

AVS asks us to send OTR messages.
We delegate that to MessageSender, which has no order guarantee.

If called with two messages and one takes longer than the other, the MessageSender might switch the order around.

Solutions

  • Rename OnHttpRequest to CallingMessageSender, which is focused exactly on sending calling messages.
  • CallingMessageSender has a queue of CallingMessageInstructions to process in order.
  • In case of success or failure, it calls AVS.

Testing

Test Coverage

  • I have added automated test to this contribution

PR Post Merge Checklist for internal contributors

  • If any soft of configuration variable was introduced by this PR, it has been added to the relevant documents and the CI jobs have been updated.

References
  1. https://sparkbox.com/foundry/semantic_commit_messages
  2. https://github.com/wireapp/.github#usage
  3. E.g. feat(conversation-list): Sort conversations by most emojis in the title #SQPIT-764.

@echoes-hq echoes-hq bot added the echoes: unplanned Any work item that isn’t part of the product or technical roadmap. label Jul 30, 2024
Copy link
Contributor

github-actions bot commented Jul 30, 2024

Test Results

3 041 tests   2 934 ✔️  3m 5s ⏱️
   521 suites     107 💤
   521 files           0

Results for commit a39370f.

♻️ This comment has been updated with latest results.

@datadog-wireapp
Copy link

datadog-wireapp bot commented Jul 30, 2024

Datadog Report

All test runs 85ac3be 🔗

2 Total Test Services: 0 Failed, 2 Passed

Test Services
Service Name Failed Known Flaky New Flaky Passed Skipped Total Time Test Service View
kalium-ios 0 0 0 2789 123 1.29s Link
kalium-jvm 0 0 0 2934 107 7.21s Link

Copy link

@vitorhugods vitorhugods enabled auto-merge (squash) July 30, 2024 16:52
@vitorhugods vitorhugods disabled auto-merge July 30, 2024 16:52
@vitorhugods vitorhugods merged commit 28e19a6 into release/android-cycle-4.6 Jul 30, 2024
19 checks passed
@vitorhugods vitorhugods deleted the 4.6/fix/enqueue-calling-messages branch July 30, 2024 16:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
echoes: unplanned Any work item that isn’t part of the product or technical roadmap. 👕 size: XL type: bug / fix 🐞
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants