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: sync failing when trying to update unknown user [WPB-4805] #2078

Merged
merged 1 commit into from
Sep 21, 2023

Conversation

saleniuk
Copy link
Contributor


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

The user is "stuck in connecting" after another user was added and removed from the team while he/she was offline.

Causes (Optional)

The app is stuck, because, while being offline, the other user was added and removed from the team we are in, so when we are back online, we receive team.memberjoin, user.update and only then team.memberleave. At the time we handle first one, the user is no longer in the team, so we get 404 when fetching user data and this user is not added to our local db. We ignore errors for team-specific events so it moves to handling the next one, which is user.update and again - we can’t handle it because we can’t update the user that’s not in our local db, but this time it results in a sync failure. lastProcessedEventId is not being updated because of this failure so it loops indefinitely when we retry the sync, because we try to handle the same event again.

Solutions

If the user that we want to update is not found in the local db, it means that this user is not in my team, is not my contact nor member of any of groups that I'm in. This means, that we can just safely ignore this event and move on to handle next ones without a failure.

Testing

Test Coverage (Optional)

  • I have added automated test to this contribution

How to Test

STR:

  • log in using the team member account A
  • terminate the app to be sure it’s offline
  • invite another member B to the team
  • join to the team by member B and set up the user handle (it probably creates user.update event)
  • remove this new member B from the team
  • open the app with user A logged in and it’s stuck

PR Post Submission Checklist for internal contributors (Optional)

  • Wire's Github Workflow has automatically linked the PR to a JIRA issue

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.

@github-actions
Copy link
Contributor

github-actions bot commented Sep 21, 2023

Unit Test Results

       6 files         6 suites   10m 10s ⏱️
2 246 tests 2 228 ✔️ 18 💤 0

Results for commit 51dc35f.

♻️ This comment has been updated with latest results.

@saleniuk saleniuk requested review from a team, yamilmedina, alexandreferris, borichellow, Garzas and trOnk12 and removed request for a team and trOnk12 September 21, 2023 13:31
@saleniuk saleniuk merged commit 395c2c1 into release/candidate Sep 21, 2023
@saleniuk saleniuk deleted the fix/updating_not_found_user branch September 21, 2023 13:46
github-merge-queue bot pushed a commit that referenced this pull request Sep 21, 2023
* fix: sync failing when trying to update unknown user [WPB-4805] (#2078)

* trigger build

---------

Co-authored-by: Michał Saleniuk <[email protected]>
Co-authored-by: Michał Saleniuk <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants