Skip to content
This repository has been archived by the owner on Apr 26, 2024. It is now read-only.

system alerts keeps reinviting mozilla users despite being empty #6815

Closed
ara4n opened this issue Jan 30, 2020 · 6 comments
Closed

system alerts keeps reinviting mozilla users despite being empty #6815

ara4n opened this issue Jan 30, 2020 · 6 comments
Assignees
Labels
z-bug (Deprecated Label)

Comments

@ara4n
Copy link
Member

ara4n commented Jan 30, 2020

if mozilla users part the system alerts room, they keep getting reinvited to it (perhaps every time the server restarts)? despite the room being empty

@babolivier
Copy link
Contributor

Are we sure it's inviting these users to the same room they left?

From looking at the code, what's the most likely to be happening is that something triggers a server notice to be sent every now and then. In this case, and if a user has left the previous room, Synapse will create a new room and invite the user to it. FWIW the sending of a notice can be triggered either if the user hasn't accepted the latest ToS, if the server's MAU is reached, or if someone sent a notice via the admin API.

I created an account on the homeserver to investigate further when that happens next.

@babolivier babolivier self-assigned this Jan 30, 2020
@babolivier
Copy link
Contributor

Hmm, I just witnessed it happen with my account, and the System Alerts user isn't sending any message to the new room (which it shouldn't do). I'll investigate further.

@neilisfragile neilisfragile added p1 z-bug (Deprecated Label) labels Feb 10, 2020
@babolivier
Copy link
Contributor

Quick update on this issue, I haven't seen it happen on my account over the past month or so. I'll wait until the end of this month, and if it still doesn't happen, I'll close this issue (unless I'm told people are still getting bitten by this, which would mean that I'm just not trying to repro it the right way).

@babolivier
Copy link
Contributor

babolivier commented Mar 31, 2020

This happened to me on the 23rd (Monday last week) on vector.modular.im.

After digging through the logs, the database and the code, I believe that the issue is that maybe_send_server_notice_to_user, which checks whether a user should be notified about MAU, creates a room and invites the user to it before it knows whether it should notify the user.

The fix should be fairly simple (i.e. introduce a flag to get_notice_room_for_user that make it skip the invite part), but first I want to make sure this is not a symptom of something else, since afaict this bug should happen every time a user syncs.

@babolivier
Copy link
Contributor

Right, turns out that get_notice_room_for_user is cached (with a @cachedInlineCallbacks() decorator), which never expires except when the homeserver restarts (which clears all of the caches), which explains why it isn't run on every sync (but, presumably, the first one since the latest restart).

babolivier added a commit that referenced this issue Apr 4, 2020
Fixes #6815

Before figuring out whether we should alert a user on MAU, we call get_notice_room_for_user to get some info on the existing server notices room for this user. This function, if the room doesn't exist, creates it and invites the user in it. This means that, if we decide later that no server notice is needed, the user gets invited in a room with no message in it. This happens at every restart of the server, since the room ID returned by get_notice_room_for_user is cached.

This PR fixes that by moving the inviting bit to a dedicated function, that's only called when the server actually needs to send a notice to the user. A potential issue with this approach is that the room that's created by get_notice_room_for_user doesn't match how that same function looks for an existing room (i.e. it creates a room that doesn't have an invite or a join for the current user in it, so it could lead to a new room being created each time a user syncs), but I'm not sure this is a problem given it's cached until the server restarts, so that function won't run very often.

It also renames get_notice_room_for_user into get_or_create_notice_room_for_user to make what it does clearer.
@babolivier
Copy link
Contributor

Fixed in #7199

phil-flex pushed a commit to phil-flex/synapse that referenced this issue Jun 16, 2020
…rg#7199)

Fixes matrix-org#6815

Before figuring out whether we should alert a user on MAU, we call get_notice_room_for_user to get some info on the existing server notices room for this user. This function, if the room doesn't exist, creates it and invites the user in it. This means that, if we decide later that no server notice is needed, the user gets invited in a room with no message in it. This happens at every restart of the server, since the room ID returned by get_notice_room_for_user is cached.

This PR fixes that by moving the inviting bit to a dedicated function, that's only called when the server actually needs to send a notice to the user. A potential issue with this approach is that the room that's created by get_notice_room_for_user doesn't match how that same function looks for an existing room (i.e. it creates a room that doesn't have an invite or a join for the current user in it, so it could lead to a new room being created each time a user syncs), but I'm not sure this is a problem given it's cached until the server restarts, so that function won't run very often.

It also renames get_notice_room_for_user into get_or_create_notice_room_for_user to make what it does clearer.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
z-bug (Deprecated Label)
Projects
None yet
Development

No branches or pull requests

3 participants