Sliding Sync: bugfix: ensure we can sync with SSS even with missing rooms

[kegsay]

Fixes element-hq/element-x-ios#3300

Some rooms are missing from sliding_sync_joined_rooms. When this happens, the first call will succeed, but any subsequent calls for this room ID will cause the cache to return None for the room ID, rather than not having the key at all. This then causes the <= check to throw.

Root cause: #17726

Pull Request Checklist

• Pull request is based on the develop branch
• Pull request includes a changelog file. The entry should:
  • Be a short description of your change which makes sense to users. "Fixed a bug that prevented receiving messages from other servers." instead of "Moved X method from EventStore to EventWorkerStore.".
  • Use markdown where necessary, mostly for code blocks.
  • End with either a period (.) or an exclamation mark (!).
  • Start with a capital letter.
  • Feel free to credit yourself, by adding a sentence "Contributed by @github_username." or "Contributed by [Your Name]." to the end of the entry.
• Code style is correct
  (run the linters)

[devonh]

Is continuing the right thing to do here?
Or should this fall into the else case below for further processing?

[kegsay]

I'm following the same semantics as if the room was not in uncapped_results at all, which seems to just ignore the room entirely.

[devonh]

Yeah that makes sense to me. (This is my first time reading through all this code & it seems to jump around / duplicate itself in places which was a little confusing)

[erikjohnston]

The caller of this function seems to expect an entry for each room ID?

synapse/synapse/handlers/sliding_sync/room_lists.py

Lines 1909 to 1928 in af998e6

	joined_room_positions = (
	await self.store.bulk_get_last_event_pos_in_room_before_stream_ordering(
	[
	room_id
	for room_id, room_for_user in sync_room_map.items()
	if room_for_user.membership == Membership.JOIN
	],
	to_token.room_key,
	)
	)
	last_activity_in_room_map.update(joined_room_positions)
	return sorted(
	sync_room_map.values(),
	# Sort by the last activity (stream_ordering) in the room
	key=lambda room_info: last_activity_in_room_map[room_info.room_id],
	# We want descending order
	reverse=True,
	)

It looks to me that things have gotten a bit confused here around what to do if a bump stamp can't be found, as we are probably assuming we can always find a bump stamp?

I guess the right thing to do is actually handle missing bump stamps at the call site (on top of this change)

[erikjohnston]

Looks like this was due to room with an unknown room version.

Have fixed and added regression test in #17733

[devonh]

Yikes. We'll need to look much closer at #17726.
But this should alleviate the immediate issue.