From eafcb1df0d24cc2ec04b1189a176aa9b2bd72d04 Mon Sep 17 00:00:00 2001
From: Patrick Cloke <patrickc@matrix.org>
Date: Thu, 28 Jan 2021 13:44:27 -0500
Subject: [PATCH] Handle multiple notifications with an empty room.

---
 synapse/push/mailer.py   | 22 +++++++++++++++++++---
 tests/push/test_email.py | 32 ++++++++++++++++++++++++++++++--
 2 files changed, 49 insertions(+), 5 deletions(-)

diff --git a/synapse/push/mailer.py b/synapse/push/mailer.py
index e2d74bab657a..71cde30c14f4 100644
--- a/synapse/push/mailer.py
+++ b/synapse/push/mailer.py
@@ -664,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:
diff --git a/tests/push/test_email.py b/tests/push/test_email.py
index bebf9739bfd2..22f452ec248d 100644
--- a/tests/push/test_email.py
+++ b/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)
@@ -236,6 +241,26 @@ def test_empty_room(self):
         # 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(
@@ -288,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 = []