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.

Sign up for GitHub

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

Jump to bottom

fix: send calling messages in order [WPB-10051] #2915

Merged
merged 3 commits into from
Jul 30, 2024
Merged

fix: send calling messages in order [WPB-10051] #2915

merged 3 commits into from
Jul 30, 2024

Conversation

vitorhugods
Copy link
Member

@vitorhugods vitorhugods commented Jul 30, 2024 •
edited by github-actions bot
Loading

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
@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Jul 30, 2024 •
edited
Loading

Test Results

3 134 tests   3 029 ✔️  3m 36s ⏱️
   536 suites     105 💤
   536 files           0 ❌

Results for commit 5265c5b.

♻️ This comment has been updated with latest results.

@sonarqubecloud SonarQubeCloud
Copy link

Quality Gate Passed Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarCloud

@datadog-wireapp
Copy link

datadog-wireapp bot commented Jul 30, 2024 •
edited
Loading

Datadog Report

Branch report: rc/fix/enqeue-calling-messages
Commit report: 398c2fa
Test service: kalium-jvm

✅ 0 Failed, 3029 Passed, 105 Skipped, 11.02s Total Time

ohassine
ohassine approved these changes Jul 30, 2024
View reviewed changes
Copy link
Member

@ohassine ohassine left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✨✨

@vitorhugods vitorhugods enabled auto-merge (squash) July 30, 2024 08:14
@vitorhugods vitorhugods disabled auto-merge July 30, 2024 08:32
@vitorhugods vitorhugods merged commit fb092a8 into release/candidate Jul 30, 2024
21 checks passed
@vitorhugods vitorhugods deleted the rc/fix/enqeue-calling-messages branch July 30, 2024 08:33
github-actions bot pushed a commit that referenced this pull request Jul 30, 2024
@vitorhugods @github-actions
fix: send calling messages in order [WPB-10051] (#2915)
b4286ee
@github-actions github-actions bot mentioned this pull request Jul 30, 2024
Merged
6 tasks
vitorhugods added a commit that referenced this pull request Jul 30, 2024
@vitorhugods
fix: send calling message in order [WPB-10051] (#2915)
23cba55
@vitorhugods vitorhugods mentioned this pull request Jul 30, 2024
Merged
6 tasks
vitorhugods added a commit that referenced this pull request Jul 30, 2024
@github-actions @vitorhugods
fix: send calling messages in order [WPB-10051] (#2915) (#2916)
7d961c0 
Co-authored-by: Vitor Hugo Schwaab <[email protected]>
@github-actions github-actions bot mentioned this pull request Jul 30, 2024
Closed
6 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ohassine ohassine ohassine approved these changes

@alexandreferris alexandreferris alexandreferris approved these changes

@yamilmedina yamilmedina Awaiting requested review from yamilmedina

@MohamadJaara MohamadJaara Awaiting requested review from MohamadJaara

@Garzas Garzas Awaiting requested review from Garzas

@saleniuk saleniuk Awaiting requested review from saleniuk

Assignees

@vitorhugods vitorhugods

Labels
echoes: unplanned Any work item that isn’t part of the product or technical roadmap. 👕 size: XL type: bug / fix 🐞
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@vitorhugods @alexandreferris @ohassine