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

Room name/avatar replaced with older value after historical events emitted #1772

Open
1 of 2 tasks
PhantomRay opened this issue Apr 22, 2024 · 17 comments
Open
1 of 2 tasks
Labels
bug Something isn't working

Comments

@PhantomRay
Copy link
Contributor

PhantomRay commented Apr 22, 2024

Checklist

  • I could not find a solution in the documentation, the existing issues or discussions.
  • I already asked for help in the chat

In which Project did the bug appear?

Matrix dart SDK

On which platform did the bug appear?

Android, iOS

SDK Version

0.27

Describe the problem caused by this bug

Room info replaced after timeline. timeline.requestHistory(). From this point onwards, the room's state is changed in memory and local database.

Steps To Reproduce

  1. Sync rooms' data as usual
  2. use timeline.requestHistory for a particular room
  3. when the older events contain old room state like display name and avatar, it will override room's current value in memory and local database

Tasks

Preview Give feedback
No tasks being tracked yet.
@PhantomRay PhantomRay added the bug Something isn't working label Apr 22, 2024
@krille-chan
Copy link
Contributor

Hello @PhantomRay can you try this again with 0.27.0? We have refactored the logic there how to handle incoming events

@PhantomRay
Copy link
Contributor Author

Thanks a lot @krille-chan. I cannot test it directly in my app. But can certain use your demo app.

We rely on modified matrix api lite which supports jwt.

Hopefully you guys can support it very soon. Only a few lines of code.

@PhantomRay
Copy link
Contributor Author

In fact, I will try to integrate jwt feature to 0.27 now.

@PhantomRay
Copy link
Contributor Author

PhantomRay commented Apr 22, 2024

@krille-chan I just tested it in fluffychat with 0.27.0, the issue still remains.

@PhantomRay
Copy link
Contributor Author

Hi @krille-chan could you please treat this bug as high priority?
Because once it happens, re-launch the app will show incorrect room name and avatar.

@PhantomRay
Copy link
Contributor Author

Thanks @krille-chan Much appreciated.

@krille-chan
Copy link
Contributor

@PhantomRay I've tried to reproduce the bug in this way:

  1. Create a new room in FluffyChat and invite another test account
  2. Close the app
  3. From the other test account change the room name and send more than 10 messages
  4. Restart FluffyChat and load history because we received limited timeline
  5. Room Name was still correct 🤔

Do you have better way how to reproduce this?

@PhantomRay
Copy link
Contributor Author

I think my scenario might be different to yours, because you must be editing data using client API.
In my case, Matrix is integrated with our own system. We rely on admin API to push display name and avatar changes to matrix.

We use PUT /_synapse/admin/v2/users/<user_id> and change only displayname and avatar_url.

But either way, this shouldn't happen.

@PhantomRay
Copy link
Contributor Author

Btw, I'm currently testing direct chat (1 to1) only.

@nico-famedly
Copy link
Member

This should be fixed in 0.29.10, please reopen, if you can still reproduce it with that.

@PhantomRay
Copy link
Contributor Author

Thanks a lot!

@PhantomRay
Copy link
Contributor Author

In 0.29.10, it's still not fixed. Just to re-iterate, both of the following are overridden by older events:

  • room name/avatar
  • user name/avatar

@nico-famedly
Copy link
Member

Are you sure this isn't just an effect of not having cleared the cache? Does this affect new sessions or existing ones?

@PhantomRay
Copy link
Contributor Author

Our test is comprehensive including deleting the app.

Checked your recent code, noticed that you put some checks on event types. However I think that's not enough because we also have to compare the events' timestamps. If it's older than the time when room was loaded, or user's info previously set, then ignore.

@nico-famedly
Copy link
Member

The checks are on the event update types. We should only apply the state updates, if the update is a forward update, i.e. when there is a new sync response. Could you describe in more detail, where you are seeing the wrong state updates, i.e. what steps you are doing like jumping to some timeline location and then paginating forward or similar?

Timestamps would be incorrect, but we really should only be writing event updates to the database in the sync loop and we might still be missing some cases, where they get written outside of that loop?

@nico-famedly nico-famedly reopened this Jun 12, 2024
@PhantomRay
Copy link
Contributor Author

Steps To Reproduce

  1. create a direct chat room
  2. update user name/avatar
  3. send 50 or more messages
  4. change the user's name/avatar again
  5. send another 50+ messages
  6. sign out
  7. Sync rooms' data as usual (at this point, the room's state would reflect the latest data correctly)
  8. enter the room again and use timeline.requestHistory till the last message.
  9. then you would see that room's current value in memory and local database has been changed, including the user's info.

@nico-famedly
Copy link
Member

Thank you for the report, I will try to reproduce that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants