Stopped file sends failing on the throttled members route

src/application/abstractions/i_chat_sender.py

@@ -10,6 +10,9 @@ class IChatSender(ABC):
 
     SELF_CHAT: str
 
+    @abstractmethod
+    def chat_members(self, note: IGraphClient, chat_id: str) -> list[dict]: ...
+
     @abstractmethod
     def validate(self, msg: OutgoingMessage) -> OutgoingMessage: ...
 

src/application/commands/schedule_chat_meeting_command.py

@@ -37,7 +37,7 @@ async def handle(self, command: ScheduleChatMeetingCommand) -> dict:
             raise ServiceError(400, "end must be after start.")
 
         def create(note, broker):
-            members = [] if command.chat_id == self._sender.SELF_CHAT else note.list(f"chats/{command.chat_id}/members")
+            members = [] if command.chat_id == self._sender.SELF_CHAT else self._sender.chat_members(note, command.chat_id)
             me = self._profile.get(note, broker)
             mine = {(me.get("mail") or "").lower(), (me.get("userPrincipalName") or "").lower()}
             attendees = [{"emailAddress": {"address": m["email"], "name": m.get("displayName") or m["email"]}, "type": "required"} for m in members if m.get("email") and m["email"].lower() not in mine]

src/infrastructure/teams/chat_sender.py

@@ -8,9 +8,11 @@
 """
 
 import base64
+import logging
 import re
 from urllib.parse import quote
 from entrance import http_client
+from outnote import GraphError
 
 from src.application.abstractions.i_chat_sender import IChatSender
 from src.application.abstractions.i_emoji_catalogue import IEmojiCatalogue
@@ -22,6 +24,7 @@
 from src.domain.chat.upload import Upload
 from src.domain.errors.emoji_error import EmojiError
 from src.domain.errors.send_error import SendError
+from src.infrastructure.logging_config import LoggerNames
 
 
 class ChatSender(IChatSender):
@@ -40,7 +43,9 @@ class ChatSender(IChatSender):
     IMPORTANCE = ("normal", "high", "urgent")
     GUID_IN_ETAG = re.compile(r"\{([0-9A-Fa-f]{8}-[0-9A-Fa-f]{4}-[0-9A-Fa-f]{4}-[0-9A-Fa-f]{4}-[0-9A-Fa-f]{12})\}")
     USER_ID = re.compile(r"[A-Za-z0-9@._-]{1,200}")
+    EXPANDED_MEMBERS_CEILING = 20  # at this many, Graph may have cut the expanded list short
     http_client = staticmethod(http_client)
+    log = logging.getLogger(LoggerNames.SERVICE)
 
     def __init__(self, html: ChatHtmlSanitizer, emojis: IEmojiCatalogue, profile: IProfileReader):
         self._html = html
@@ -165,12 +170,25 @@ def upload_file(self, note, upload: Upload) -> dict:
             raise SendError(502, "OneDrive did not confirm the upload.")
         return item
 
+    def chat_members(self, note, chat_id: str) -> list[dict]:
+        """Graph throttles `chats/{id}/members` on its own while the same chat with its members expanded answers, so a long list is the only reason to ask the throttled route."""
+        members = note.get(f"chats/{chat_id}", {"$expand": "members"}).get("members") or []
+        if len(members) < self.EXPANDED_MEMBERS_CEILING:
+            return members
+        try:
+            return note.list(f"chats/{chat_id}/members")
+        except GraphError as ex:
+            if ex.status != 429:
+                raise
+            self.log.warning("Chat members route throttled; using the %d expanded members, which may be incomplete", len(members))
+            return members
+
     def chat_member_ids(self, note, broker, chat_id: str) -> list[str]:
         if chat_id == self.SELF_CHAT:
             return []
         me = self._profile.get(note, broker).get("id")
         ids = []
-        for m in note.list(f"chats/{chat_id}/members"):
+        for m in self.chat_members(note, chat_id):
             uid = m.get("userId")
             if isinstance(uid, str) and uid != me and uid not in ids:
                 ids.append(uid)

tests/test_teams_extras.py

@@ -29,6 +29,8 @@ def __call__(self, client_id, scope=None):
 class Note:
     def __init__(self):
         self.calls = []
+        self.members_throttled = False
+        self.expanded = None
         self.messages = []
         self.members = [{"userId": "me-id", "email": "me@school.ch"}, {"userId": "u2", "email": "bo@school.ch", "displayName": "Bo"}]
 
@@ -53,13 +55,17 @@ def batch(self, requests):
     def list(self, path, params=None, limit=None):
         self.calls.append(("list", path))
         if path.endswith("/members"):
+            if self.members_throttled:
+                raise GraphError(429, "throttled", retry_after=62)
             return self.members
         if path == "me/chats":
             return [{"id": "19:g@thread.v2", "chatType": "group"}, {"id": "19:o@unq.gbl.spaces", "chatType": "oneOnOne"}]
         return self.messages
 
     def get(self, path, params=None, raw=False):
         self.calls.append(("get", path, raw))
+        if params == {"$expand": "members"}:
+            return {"members": self.expanded if self.expanded is not None else self.members}
         if path == "me":
             return {"id": "me-id", "mail": "me@school.ch"}
         if "hostedContents" in path:
@@ -343,6 +349,39 @@ def test_meeting_invites_the_chat_members_and_returns_the_join_url(env):
     assert [a["emailAddress"]["address"] for a in event["attendees"]] == ["bo@school.ch"]
 
 
+def _send_file(note):
+    data = {"content": "hi", "contentType": "text"}
+    return TestClient(app).post("/api/chats/19:g@thread.v2/messages", headers=H, data=data, files=[("files", ("a.png", PNG, "image/png"))])
+
+
+def _invited(note):
+    return [r["objectId"] for r in next(c for c in note.calls if c[0] == "post" and c[1].endswith("/invite"))[2]["recipients"]]
+
+
+def test_a_file_send_survives_a_throttled_members_route(env):
+    note, _ = env
+    note.members_throttled = True
+    assert _send_file(note).status_code < 300
+    assert _invited(note) == ["u2"]
+    assert not any(c[:2] == ("list", "chats/19:g@thread.v2/members") for c in note.calls)
+
+
+def test_a_long_expanded_list_asks_the_members_route_for_everyone(env):
+    note, _ = env
+    note.expanded = [{"userId": f"e{i}"} for i in range(20)]
+    note.members = [{"userId": f"e{i}"} for i in range(30)]
+    assert _send_file(note).status_code < 300
+    assert len(_invited(note)) == 30
+
+
+def test_a_long_expanded_list_is_used_when_the_members_route_is_throttled(env):
+    note, _ = env
+    note.members_throttled = True
+    note.expanded = [{"userId": f"e{i}"} for i in range(20)]
+    assert _send_file(note).status_code < 300
+    assert len(_invited(note)) == 20
+
+
 def test_gif_search_needs_a_giphy_key(env, monkeypatch):
     monkeypatch.delenv("GIPHY_API_KEY", raising=False)
     r = TestClient(app).get("/api/gifs/search", headers=H, params={"q": "cat"})