It turns out that the failure came when using the known room path in the
room preview: Alice knows about the room, but for some reason the client
didn't retrieve all the state events from the sliding sync proxy yet.
Before, the sync would be considered stable after 2 seconds. This is too
little, considering that events come from the proxy that listens to
events from synapse. Raising this threshold to 15 seconds should help
getting all the room information from the proxy, and thus get all the
information we expected in the client.
While this mutex is taken, in `oldest_token()` we can get a hold of the
RoomEvent mutex, making it so that the `waited_...` token covers the
critical region of room events.
Unfortunately, in `clear()`, we're taking these two locks in the
opposite order, implying that the room events critical region now
overlaps that of the `waited_...` lock.
The way to break the cycle is to keep the `waited_` token for as short
as possible.
So as to not use sync mutexes across await points, we have to use an
async mutex, BUT it can't be immediately called in a wiremock responder,
so we need to shoe-horn a bit: create a new tokio runtime, which can
only be called from another running thread, etc. It's a bit ugly, but I
couldn't find another mechanism to block the responder from returning a
Response immediately; happy to change that anytime if there's a simpler
way.
A client would require this to know that the sending queue has been
globally disabled, across all the rooms, otherwise they couldn't know
that it needs to be re-enabled later on.
This implements the following features:
- enabling/disabling the sending queue at a client-wide granularity,
- one sending queue per room, that lives until the client is shutting
down,
- retrying sending an event three times, if it failed in the past, with
some exponential backoff to attempt to re-send,
- allowing to cancel sending an event we haven't sent yet.
fixup sdk
This can be equivalently retrieved using:
`get_timeline_event_by_event_id(event_id).content().as_message()?.content()`
As per the commit date, this is used in one place in both EXA and EXI,
so it seems fine to change the caller's call sites.
And this avoids one explicit call site of
`Timeline::item_by_event_id()`.
If an event cannot be decrypted after a grace period, it is reported to the application (and thence Posthog) as a UTD event. Currently, if it is then successfully decrypted, the event is then re-reported as a "late decryption".
This does not match the expected behaviour in Posthog - an event is *either* a UTD, or a late decryption; it makes no sense for it to be both. This PR fixes the problem.
I've attempted to make the commits sensible, but I'm not entirely sure I've succeeded. The tests do pass after each commit, though. The interesting change itself is somewhere in the middle; there is non-functional groundwork before and cleanup afterwards.
---
* ui: Factor out UTD report code to a closure
For now, this doesn't help much, but in future there will be more logic here,
and it helps reduce the repetition between the delay and no-delay cases.
* ui: Convert UtdHookManager::pending_delayed to a HashMap
* ui: Store decryption time in `UtdHookManager::pending_delayed`
This is a step on getting rid of `known_utds`
* ui: Fix re-reporting of late decryptions
This fixes the problem where a message that was previously reported as a UTD,
and was then subsequently successfully decrypted, is then re-reported as a late
decryption. This artificially inflated the UTD metrics.
We do this by checking the `pending_delayed` list in `on_late_decrypt`, instead
of the `known_utds` list. There is some associated reordering of code to get
the locking right.
* ui: Remove unused "utd report time" from `UtdHookManager::known_utds`
* ui: Replace `UtdHookManager::known_utds` with `reported_utds`
Keep a list of the UTDs we've actually reported, rather than the union of those
we've reported together with those we might report in a while.
I find this much easier to reason about.
* Address minor review comments
* Reinstate assertion in UTD hook tests
* Reinstate `known_utds`
To differentiate the SAS state between the party
that sent the verification start and the party that received it.
Signed-off-by: Kévin Commaille <zecakeh@tedomum.fr>
This patch adds a new argument to the `until` argument closure of
`RoomPagination::run_backwards`: `timeline_has_been_reset`, which
designates when the timeline has been reset.
A test has been updated accordingly.
* chore(sdk): Rename `RoomEventCacheUpdate::UpdateReadMarker`.
This patch renames `RoomEventCacheUpdate::UpdateReadMarker` to
`ReadMarker`. The `Update` prefix is already part of the enum name.
* feat(sdk): Rename `RoomEventCacheUpdate::ReadMarker::event_id` to `move_to`.
This patch renames `RoomEventCacheUpdate::ReadMarker::event_id` to
`ReadMarker::move_to` as I feel like it conveys a better semantics.
* feat(sdk): Extract `RoomEventCacheUpdate::Append::ambiguity_changes`.
This patch extracts `RoomEventCacheUpdate::Append::ambiguity_changes`
into a new variant `RoomEventCacheUpdate::Members { ambiguity_changes }
`.
This patch also creates a new private
`RoomEventCacheInner::send_grouped_updates_for_events` method to ensure
the updates are sent in a particular order.
* feat(sdk): Rename `RoomEventCacheUpdate::Append`.
This patch renames `RoomEventCacheUpdate::Append` to `SyncEvents`. The
field `events` is renamed `timeline`.
This patch also renames some variables to clarify the code and to match
the renamings in `RoomEventCacheUpdate`.
* feat(sdk): Rename `RoomEventCacheUpdate::ReadMarker` and `Members`.
This patch renames `ReadMarker { move_to: … }` to `MoveReaderMarkerTo
{ event_id: … }`.
This patch also renames `Members` to `UpdateMembers`.
Finally, this patch renames some other variables to avoid clashes with
terminology in `matrix_sdk_ui`.
* feat(sdk): Split `RoomEventCacheUpdate::SyncEvents`.
This patch splits `RoomEventCacheUpdate::SyncEvents` into 2 new variants:
`AddTimelineEvents` and `AddEphemeralEvents`.
This patch takes this opportunity to update `matrix_sdk_ui::timeline`
a little bit too. `handle_sync_events` is renamed
`handle_ephemeral_events`, and the `SyncTimelineEvent` argument is
removed: it's possible to use `add_events_at` directly to handle the
`SyncTimelineEvent`s.
* fix(sdk): Do not send `RoomEventCacheUpdate` if values are empty.
This patch prevents sending useless `RoomEventCacheUdpate` if their
respective values are empty.
* chore(ui): Update a log message.
* Apply suggestions from code review
Signed-off-by: Benjamin Bouvier <public@benj.me>
---------
Signed-off-by: Benjamin Bouvier <public@benj.me>
Co-authored-by: Benjamin Bouvier <public@benj.me>
This patch adds a new argument to `RoomPagination::run_backwards`:
`until`. It becomes:
pub async fn run_backwards<F, B, Fut>(&self,
batch_size: u16,
mut until: F
) -> Result<B>
where
F: FnMut(BackPaginationOutcome) -> Fut,
Fut: Future<Output = ControlFlow<B, ()>>,
The idea behind `until` is to run pagination _until_ `until` returns
`ControlFlow::Break`, otherwise it continues paginating.
This is useful is many scenearii (cf. the documentation). This is
also and primarily the first step to stop adding events directly from
the pagination, and starts adding events only and strictly only from
`event_cache::RoomEventCacheUpdate` (again, see the `TODO` in the
documentation). This is not done in this patch for the sake of ease
of review.
