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

Searching for users sometimes appears not to work properly. #6599

Closed
AmandineLP opened this issue Apr 25, 2018 · 16 comments
Closed

Searching for users sometimes appears not to work properly. #6599

AmandineLP opened this issue Apr 25, 2018 · 16 comments
Labels
P1 S-Critical Prevents work, causes data loss and/or has no workaround T-Defect X-Regression Z-Synapse
Milestone

Comments

@AmandineLP
Copy link
Contributor

I want to start a chat with benoit, we're both in the team internal room, but when clicking Start chat and searching for 'benoit' he doesn't appear in the results

@lampholder lampholder added T-Defect P1 S-Major Severely degrades major functionality or product features, with no satisfactory workaround labels Apr 25, 2018
@lampholder lampholder modified the milestones: RW009 - Candidates, RW008 - Candidates, Temporary Bug Bin Apr 26, 2018
@ara4n
Copy link
Member

ara4n commented May 2, 2018

relatedly, icons seem to be missing entirely from search results too. plus I also couldn't find stevio earlier.

@ara4n
Copy link
Member

ara4n commented May 2, 2018

this is precisely the sort of regression that i think we should be prioritising fixes for rather than letting it overflow into future sprints...

@lampholder lampholder modified the milestones: Temporary Bug Bin, RW010 Jun 11, 2018
@t3chguy
Copy link
Member

t3chguy commented Jun 16, 2018

It seems to work for me so could you verify whether it still occurs for you @AmandineLP
image

@dbkr
Copy link
Member

dbkr commented Jul 17, 2018

I also can't repro this (ie. he appears for me)

@lampholder
Copy link
Member

I've heard a bunch of anecdotal reports of users not turning up in search results (whereby search == invite user dialogue).

There are a few ingredients to this problem, including:

  • the not-always-obvious rules about when users are supposed to turn up in search results
  • the varying settings server to server
  • federation
  • confusing ux (whereby riot 'completes' a pill even if it hasn't actually found a user if the mxid is valid)
  • nondeterministic behaviour (searching sometimes yields different results for two searchs in succession)

@lampholder
Copy link
Member

lampholder commented Sep 21, 2018

First up - what is the behaviour meant to be? Forgetting for a moment about the UX - who should come back in search results when you search?

I think it is something like:

  • if homeserver.com's homeserver.yaml contains user-directory: search_all_users: true and othershomeserver.com's homeserver.yaml contains user-directory: search_all_users: false:
    • when @ bob:homeserver.com searches, he should see:
      • all the users on homeserver.com
      • users on otherhomeserver.com with whom he has ever shared a room
    • when @ alice:otherhomeserver.com searches, she should see
      • only users on homeserver.com and otherhomeserver.com with whom she has ever shared a room

What part do public rooms play in the above? @ara4n ?

@lampholder
Copy link
Member

lampholder commented Sep 21, 2018

EDIT: UX thread extracted to https://github.com/vector-im/riot-web/issues/7373

@lampholder lampholder changed the title benoit is not appearing in the search results Searching for users sometimes appears not to work properly. Sep 21, 2018
@lampholder
Copy link
Member

Right, I'm going to break this issue in two. This issue will represent 'inviting is broken'. I'll make a separate issue for 'invite UX is confusing'.

Some useful conversation from #6970 which I've killed off for simplicity:
screenshot 2018-09-21 at 17 56 17

@ara4n
Copy link
Member

ara4n commented Sep 22, 2018

First up - what is the behaviour meant to be? Forgetting for a moment about the UX - who should come back in search results when you search?

If your server has search_all_users: false (the default), search results should return:

  • Users who you share a room with.
  • Users who are in public rooms.

If search_all_users: true, search results should return:

  • All users known to your server (i.e. local & remote users which participate in a room which exists on that server)

The directory is maintained only by your local server, and does not interact with the directories maintained by other servers (which are not exposed over federation).

There seem to be two separate bugs here:

My theory is that synapse is failing to keep the user directory maintained correctly... and Riot is failing to sometimes provide the right profile info (given this is certainly an intermittent bug).

@ara4n ara4n added S-Critical Prevents work, causes data loss and/or has no workaround and removed S-Major Severely degrades major functionality or product features, with no satisfactory workaround labels Sep 22, 2018
@turt2live
Copy link
Member

Author's note: This comment is just a research notes dump to hopefully help track down possible problems here.


Riot is failing to sometimes provide the right profile info

This sounds like a symptom of synapse not knowing entirely what to return, given I would expect Riot to just use the profile info returned from the search results. Unless Riot does something weird, in which case it probably shouldn't. Profiles being broken sounds a bit like matrix-org/synapse#3310

Because this issue is most prominent on matrix.org (which might be a symptom of the user count) and matrix.org uses workers, I'm tempted to blame matrix-org/synapse#3714 to a degree. matrix-org/synapse#3631 might also be related, although that doesn't appear to be the case in this issue.

The directory being unstable appears to be hinted at by matrix-org/synapse#2306

@turt2live
Copy link
Member

also for information gathering purposes: matrix.org uses update_user_directory: false (because worker) and search_all_users: false, implying this might be a race condition (or similar) in synapse, as described by matrix-org/synapse#3714 - although shouldn't the race condition be fixed by update_user_directory: false? (to be continued...)

@turt2live turt2live self-assigned this Oct 4, 2018
@turt2live
Copy link
Member

more info:

meanwhile, i suspect riot might have a codepath to insert local members in to the user lookup rather than just use the directory
which might explain why it sometimes magically starts working

@turt2live
Copy link
Member

I can't seem to replicate the issue locally, but it is easily reproducible on matrix.org:

  1. Create 2 accounts and log in to both
  2. Create a new room with Account 1
  3. Try to invite Account 2 by name only - it should fail to find a result, requiring the full user ID
  4. Finish inviting the user and accept the invite on Account 2
  5. Create another new room with Account 1
  6. Try the same process from step 3, except this time it should give some result - this fails on matrix.org but not with a self-hosted server. I even tried with workers and wasn't able to reproduce the issue.

I suspect this a result of the load matrix.org is under, and I'm not about to spin up a test environment to match the same scale to see if it fails in the same way.

@richvdh
Copy link
Member

richvdh commented Jul 8, 2019

The user directory was rewritten by matrix-org/synapse#4537 and friends, and I wasn't able to reproduce this on matrix.org, so I think it's fixed.

@richvdh richvdh closed this as completed Jul 8, 2019
@CybotTM
Copy link

CybotTM commented Oct 4, 2019

Still have similar problems with web and desktop client - not with new RiotX client.
Seems client is messing with server results - there are just results in the result list missing, which should be there - and correctly appear in RiotX client search results.

@t3chguy
Copy link
Member

t3chguy commented Oct 4, 2019

Please make a new issue with some usable information so this can be investigated, results from both, can be anonymized, ideally API responses from the network tab etc

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
P1 S-Critical Prevents work, causes data loss and/or has no workaround T-Defect X-Regression Z-Synapse
Projects
None yet
Development

No branches or pull requests

8 participants