Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
ara4n wants to merge 2 commits into
base: develop
Choose a base branch
from
+54 −17
Open

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
ed885f5
Don't 500 /sync when an always-included event has no state
ara4n Jul 14, 2026
c53b36c
changelog
ara4n Jul 14, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions changelog.d/19960.bugfix
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Fix a bug where `/sync` could return a 500 error ("Missing state for event that is not user's own membership") when an out-of-band membership event, since de-outliered into the room's current state, appeared in a room timeline.

@MadLittleMods MadLittleMods Jul 14, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is listed as fixing #19858 which tracks a flake I specifically saw with TestFederationRoomsInvite/Parallel/Remote_invited_user_can_reject_invite_when_homeserver_is_already_participating_in_the_room

From a message you sent in the internal backend room, it looks like you saw this flake with TestFederationRoomsInvite/Parallel/Invited_user_can_reject_invite_over_federation_several_times (different test)

It makes sense that this would probably fix up both as they both involve out-of-band membership (invites over federation)

@MadLittleMods MadLittleMods Jul 14, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
Fix a bug where `/sync` could return a 500 error ("Missing state for event that is not user's own membership") when an out-of-band membership event, since de-outliered into the room's current state, appeared in a room timeline.
Fix a bug where `/sync` could return a 500 error (`Missing state for event that is not user's own membership`) when an out-of-band membership event, since de-outliered into the room's current state, appeared in a room timeline.

@MadLittleMods MadLittleMods Jul 14, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(we are also in the room and received it over federation) and enters the current state (sync's always_include_ids), while the copy being filtered (e.g. from a worker's not-yet-invalidated cache) still carries the outlier flag.

Would a better fix be to fix the cache problem?

45 changes: 28 additions & 17 deletions synapse/visibility.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -196,21 +196,17 @@ def allowed(event: EventBase) -> FilteredEvent | None:
# 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.
assert state_after_event is not None

@ara4n ara4n Jul 14, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I checked why fable downgraded the assert into a condition, and:

⏺ Because it's the same landmine one paragraph up from the one we just stepped on — an assert that encodes the belief "state is always present here", where that belief is broken by the same always_include_ids bypass.

The assert's own justification comment says "state_after_event should only be None if the event is an outlier, and earlier code should filter out outliers entirely". That's exactly the invariant #19858 disproved: _check_client_allowed_to_see_event returns outlier-flagged events without state whenever they're in always_include_ids. If a call invite ever arrives at that line with state_after_event=None, the AssertionError 500s the whole request — identical failure mode to the raise we're fixing, just a different event type.

Can it actually happen? Not via the path that bit us — sync's current_state_ids only contains state events, and this branch explicitly checks not event.is_state(). But it's reachable in principle via the other always_include_ids callers: the sticky-events paths (sync.py and sliding_sync/extensions.py) whitelist arbitrary message-like event ids, and sticky events interact with de-outliering too (they're explicitly reconsidered on de-outlier). A sticky m.call.invite with a stale outlier flag would hit the assert.

As for what the guard does when state is missing: the check exists to suppress call invites in public rooms (so they don't ring everyone). With no state we can't read the join rule, so the choices are "deliver anyway" or "suppress on uncertainty". I chose deliver, because an event in always_include_ids was deliberately whitelisted by the caller — silently dropping a whitelisted event on a cache race seemed worse than the rare risk of ringing in a public room.


room_join_rules = state_after_event.get((EventTypes.JoinRules, ""))
if (
room_join_rules is not None
and room_join_rules.content.get("join_rule") == JoinRules.PUBLIC
):
return None
# `state_after_event` should only be None if the event is an outlier
# (or our copy of the event is stale and still flagged as one, see
# below); in either case we can't check the join rules, so let the
# event through.
if state_after_event is not None:
room_join_rules = state_after_event.get((EventTypes.JoinRules, ""))
if (
room_join_rules is not None
and room_join_rules.content.get("join_rule") == JoinRules.PUBLIC
):
return None

# Annotate the event with the user's membership after the event.
#
Expand All @@ -224,8 +220,23 @@ def allowed(event: EventBase) -> FilteredEvent | None:
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: it was flagged as an outlier
# when we fetched the state above, but `always_include_ids` let it
# through anyway. This can happen when an out-of-band membership
# event (e.g. an invite, or a rejection of one, received while not
# in the room) is later de-outliered — because we are also in the
# room and received it over federation — and enters the current
# state, while the copy of the event we are working with (e.g. from
# a worker's not-yet-invalidated cache) still carries the outlier
# flag. We don't know the user's membership at this point in the
# DAG, which MSC4115 permits — return the event unannotated rather
# than failing the whole request.
logger.info(
"filter_and_transform_events_for_client: no state for always-included"
" outlier %s; returning it without a membership annotation",
event.event_id,
)
return FilteredEvent(event=filtered, membership=None)

user_membership = (
user_membership_event.membership
Expand Down
25 changes: 25 additions & 0 deletions tests/test_visibility.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -665,6 +665,31 @@ def test_out_of_band_invite_rejection(self) -> None:
[],
)

# ... unless the events are in `always_include_ids`. This mirrors what
# happens when an out-of-band membership event is later de-outliered
# (because we are also in the room and received it over federation) and
# enters the current state — which sync passes as `always_include_ids`
# — while the copy of the event being filtered still carries the
# outlier flag. We can't compute the requesting user's membership at
# such an event (we have no state for it), but it must be let through
# without a membership annotation rather than failing the whole
# request.
#
# Regression test for https://github.com/element-hq/synapse/issues/19858.
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_id}),
)
)
self.assertEqual(
[e.event.event_id for e in filtered_events],
[invite_event_id],
)
self.assertEqual([e.membership for e in filtered_events], [None])


async def inject_visibility_event(
hs: HomeServer,
Expand Down
Loading