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

Indexed DB can have inconsistency in membership event content #4198

Open
AndrewFerr opened this issue May 10, 2024 · 6 comments
Open

Indexed DB can have inconsistency in membership event content #4198

AndrewFerr opened this issue May 10, 2024 · 6 comments
Labels

Comments

@AndrewFerr
Copy link
Member

AndrewFerr commented May 10, 2024

It's possible for the sync store to have incorrect content of a m.room.member event in roomsData.join.$roomID.state[*], but correct content for that same event in roomsData.join.$roomID.timeline[*].

To reproduce:

  • Visit develop.element.io in two tabs, each tab signed in with a different user
  • Create/visit a room that has both users in it
  • Have one user leave the room
  • With the user that remains in the room:
    • Open Element devtools and view Explore room state > m.room.member > MXID of user who left
    • Open browser devtools and view Indexed DB for https://develop.element.io > matrix-js-sdk:riot-web-sync > sync
  • See that Element devtools shows the correct value of content.membership = "leave"
  • See that the Indexed DB has the same value for content.membership in the timeline event for the same eventID
  • See that the Indexed DB has an *incorrect value of content.membership = "join" in the state event for the same eventID

With that said, any client that uses timeline to load state won't have any problems (and I assume Element Web does that, as it has accurate member lists in rooms). But if a client were read from state, it would get incorrect data.

@dosubot dosubot bot added the T-Defect label May 10, 2024
@AndrewFerr
Copy link
Member Author

For example:

image

Element devtools & Indexed DB are showing the same event (as the room ID & event ID between the two are the same), but they each have a different value for "membership".

The correct content is in the timeline version of the event, though:

image

I assume that, unlike /sync's Joined Room response objects, the js-sdk's Indexed DB state should be a snapshot of the current room state, and not whatever the state was up until the start of timeline. But even if that weren't the case, it's still a problem that two occurrences of the same event in the Indexed DB have different "content"s.

@dbkr
Copy link
Member

dbkr commented May 13, 2024

Just looking at this for triage, but how are you getting two tabs signed in as different users? I don't see how this would be possible since they share the same localstorage etc: they would be logged in as the same user.

@MidhunSureshR
Copy link
Contributor

To add to the above comment, the app should also show you an error:

Image

@MidhunSureshR MidhunSureshR added the X-Needs-Info This issue is blocked awaiting information from the reporter label May 14, 2024
@AndrewFerr
Copy link
Member Author

how are you getting two tabs signed in as different users?

With Firefox container tabs.

But since the repro steps involve only 2 users, an alternative to using container tabs is to have one of the 2 users in a private browsing / incognito window.

@toger5
Copy link
Contributor

toger5 commented Jun 7, 2024

This is potentially related.
https://github.com/matrix-org/matrix-js-sdk/pull/4153/files

There was a switch from using a currentState cache to reading the state from the timeline.getState() method.

I tried refactoring this with the help of @t3chguy but there was more to do so the PR is currently put on ice.

@toger5
Copy link
Contributor

toger5 commented Jun 13, 2024

There is a new finding outlined here:
https://matrix.to/#/!jxlRxnrZCsjpjDubDX:matrix.org/$2X7ysozWciiyDHVev8iJ5yaFcZbZr57H2u8v5-7ysXw?via=matrix.org&via=envs.net&via=mozilla.org

State seems to be updated with older state.
The sync loop passes state to setStateEvents twice (occasionally) so we end up with multiple state changes.

@richvdh richvdh removed the X-Needs-Info This issue is blocked awaiting information from the reporter label Jun 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

5 participants