SDK: Cache the RTC foci from the well-known file. #5167

pixlwave · 2025-06-04T13:29:26Z

This PR makes the following changes:

Refactor ServerCapabilities into ServerInfo
- It had nothing to do with /capabilities and was confusing.
Include the /.well-known/matrix/client response in the ServerInfo cache.
Add Client::rtc_foci which uses the cached well-known.
Add Client::is_livekit_rtc_supported to the FFI using the cache RTC foci.

Requires ruma/ruma#2091 before merging

Cargo.toml

codecov · 2025-06-04T14:26:28Z

Codecov Report

Attention: Patch coverage is 95.92593% with 11 lines in your changes missing coverage. Please review.

Project coverage is 90.18%. Comparing base (47c8df0) to head (e69b33f).
Report is 16 commits behind head on main.

✅ All tests successful. No failed tests found.

Files with missing lines	Patch %	Lines
crates/matrix-sdk/src/client/mod.rs	96.31%	3 Missing and 4 partials ⚠️
...tes/matrix-sdk-base/src/store/integration_tests.rs	95.23%	0 Missing and 1 partial ⚠️
crates/matrix-sdk-base/src/store/traits.rs	96.55%	0 Missing and 1 partial ⚠️
crates/matrix-sdk-sqlite/src/state_store.rs	75.00%	0 Missing and 1 partial ⚠️
...matrix-sdk/src/client/builder/homeserver_config.rs	94.73%	0 Missing and 1 partial ⚠️

Additional details and impacted files

@@           Coverage Diff            @@
##             main    #5167    +/-   ##
========================================
  Coverage   90.18%   90.18%            
========================================
  Files         334      334            
  Lines      104681   104845   +164     
  Branches   104681   104845   +164     
========================================
+ Hits        94402    94551   +149     
- Misses       6226     6239    +13     
- Partials     4053     4055     +2

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

bmarty · 2025-06-05T12:28:12Z

I have got a crash testing this branch on Android:

org.matrix.rustcomponents.sdk.InternalException: called Option::unwrap() on a None value

pixlwave · 2025-06-05T13:12:32Z

org.matrix.rustcomponents.sdk.InternalException: called Option::unwrap() on a None value

Have you got a stack trace for this?

bmarty · 2025-06-05T13:36:35Z

I have not a lot. Just this:

log: thread '' panicked at 'called Option::unwrap() on a None value': /Users/bmarty/workspaces/matrix-rust-sdk/crates/matrix-sdk/src/client/mod.rs:1869

pixlwave · 2025-06-05T13:48:16Z

Hmmm, well that line is unchanged in the PR and I'm not massively sure how this works.

matrix-rust-sdk/crates/matrix-sdk/src/client/mod.rs

Lines 1868 to 1869 in e0f18e7

    
           // SAFETY: both fields were set above, so the function will always return some. 
        
           Ok(f(&guard).unwrap())

I set the new field I just added, maybe we can get some clarity in the review 🤔

bnjbvr · 2025-06-05T13:55:16Z

Hmmm, well that line is unchanged in the PR and I'm not massively sure how this works.

matrix-rust-sdk/crates/matrix-sdk/src/client/mod.rs

Lines 1868 to 1869 in e0f18e7

// SAFETY: both fields were set above, so the function will always return some.

Ok(f(&guard).unwrap())

I set the new field I just added, maybe we can get some clarity in the review 🤔

Let's break out the statement: f(&guard).unwrap():

&guard gives you information that's supposed to be accurate now
f(&guard) will call the closure parameter on it, that returns an option. In the new case you've added, it's this function:

server_info
                .well_known
                .as_ref()
                .and_then(|well_known| well_known.rtc_foci.clone().into())

if this callback function returns None, then this .unwrap() will fail.

I suppose that if the .well-known is None (or if rtc_foci can be None?), then this will return None, and cause the unwrap to fail.

pixlwave · 2025-06-05T14:05:00Z

Ohhhh, I thought this line was about re-locking access to the inner.caches 😂🤦‍♂️

I see it clear as day now. Will rename some stuff.

Thanks Benji 🙏

pixlwave · 2025-06-05T16:02:37Z

Ok, I've fixed it in 5476b5c with a double optional but then I decided that the assumption to unwrap the result of that closure is very fragile, so I added a CachedValue wrapper to make it compile safe in 80435fe.

I'm less sure about how idiomatic the latter commit is - in Swift I would use a KeyPath parameter instead of the problematic closure, but I couldn't find anything like that for Rust. Hence I kept it separate and am happy to revert it or squash it based on feedback :)

bmarty · 2025-06-05T18:46:48Z

With the latest commits, it's not crashing anymore, but isLivekitRtcSupported() is returning false when testing on a matrix.org account. I believe that the file https://matrix.org/.well-known/matrix/client has the expected field that should make the method return true. Am I missing something?

pixlwave · 2025-06-06T10:11:47Z

Ah, this is presumably because the old data is still cached without a well-known. I've just seen the same thing. Does it work if you call Client.resetServerInfo() once?

bmarty · 2025-06-06T13:13:33Z

Does it work if you call Client.resetServerInfo() once?

It did not fix the issue, but I'll build the SDK again with the latest commits

bmarty · 2025-06-06T13:49:13Z

Tested with d7f2550 and calling resetServerInfo() before isLivekitRtcSupported() and isLivekitRtcSupported() still returns false for a matrix.org account.

pixlwave · 2025-06-06T13:57:22Z

Huh, it's working great for me both on matrix.org and my test server which doesn't have any RTC foci 🤔

pixlwave · 2025-06-06T14:03:34Z

Do you want to try doug/rtc-focus-logs and filter your logs for [BENOIT] and report back what you see?

bmarty · 2025-06-11T10:27:54Z

Hello @pixlwave , here are the logs:

2025-06-11 12:24:06.272 32271-32533 org.matrix.rust.sdk     io.element.android.x.debug           E  matrix_sdk_ffi::client: [BENOIT] RTC foci: [LiveKit(LiveKitRtcFocusInfo { service_url: "https://livekit-jwt.call.matrix.org" })] | bindings/matrix-sdk-ffi/src/client.rs:1498
2025-06-11 12:24:14.006 32271-32533 org.matrix.rust.sdk     io.element.android.x.debug           E  matrix_sdk_ffi::client: [BENOIT] RTC foci: [LiveKit(LiveKitRtcFocusInfo { service_url: "https://livekit-jwt.call.matrix.org" })] | bindings/matrix-sdk-ffi/src/client.rs:1498
2025-06-11 12:24:15.903 32271-32533 org.matrix.rust.sdk     io.element.android.x.debug           E  matrix_sdk_ffi::client: [BENOIT] RTC foci: [LiveKit(LiveKitRtcFocusInfo { service_url: "https://livekit-jwt.call.matrix.org" })] | bindings/matrix-sdk-ffi/src/client.rs:1498
---------------------------- PROCESS STARTED (4802) for package io.element.android.x.debug ----------------------------
2025-06-11 12:26:27.133  4802-4927  org.matrix.rust.sdk     io.element.android.x.debug           E  matrix_sdk_ffi::client: [BENOIT] RTC foci: [LiveKit(LiveKitRtcFocusInfo { service_url: "https://livekit-jwt.call.matrix.org" })] | bindings/matrix-sdk-ffi/src/client.rs:1498
2025-06-11 12:26:28.148  4802-5173  org.matrix.rust.sdk     io.element.android.x.debug           E  matrix_sdk_ffi::client: [BENOIT] RTC foci: [LiveKit(LiveKitRtcFocusInfo { service_url: "https://livekit-jwt.call.matrix.org" })] | bindings/matrix-sdk-ffi/src/client.rs:1498
2025-06-11 12:26:28.743  4802-5081  org.matrix.rust.sdk     io.element.android.x.debug           E  matrix_sdk::client: [BENOIT] Fetching client well-known from https://matrix-client.matrix.org/ | crates/matrix-sdk/src/client/mod.rs:1773

And it's working fine now, so I am not sure what happened previously.

bmarty

Working fine on Element X Android on a matrix.org account.

poljar · 2025-06-11T11:46:08Z

Can this be reviewed now? Could you point to the correct Ruma branch or release? This will get rid of the CI issue.

bmarty · 2025-06-11T12:23:58Z

ruma/ruma#2091 is not available in the latest Ruma release 0.12.3, so I guess we need to wait for the next release?

poljar · 2025-06-11T12:28:33Z

ruma/ruma#2091 is not available in the latest Ruma release 0.12.3, so I guess we need to wait for the next release?

It's part of the stable branch, so we can depend on that ruma/ruma@d1d53e2.

bnjbvr · 2025-06-11T14:31:16Z

Hello @pixlwave , here are the logs:

2025-06-11 12:24:06.272 32271-32533 org.matrix.rust.sdk     io.element.android.x.debug           E  matrix_sdk_ffi::client: [BENOIT] RTC foci: [LiveKit(LiveKitRtcFocusInfo { service_url: "https://livekit-jwt.call.matrix.org" })] | bindings/matrix-sdk-ffi/src/client.rs:1498
2025-06-11 12:24:14.006 32271-32533 org.matrix.rust.sdk     io.element.android.x.debug           E  matrix_sdk_ffi::client: [BENOIT] RTC foci: [LiveKit(LiveKitRtcFocusInfo { service_url: "https://livekit-jwt.call.matrix.org" })] | bindings/matrix-sdk-ffi/src/client.rs:1498
2025-06-11 12:24:15.903 32271-32533 org.matrix.rust.sdk     io.element.android.x.debug           E  matrix_sdk_ffi::client: [BENOIT] RTC foci: [LiveKit(LiveKitRtcFocusInfo { service_url: "https://livekit-jwt.call.matrix.org" })] | bindings/matrix-sdk-ffi/src/client.rs:1498
---------------------------- PROCESS STARTED (4802) for package io.element.android.x.debug ----------------------------
2025-06-11 12:26:27.133  4802-4927  org.matrix.rust.sdk     io.element.android.x.debug           E  matrix_sdk_ffi::client: [BENOIT] RTC foci: [LiveKit(LiveKitRtcFocusInfo { service_url: "https://livekit-jwt.call.matrix.org" })] | bindings/matrix-sdk-ffi/src/client.rs:1498
2025-06-11 12:26:28.148  4802-5173  org.matrix.rust.sdk     io.element.android.x.debug           E  matrix_sdk_ffi::client: [BENOIT] RTC foci: [LiveKit(LiveKitRtcFocusInfo { service_url: "https://livekit-jwt.call.matrix.org" })] | bindings/matrix-sdk-ffi/src/client.rs:1498
2025-06-11 12:26:28.743  4802-5081  org.matrix.rust.sdk     io.element.android.x.debug           E  matrix_sdk::client: [BENOIT] Fetching client well-known from https://matrix-client.matrix.org/ | crates/matrix-sdk/src/client/mod.rs:1773

And it's working fine now, so I am not sure what happened previously.

Note the server-side data is cached for some time, and must be re-fetched after some time. So if you happened to have a cached entry, with the deserialization defaulting to filling the new info as empty if it's not been found during deserialization, then the app could conclude that the info was missing, while it's just outdated.

bmarty · 2025-06-12T15:51:11Z

@bnjbvr thanks, but I was forcing a refresh by invoking reset_server_info() 🤷‍♂️

bmarty · 2025-06-16T14:13:14Z

I have added a commit, please let me know if it's correct (I am not 💯 sure) and if we need anything else to be able to merge this PR. Thanks!

poljar

Not yet quite right. As the CI also complains about.

poljar · 2025-06-17T12:21:18Z

Cargo.toml

@@ -78,7 +78,7 @@ ruma = { git = "https://github.com/pixlwave/Ruma", branch = "doug/sdk+patches",
    "unstable-msc4278",
    "unstable-msc4286",
 ] }
-ruma-common = { git = "https://github.com/pixlwave/Ruma", branch = "doug/sdk+patches" }
+ruma-common = "0.15.2"


You need to depend on the same Ruma version for all the Ruma crates, otherwise Rust will complain about types not having the same version.

pixlwave · 2025-06-19T10:27:32Z

I've rebased on main and squashed Benoit's change into the history so it doesn't get merged with my fork in the history.

@bmarty: I think the issue was running cargo update - I reverted the lock file and let RustRover load the project and the resulting lock file changes were much smaller 🤷‍♂️

poljar · 2025-06-19T10:39:35Z

Thanks, sorry for the delay we had a bit of trouble with our CI setup. I'm going to review this now.

poljar

Ok, I think that this is fine logic-wise. The changelog entries need some fixing though.

poljar · 2025-06-19T11:09:44Z

crates/matrix-sdk-base/CHANGELOG.md

+- The cached `ServerCapabilities` has been renamed to `ServerInfo` and additionally contains
+  the well-known response alongside the existing server versions. Despite the old name, it
+  does not contain the server capabilities (yet?!).


This is now in the completely wrong place, it also lacks a link to the PR. Finally, the "yet?!", feels a bit tacky for a changelog entry.

poljar · 2025-06-19T11:11:04Z

crates/matrix-sdk/CHANGELOG.md

+- `ClientServerCapabilities` has been renamed to `ClientServerInfo`. Alongside this,
+  `Client::reset_server_info` is now `Client::reset_server_info` and `Client::fetch_server_capabilities`
+  is now `Client::fetch_server_versions`, returning the server versions response directly.


This one's in the wrong place as well, please add new entries to the top, so this would have been a merge conflict if it was at the top.

PR link is also missing.

poljar · 2025-06-19T11:12:18Z

bindings/matrix-sdk-ffi/CHANGELOG.md

@@ -22,6 +22,7 @@ Breaking changes:
 - `RoomInfo` replaces its field `is_tombstoned: bool` with `tombstone: Option<RoomTombstoneInfo>`,
  containing the data needed to implement the room migration UI, a message and the replacement room id.
  ([#5027](https://github.com/matrix-org/matrix-rust-sdk/pull/5027))
+- `Client::reset_server_capabilities` has been renamed to `Client::reset_server_info`.


Wrong place, PR link please.

pixlwave · 2025-06-19T12:02:35Z

Changelogs updated (TIL you put them in reverse chronological order).

poljar · 2025-06-19T15:02:16Z

bindings/matrix-sdk-ffi/CHANGELOG.md

+- `Client::reset_server_capabilities` has been renamed to `Client::reset_server_info`.
+  ([#5167](https://github.com/matrix-org/matrix-rust-sdk/pull/5167))


Well that's still in the wrong place, this PR is not part of the 0.12.0 release from two weeks ago.

Ohhhh sorry I see what you're saying, I conflated the ordering comment with the "wrong place" comments… 🙈

poljar · 2025-06-19T15:02:59Z

crates/matrix-sdk-base/CHANGELOG.md

+- The cached `ServerCapabilities` has been renamed to `ServerInfo` and
+  additionally contains the well-known response alongside the existing server versions.
+  Despite the old name, it does not contain the server capabilities.
+  ([#5167](https://github.com/matrix-org/matrix-rust-sdk/pull/5167))


This one is under the 0.11.0 release, which is happened two months ago.

poljar · 2025-06-19T15:03:33Z

crates/matrix-sdk/CHANGELOG.md

+- `ClientServerCapabilities` has been renamed to `ClientServerInfo`. Alongside this,
+  `Client::reset_server_info` is now `Client::reset_server_info` and `Client::fetch_server_capabilities`
+  is now `Client::fetch_server_versions`, returning the server versions response directly.
+  ([#5167](https://github.com/matrix-org/matrix-rust-sdk/pull/5167))


This one is under the 0.12.0 release.

It has nothing to do with /capabilities so is confusing. We can use this new struct to combine the well-known response into a single cache too.

…uses it.

pixlwave · 2025-06-19T15:26:35Z

Alright, moved 👍

Out of interest, are there any plans to adopt Towncrier for these changelogs? It would be an awful lot easier for contributors (especially as we use them elsewhere) 🙂

poljar · 2025-06-19T15:38:17Z

Out of interest, are there any plans to adopt Towncrier for these changelogs? It would be an awful lot easier for contributors (especially as we use them elsewhere) 🙂

No, we had plans to use git-cliff but ultimately it was too complicated of a setup.

I think towncrier would reintroduce some of the downsides git-cliff had.

pixlwave · 2025-06-19T15:57:29Z

No, we had plans to use git-cliff but ultimately it was too complicated of a setup.

I think towncrier would reintroduce some of the downsides git-cliff had.

What were the downsides of git-cliff? Towncrier doesn't use commits, just changelog entries as a text file so it mimicks what we're doing right now updating the CHANGELOG.md, but eradicates conflicts, accidentally rebasing entries into the wrong version and semi-automates the PR link generation (from the file number). It would be a very nice improvement to the workflow :)

We could quite easily set up a changelog.d directory per crate and then tweak the existing matrix-org style guide for contributors to match the multi-crate structure.

pixlwave requested a review from a team as a code owner June 4, 2025 13:29

pixlwave requested review from andybalaam and removed request for a team June 4, 2025 13:29

pixlwave commented Jun 4, 2025

View reviewed changes

Cargo.toml Show resolved Hide resolved

pixlwave changed the title ~~Doug/live kit rtc focus~~ SDK: Cache the RTC foci from the well-known file. Jun 4, 2025

pixlwave force-pushed the doug/live-kit-rtc-focus branch from 2ffc5cb to c72c656 Compare June 4, 2025 14:10

pixlwave force-pushed the doug/live-kit-rtc-focus branch from c72c656 to e0f18e7 Compare June 4, 2025 14:38

poljar requested review from poljar and removed request for andybalaam June 5, 2025 09:04

pixlwave force-pushed the doug/live-kit-rtc-focus branch from 80435fe to d7f2550 Compare June 6, 2025 11:05

bmarty approved these changes Jun 11, 2025

View reviewed changes

poljar requested changes Jun 17, 2025

View reviewed changes

pixlwave force-pushed the doug/live-kit-rtc-focus branch from 07dfec2 to dbf2014 Compare June 19, 2025 10:01

pixlwave requested a review from poljar June 19, 2025 10:28

poljar requested changes Jun 19, 2025

View reviewed changes

pixlwave requested a review from poljar June 19, 2025 12:02

poljar requested changes Jun 19, 2025

View reviewed changes

pixlwave force-pushed the doug/live-kit-rtc-focus branch from a93a542 to 0df12a7 Compare June 19, 2025 15:23

pixlwave added 6 commits June 19, 2025 16:23

chore: Refactor ServerCapabilities into ServerInfo.

d72d5dd

It has nothing to do with /capabilities so is confusing. We can use this new struct to combine the well-known response into a single cache too.

sdk: Cache the client well-known file and add Client::rtc_foci which …

52b73d9

…uses it.

ffi: Expose a check for LiveKit RTC support.

4f1975f

fix: Handle a crash accessing the RTC foci when the well-known was None.

f7c57c3

chore: Make the RTC foci crash fix type-safe.

7ee1d40

chore: Fix changelogs after rebase

e69b33f

pixlwave force-pushed the doug/live-kit-rtc-focus branch from 0df12a7 to e69b33f Compare June 19, 2025 15:24

pixlwave requested a review from poljar June 19, 2025 15:26

poljar approved these changes Jun 19, 2025

View reviewed changes

poljar merged commit 06b3871 into main Jun 19, 2025
45 checks passed

poljar deleted the doug/live-kit-rtc-focus branch June 19, 2025 15:40

pixlwave mentioned this pull request Jun 20, 2025

Expose isLiveKitRTCSupported on the ClientProxy. element-hq/element-x-ios#4238

Merged

		- `Client::reset_server_capabilities` has been renamed to `Client::reset_server_info`.
		([#5167](https://github.com/matrix-org/matrix-rust-sdk/pull/5167))

SDK: Cache the RTC foci from the well-known file. #5167

SDK: Cache the RTC foci from the well-known file. #5167

Uh oh!

Conversation

pixlwave commented Jun 4, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

codecov bot commented Jun 4, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

bmarty commented Jun 5, 2025

Uh oh!

pixlwave commented Jun 5, 2025

Uh oh!

bmarty commented Jun 5, 2025

Uh oh!

pixlwave commented Jun 5, 2025

Uh oh!

bnjbvr commented Jun 5, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

pixlwave commented Jun 5, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

pixlwave commented Jun 5, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

bmarty commented Jun 5, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

pixlwave commented Jun 6, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

bmarty commented Jun 6, 2025

Uh oh!

bmarty commented Jun 6, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

pixlwave commented Jun 6, 2025

Uh oh!

pixlwave commented Jun 6, 2025

Uh oh!

bmarty commented Jun 11, 2025

Uh oh!

bmarty left a comment

Choose a reason for hiding this comment

Uh oh!

poljar commented Jun 11, 2025

Uh oh!

bmarty commented Jun 11, 2025

Uh oh!

poljar commented Jun 11, 2025

Uh oh!

bnjbvr commented Jun 11, 2025

Uh oh!

bmarty commented Jun 12, 2025

Uh oh!

bmarty commented Jun 16, 2025

Uh oh!

poljar left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

pixlwave commented Jun 19, 2025

Uh oh!

poljar commented Jun 19, 2025

Uh oh!

poljar left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

pixlwave commented Jun 19, 2025

pixlwave commented Jun 4, 2025 •

edited

Loading

codecov bot commented Jun 4, 2025 •

edited

Loading

bnjbvr commented Jun 5, 2025 •

edited

Loading

pixlwave commented Jun 5, 2025 •

edited

Loading

pixlwave commented Jun 5, 2025 •

edited

Loading

bmarty commented Jun 5, 2025 •

edited

Loading

pixlwave commented Jun 6, 2025 •

edited

Loading

bmarty commented Jun 6, 2025 •

edited

Loading

pixlwave commented Jun 19, 2025 •

edited

Loading