Просмотр исходного кода

Fix presence_changed reading a user_id the realtime event no longer carries

The realtime PresenceChangedEvent carries only a status field — the user
whose presence moved is the envelope's actor_id. The adapter read
event.payload.user_id, raised AttributeError on every presence change,
and the event loop answered that with a reconnect backoff of up to 30s.
Messages that arrived inside the reconnect gap never reached the agent:
a /restart sent at 13:49 was lost exactly that way, ten reconnects over
the day, one of them swallowing the very command that was meant to load
the fix's prerequisite.

The user is now read from event.actor_id, mirroring chattolib's own
bot.py dispatch. _make_presence_event mocks the payload with a spec of
['status'] so the wire shape is the test's contract: a regression to
payload.user_id fails the roster tests instead of passing by
MagicMock's answer-anything attribute access.
Liv Klumpp 1 неделя назад
Родитель
Сommit
198b7d279a
2 измененных файлов с 47 добавлено и 5 удалено
  1. 12 2
      adapter.py
  2. 35 3
      test_adapter.py

+ 12 - 2
adapter.py

@@ -2224,11 +2224,21 @@ class ChattoAdapter(BasePlatformAdapter):
 
         elif event.kind == "presence_changed":
             # A participant's presence changed — patch the cached user in place.
-            if (user := self._user_cache.get(event.payload.user_id)) is not None:
+            # The identified user is the event's own actor: the realtime
+            # PresenceChangedEvent carries only a status field, and a
+            # payload.user_id read here raised AttributeError on every
+            # presence change, which the event loop mistook for a broken
+            # stream and answered with a reconnect backoff — dropping
+            # inbound messages that arrived inside the gap.
+            changed_user_id = event.actor_id
+            if (
+                changed_user_id
+                and (user := self._user_cache.get(changed_user_id)) is not None
+            ):
                 user.presence_status = event.payload.status
                 logger.debug(
                     "Chatto: patched presence of %s to %s",
-                    event.payload.user_id,
+                    changed_user_id,
                     event.payload.status.name,
                 )
 

+ 35 - 3
test_adapter.py

@@ -1170,6 +1170,33 @@ class TestReactionForwarding:
         adapter.set_reaction_handler(AsyncMock(side_effect=RuntimeError("hook boom")))
         await adapter._handle_realtime_event(self._event("reaction_added"))
 
+    async def test_presence_event_without_a_payload_user_id_does_not_raise(
+        self, adapter
+    ):
+        """The realtime PresenceChangedEvent carries only a status field — the
+        user is the event's own actor. A payload.user_id read used to raise
+        AttributeError on every presence change, and the event loop answered
+        that with a reconnect backoff that dropped messages arriving inside
+        the gap (a /restart was lost exactly that way)."""
+        event = self._event("presence_changed", actor_id="human-1")
+        # A real PresenceChangedEvent payload: no user_id attribute at all.
+        event.payload = MagicMock(spec=["status"])
+        event.payload.status = PresenceStatus.OFFLINE
+        adapter._user_cache["human-1"] = _make_user("human-1", "human")
+
+        await adapter._handle_realtime_event(event)
+
+        assert adapter._user_cache["human-1"].presence_status is PresenceStatus.OFFLINE
+
+    async def test_presence_of_an_uncached_user_is_skipped(self, adapter):
+        """Presence of someone the adapter has not cached is not an error —
+        there is nothing to patch, and the event is not interesting either."""
+        event = self._event("presence_changed", actor_id="stranger-9")
+        event.payload = MagicMock(spec=["status"])
+        event.payload.status = PresenceStatus.ONLINE
+
+        await adapter._handle_realtime_event(event)
+
 
 # -- chat_type mapping --
 
@@ -2322,13 +2349,18 @@ def _make_edited_event(room_id="room-1", message_event_id="msg-1", actor_id="hum
 def _make_presence_event(
     user_id="user-1", actor_id=None, status=PresenceStatus.OFFLINE
 ):
-    """A presence_changed envelope — actor is the user whose status moved."""
+    """A presence_changed envelope — actor is the user whose status moved.
+
+    The payload mirrors the wire shape (PresenceChangedEvent carries ONLY a
+    status field), so a spec'd payload here fails any regression to reading
+    payload.user_id — the bug that killed the realtime stream on every
+    presence change.
+    """
     event = MagicMock()
     event.id = f"evt-presence-{user_id}"
     event.kind = "presence_changed"
     event.actor_id = actor_id if actor_id is not None else user_id
-    payload = MagicMock()
-    payload.user_id = user_id
+    payload = MagicMock(spec=["status"])
     payload.status = status
     event.payload = payload
     return event