Handle empty rooms when generating email notifications.

changelog.d/9257.bugfix

@@ -0,0 +1 @@
+Fix long-standing KeyError in synapse.push.mailer in get_message_vars.

synapse/push/mailer.py

@@ -443,10 +443,15 @@ async def get_message_vars(
         if event.type != EventTypes.Message and event.type != EventTypes.Encrypted:
             return None
 
-        sender_state_event_id = room_state_ids[("m.room.member", event.sender)]
-        sender_state_event = await self.store.get_event(sender_state_event_id)
-        sender_name = name_from_member_event(sender_state_event)
-        sender_avatar_url = sender_state_event.content.get("avatar_url")
+        sender_state_event_id = room_state_ids.get(("m.room.member", event.sender))
+        if sender_state_event_id:
+            sender_state_event = await self.store.get_event(sender_state_event_id)
+            sender_name = name_from_member_event(sender_state_event)
+            sender_avatar_url = sender_state_event.content.get("avatar_url")
+        else:
+            # The sender is no longer in the room for some reason.
+            sender_name = event.sender
+            sender_avatar_url = None
 
         # 'hash' for deterministically picking default images: use
         # sender_hash % the number of default images to choose from
@@ -659,9 +664,25 @@ async def make_summary_text_from_member_events(
         # are from explicitly to avoid, "messages in the Bob room"
         sender_ids = {notif_events[n["event_id"]].sender for n in notifs}
 
-        member_events = await self.store.get_events(
-            [room_state_ids[("m.room.member", s)] for s in sender_ids]
-        )
+        # Attempt to get some names of the senders.
+        member_event_ids = [
+            room_state_ids[("m.room.member", s)]
+            for s in sender_ids
+            if ("m.room.member", s) in room_state_ids
+        ]
+
+        if not member_event_ids:
+            # No member events were found! Maybe the room is empty?
+            # Fallback to the room ID (note that if there was a room name this
+            # would already have been used previously).
+            return self.email_subjects.messages_in_room % {
+                "room": room_id,
+                "app": self.app_name,
+            }
+
+        # Get the actual member events (in order to calculate a pretty name for
+        # the room).
+        member_events = await self.store.get_events(member_event_ids)
 
         # There was a single sender.
         if len(sender_ids) == 1:

tests/push/test_email.py

@@ -124,13 +124,18 @@ def test_simple_sends_email(self):
         )
         self.helper.join(room=room, user=self.others[0].id, tok=self.others[0].token)
 
-        # The other user sends some messages
+        # The other user sends a single message.
         self.helper.send(room, body="Hi!", tok=self.others[0].token)
-        self.helper.send(room, body="There!", tok=self.others[0].token)
 
         # We should get emailed about that message
         self._check_for_mail()
 
+        # The other user sends multiple messages.
+        self.helper.send(room, body="Hi!", tok=self.others[0].token)
+        self.helper.send(room, body="There!", tok=self.others[0].token)
+
+        self._check_for_mail()
+
     def test_invite_sends_email(self):
         # Create a room and invite the user to it
         room = self.helper.create_room_as(self.others[0].id, tok=self.others[0].token)
@@ -217,6 +222,45 @@ def test_multiple_rooms(self):
         # We should get emailed about those messages
         self._check_for_mail()
 
+    def test_empty_room(self):
+        """All users leaving a room shouldn't cause the pusher to break."""
+        # Create a simple room with two users
+        room = self.helper.create_room_as(self.user_id, tok=self.access_token)
+        self.helper.invite(
+            room=room, src=self.user_id, tok=self.access_token, targ=self.others[0].id
+        )
+        self.helper.join(room=room, user=self.others[0].id, tok=self.others[0].token)
+
+        # The other user sends a single message.
+        self.helper.send(room, body="Hi!", tok=self.others[0].token)
+
+        # Leave the room before the message is processed.
+        self.helper.leave(room, self.user_id, tok=self.access_token)
+        self.helper.leave(room, self.others[0].id, tok=self.others[0].token)
+
+        # We should get emailed about that message
+        self._check_for_mail()
+
+    def test_empty_room_multiple_messages(self):
+        """All users leaving a room shouldn't cause the pusher to break."""
+        # Create a simple room with two users
+        room = self.helper.create_room_as(self.user_id, tok=self.access_token)
+        self.helper.invite(
+            room=room, src=self.user_id, tok=self.access_token, targ=self.others[0].id
+        )
+        self.helper.join(room=room, user=self.others[0].id, tok=self.others[0].token)
+
+        # The other user sends a single message.
+        self.helper.send(room, body="Hi!", tok=self.others[0].token)
+        self.helper.send(room, body="There!", tok=self.others[0].token)
+
+        # Leave the room before the message is processed.
+        self.helper.leave(room, self.user_id, tok=self.access_token)
+        self.helper.leave(room, self.others[0].id, tok=self.others[0].token)
+
+        # We should get emailed about that message
+        self._check_for_mail()
+
     def test_encrypted_message(self):
         room = self.helper.create_room_as(self.user_id, tok=self.access_token)
         self.helper.invite(
@@ -269,3 +313,6 @@ def _check_for_mail(self):
         pushers = list(pushers)
         self.assertEqual(len(pushers), 1)
         self.assertTrue(pushers[0].last_stream_ordering > last_stream_ordering)
+
+        # Reset the attempts.
+        self.email_attempts = []