Don't 500 /sync when an always-included event has no state

filter_and_transform_events_for_client annotates each event with the
requesting user's membership (MSC4115, `unsigned.membership`). It reads
that from the state after the event, and it fetches state only for
events whose in-memory copy is not flagged as an outlier. An event let
through by `always_include_ids` has no state to look in when our copy
still carries the outlier flag, and the "unreachable!" branch raised,
failing the whole `/sync` with a 500.

Two reads inside one `/sync` disagree. `_load_filtered_recents` builds
`always_include_ids` from an uncached read of `current_state_events`,
which sees an out-of-band invite or rejection the moment the persister
de-outliers it into the current state. The event object comes from the
event cache and keeps `outlier=True` until the invalidation reaches
that process. No ordering of commit and invalidation closes the gap.

MSC4115 makes `unsigned.membership` optional and lets a server omit it
when computing it is impossible, so return the event unannotated and
log the event and room at info. `FilteredEvent.membership` documents
this third case where it is None.

The call-invite assert stays. Synapse only flags state events as
outliers (out-of-band memberships, auth events, MSC4242 state DAG
events), so a non-state call invite always has state. Its comment now
gives that reason instead of claiming earlier code filters outliers.

The regression test filters a federated invite and its local rejection
for a third user with the invite in `always_include_ids`. On develop it
fails with the 500's exception. The invite setup moves into a helper
shared with test_out_of_band_invite_rejection.

Fixes #19858.

Co-authored-by: Quentin Gliech <quenting@element.io>
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

changelog.d/20439.bugfix

@@ -0,0 +1 @@
+Fix a bug where `/sync` could fail with a 500 error (`Missing state for event that is not user's own membership`) shortly after an invite or invite rejection received over federation entered a room's state.

synapse/events/utils.py

@@ -120,12 +120,14 @@ class FilteredEvent:
     """The event to be serialized."""
 
     membership: str | None
-    """The user whose requesting the event's membership at the time of the
-    event was sent.
+    """The requesting user's membership at the time the event was sent.
 
     This is None if we didn't compute the membership. In Synapse this happens a)
-    when returning state events to state endpoints, or b) when the event is
-    returned to an admin.
+    when returning state events to state endpoints, b) when the event is
+    returned to an admin, or c) when we have no state at the event, which
+    happens for an event let through by `always_include_ids` whose copy still
+    carries the outlier flag (see
+    `synapse.visibility.filter_and_transform_events_for_client`).
 
     According to the spec we don't have to include the membership for any events
     if we don't want to, especially if its expensive to compute. In practice

synapse/visibility.py

@@ -197,13 +197,10 @@ async def filter_and_transform_events_for_client(
         # Filter out call invites in public rooms, as this would potentially
         # ring a lot of users.
         if event.type == EventTypes.CallInvite and not event.is_state():
-            # `state_after_event` should only be None if the event is an outlier,
-            # and earlier code should filter out outliers entirely.
-            #
-            # In addition, we only create outliers locally for out-of-band
-            # invite rejections, invites received over federation, or state
-            # events needed to authorise other events. None of this applies to
-            # call invites.
+            # `state_after_event` is None only when our copy of the event carries
+            # the outlier flag (see below). Synapse only flags state events as
+            # outliers (out-of-band memberships, auth events and MSC4242 state DAG
+            # events), so a non-state call invite always has state here.
             assert state_after_event is not None
 
             room_join_rules = state_after_event.get((EventTypes.JoinRules, ""))
@@ -216,8 +213,9 @@ async def filter_and_transform_events_for_client(
         # Annotate the event with the user's membership after the event.
         #
         # Normally we just look in `state_after_event`, but if the event is an outlier
-        # we won't have such a state. The only outliers that are returned here are the
-        # user's own membership event, so we can just inspect that.
+        # we won't have such a state. An outlier here is either the user's own
+        # membership event, which we inspect directly, or an event in
+        # `always_include_ids`, which we return without a membership.
 
         user_membership_event: EventBase | None
         if event.type == EventTypes.Member and event.state_key == user_id:
@@ -225,8 +223,23 @@ async def filter_and_transform_events_for_client(
         elif state_after_event is not None:
             user_membership_event = state_after_event.get((EventTypes.Member, user_id))
         else:
-            # unreachable!
-            raise Exception("Missing state for event that is not user's own membership")
+            # We have no state for this event: our copy carries the outlier flag,
+            # so the state fetch above skipped it, but `always_include_ids` let it
+            # through anyway. Sync builds `always_include_ids` from an uncached read
+            # of `current_state_events`, so it sees an out-of-band membership event
+            # that has been de-outliered into the current state as soon as the
+            # persister commits, while the event object we were handed still comes
+            # from a cache that has not been invalidated yet. We don't know the
+            # user's membership at this point in the DAG; the spec makes
+            # `unsigned.membership` optional, so return the event unannotated rather
+            # than dropping it or failing the whole request.
+            logger.info(
+                "filter_and_transform_events_for_client: no state for always-included"
+                " event %s in room %s; returning it without a membership annotation",
+                event.event_id,
+                event.room_id,
+            )
+            return FilteredEvent(event=filtered, membership=None)
 
         user_membership = (
             user_membership_event.membership

tests/test_visibility.py

@@ -589,9 +589,12 @@ class FilterEventsForClientTestCase(HomeserverTestCase):
 class FilterEventsOutOfBandEventsForClientTestCase(
     unittest.FederatingHomeserverTestCase
 ):
-    def test_out_of_band_invite_rejection(self) -> None:
-        # this is where we have received an invite event over federation, and then
-        # rejected it.
+    def _receive_and_reject_invite(self) -> tuple[EventBase, EventBase]:
+        """Receive an invite over federation and reject it locally.
+
+        Returns the invite event and the rejection event, both of which are
+        out-of-band membership events.
+        """
         invite_pdu = {
             "room_id": "!room:id",
             "depth": 1,
@@ -637,6 +640,10 @@ class FilterEventsOutOfBandEventsForClientTestCase(
                 [invite_event_id, reject_event_id]
             )
         )
+        return invite_event, reject_event
+
+    def test_out_of_band_invite_rejection(self) -> None:
+        invite_event, reject_event = self._receive_and_reject_invite()
 
         # the invited user should be able to see both the invite and the rejection
         filtered_events = self.get_success(
@@ -667,6 +674,28 @@ class FilterEventsOutOfBandEventsForClientTestCase(
             [],
         )
 
+    def test_always_included_event_without_state_has_no_membership(self) -> None:
+        """An event let through by `always_include_ids` that has no state is
+        returned without a membership annotation rather than failing the request.
+
+        Regression test for https://github.com/element-hq/synapse/issues/19858.
+        """
+        invite_event, reject_event = self._receive_and_reject_invite()
+
+        filtered_events = self.get_success(
+            filter_and_transform_events_for_client(
+                self.hs.get_storage_controllers(),
+                "@other:test",
+                [invite_event, reject_event],
+                always_include_ids=frozenset({invite_event.event_id}),
+            )
+        )
+        self.assertEqual(
+            [e.event.event_id for e in filtered_events],
+            [invite_event.event_id],
+        )
+        self.assertEqual([e.membership for e in filtered_events], [None])
+
 
 async def inject_visibility_event(
     hs: HomeServer,