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: delay outgoing message before sync is finished [#WPB-15141] 🍒 #3220

Merged

Conversation

github-actions[bot]
Copy link
Contributor

@github-actions github-actions bot commented Jan 9, 2025

BugWPB-15141 [Android] After playtest, coming back to the app, getting a lot of "Cannot decrypt" system messages

This PR was automatically cherry-picked based on the following PR:

Original PR description:


https://wearezeta.atlassian.net/browse/WPB-15141

What's new in this PR?

Issues

Preconditions:

  • Client was offline for a long time and has a lot of messages to sync.
  • Conversation has self deleting messages with short expiration.

User opens the app, sync is started and user see some messages that cannot be decrypted in the conversation.

Causes

  • Client receives a self delete message that expires before the sync is complete
  • Client sends delete message to sender to confirm message deletion before sync is finished.
  • Delete message is sent with old epoch value.
  • Client receives 'mls-stale-message' error and re-joins conversation.
  • All subsequent messages cannot be decrypted after re-join.

Solutions

Delay sending delete message until sync is complete. This ensures that message is sent with correct epoch value.

The issue has a broader context. Any message a client sends before sync is complete can cause re-join and break decryption. This should be covered as a part of: https://wearezeta.atlassian.net/browse/WPB-15262

@echoes-hq echoes-hq bot added the echoes: unplanned/incident Work items addressing critical product defects that require immediate action, such as bugs, platf... label Jan 9, 2025
@yamilmedina yamilmedina requested review from a team, typfel, sbakhtiarov, borichellow, MohamadJaara and Garzas and removed request for a team January 9, 2025 16:50
@MohamadJaara MohamadJaara disabled auto-merge January 13, 2025 11:00
@echoes-hq echoes-hq bot added echoes: product-roadmap/bug Work contributing to resolve a bug not critical enough to have raised an incident. and removed echoes: unplanned/incident Work items addressing critical product defects that require immediate action, such as bugs, platf... labels Jan 13, 2025
@sbakhtiarov sbakhtiarov enabled auto-merge January 15, 2025 16:42
Copy link
Contributor Author

github-actions bot commented Jan 15, 2025

Test Results

3 401 tests  ±0   3 293 ✅ ±0   5m 43s ⏱️ -5s
  584 suites ±0     108 💤 ±0 
  584 files   ±0       0 ❌ ±0 

Results for commit f199a70. ± Comparison against base commit b304f30.

♻️ This comment has been updated with latest results.

Copy link
Contributor Author

@sbakhtiarov sbakhtiarov added this pull request to the merge queue Jan 15, 2025
@codecov-commenter
Copy link

Codecov Report

Attention: Patch coverage is 42.85714% with 4 lines in your changes missing coverage. Please review.

Project coverage is 54.47%. Comparing base (b304f30) to head (f199a70).
Report is 1 commits behind head on develop.

Files with missing lines Patch % Lines
.../com/wire/kalium/logic/feature/debug/DebugScope.kt 0.00% 2 Missing ⚠️
.../wire/kalium/logic/feature/message/MessageScope.kt 0.00% 2 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff             @@
##           develop    #3220      +/-   ##
===========================================
- Coverage    54.48%   54.47%   -0.01%     
===========================================
  Files         1269     1269              
  Lines        36958    36962       +4     
  Branches      3745     3745              
===========================================
- Hits         20135    20134       -1     
- Misses       15410    15415       +5     
  Partials      1413     1413              
Files with missing lines Coverage Δ
...eteEphemeralMessageForSelfUserAsReceiverUseCase.kt 94.28% <100.00%> (+0.16%) ⬆️
.../com/wire/kalium/logic/feature/debug/DebugScope.kt 0.00% <0.00%> (ø)
.../wire/kalium/logic/feature/message/MessageScope.kt 0.00% <0.00%> (ø)

... and 3 files with indirect coverage changes


Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b304f30...f199a70. Read the comment docs.

@datadog-wireapp
Copy link

Datadog Report

Branch report: fix/outgoing-message-before-sync-finished-cherry-pick
Commit report: a9267ea
Test service: kalium-jvm

✅ 0 Failed, 3293 Passed, 108 Skipped, 1m 3.17s Total Time

Merged via the queue into develop with commit 7611f05 Jan 15, 2025
23 checks passed
@sbakhtiarov sbakhtiarov deleted the fix/outgoing-message-before-sync-finished-cherry-pick branch January 15, 2025 17:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cherry-pick PR is cherry-picking changes from another banch echoes: product-roadmap/bug Work contributing to resolve a bug not critical enough to have raised an incident. 🚨 Potential breaking changes 👕 size: S type: bug / fix 🐞
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants