Fetch capabilities in the background #4246

dbkr · 2024-06-17T16:46:54Z

& keep them up to date.

I've elected to change the interface here and split it into two functions rather than re-use the same one. That way, the cached one doesn't need to return a promise. This does make it a BREAKING CHANGE. I've given both functions different names so it's obvious that they're different. It's a trivial code update so I don't really think its worth having a backwards compat function?

It also splits some code out of client.ts(!)

Any of the tests that use runAllTimers() needed replacing with something that would just run the ones they need, since this works by continually setting a timer to keep the capabilities up to date. I don't think runAllTimers() is a sensible thing to use unless you're testing a small piece of code where you know all the timers it's setting.

Checklist

Tests written for new code (and old code if feasible).
New or updated public/exported symbols have accurate TSDoc documentation.
Linter and other CI checks pass.
Sign-off given on the changes (see CONTRIBUTING.md).

& keep them up to date

and round down the wait time for sanity

Also updates to the js-sdk interface changes in matrix-org/matrix-js-sdk#4246

t3chguy · 2024-06-18T11:23:32Z

src/models/room.ts

@@ -611,8 +612,8 @@ export class Room extends ReadReceipt<RoomEmittedEvents, RoomEventHandlerMap> {
     * Resolves to the version the room should be upgraded to.
     */
    public async getRecommendedVersion(): Promise<IRecommendedVersion> {
-        const capabilities = await this.client.getCapabilities();
-        let versionCap = capabilities["m.room_versions"];
+        const capabilities = this.client.getCachedCapabilities();


What if getRecommendedVersion is called before the first capabilities response comes down?

Good point. I've put back a function that does the same as before.

src/rendezvous/MSC3906Rendezvous.ts

src/serverCapabilities.ts

… there

t3chguy

Should we maybe have an emitter if you wanted to watch for changes, like we do for well-known polling?

dbkr · 2024-06-18T16:00:53Z

Perhaps, but I don't think it would be useful to us right now, particularly? Especially given it will only update every 6 hours, normally. I'd be inclined to keep any extra complexity out until its needed.

It's a separate method to force a capabilities fetch as of matrix-org/matrix-js-sdk#4246

BillCarsonFr

Just a few comments

src/serverCapabilities.ts

BillCarsonFr · 2024-06-19T09:25:23Z

src/client.ts

+    public async getCapabilities(): Promise<Capabilities> {
+        const caps = this.serverCapabilitiesService.getCachedCapabilities();
+        if (caps) return caps;
+        return this.serverCapabilitiesService.fetchCapabilities();


So it doesn't swallow errors anymore? only rendez-vous was calling it?

Yeah, swallowing errors silently and returning something seems like an awful behaviour, so I've changed it favour of having the places its used catch the exception instead, and if they choose to ignore it then that's up to them. It was used in a couple of places, not that many.

BillCarsonFr · 2024-06-19T09:26:19Z

src/models/room.ts

@@ -636,7 +640,7 @@ export class Room extends ReadReceipt<RoomEmittedEvents, RoomEventHandlerMap> {
                    "to be supporting a newer room version we don't know about.",
            );

-            const caps = await this.client.getCapabilities(true);
+            const caps = await this.client.fetchCapabilities();


this would throw here now? instead of returning a default value? Would impact the react-sdk?

yeah, I forgot to catch the exception here actually.

It's a separate method to force a capabilities fetch as of matrix-org/matrix-js-sdk#4246

Also updates to the js-sdk interface changes in matrix-org/matrix-js-sdk#4246

* Disable profile controls if the HS doesn't allow them to be set Also updates to the js-sdk interface changes in matrix-org/matrix-js-sdk#4246 * Remove unnecessary await * Pass disabled prop to accessiblebutton in avatarsetting * Use getCapabilities in case there are no cached capabilities * Fix test * Go back to just using getCapabilities Rather than change the other places

* Disable profile controls if the HS doesn't allow them to be set Also updates to the js-sdk interface changes in matrix-org/matrix-js-sdk#4246 * Remove unnecessary await * Pass disabled prop to accessiblebutton in avatarsetting * Move the account management button The section it lives in with the server name goes, and the button just lives on its own in the profile section. * Update test * Revert bits of previous PR that are no longer wanted because we squash merge so git can no longer make sense of what changes have been applied. * More squash-merge fails * More more squash merge fails

Fetch capabilities in the background

4854903

& keep them up to date

dbkr added the T-Enhancement label Jun 17, 2024

github-actions bot deployed to PR Documentation Preview June 17, 2024 16:48 View deployment

Add missed await

919d725

github-actions bot deployed to PR Documentation Preview June 18, 2024 10:20 View deployment

Replace some more runAllTimers

3aa37a9

and round down the wait time for sanity

github-actions bot deployed to PR Documentation Preview June 18, 2024 10:32 View deployment

dbkr added the X-Breaking-Change label Jun 18, 2024

dbkr marked this pull request as ready for review June 18, 2024 11:00

dbkr requested review from a team as code owners June 18, 2024 11:00

dbkr requested review from richvdh, BillCarsonFr and t3chguy June 18, 2024 11:00

dbkr added a commit to matrix-org/matrix-react-sdk that referenced this pull request Jun 18, 2024

Disable profile controls if the HS doesn't allow them to be set

f7a69dd

Also updates to the js-sdk interface changes in matrix-org/matrix-js-sdk#4246

dbkr mentioned this pull request Jun 18, 2024

Disable profile controls if the HS doesn't allow them to be set matrix-org/matrix-react-sdk#12634

Closed

4 tasks

t3chguy reviewed Jun 18, 2024

View reviewed changes

dbkr added 3 commits June 18, 2024 12:44

Remove double comment

d88e60a

Typo

a759454

Add a method back that will fetch capabilities if they're not already…

fad022b

… there

github-actions bot deployed to PR Documentation Preview June 18, 2024 12:36 View deployment

Add tests

382cae7

github-actions bot deployed to PR Documentation Preview June 18, 2024 12:48 View deployment

dbkr requested a review from t3chguy June 18, 2024 12:53

t3chguy reviewed Jun 18, 2024

View reviewed changes

t3chguy approved these changes Jun 18, 2024

View reviewed changes

dbkr added a commit to matrix-org/matrix-react-sdk that referenced this pull request Jun 18, 2024

Update for js-sdk interface change

f10f0d8

It's a separate method to force a capabilities fetch as of matrix-org/matrix-js-sdk#4246

dbkr mentioned this pull request Jun 18, 2024

Update for js-sdk interface change matrix-org/matrix-react-sdk#12638

Merged

4 tasks

BillCarsonFr approved these changes Jun 19, 2024

View reviewed changes

Catch exception here too

000b02f

github-actions bot deployed to PR Documentation Preview June 19, 2024 09:39 View deployment

Add test for room version code

3e8c3c0

github-actions bot deployed to PR Documentation Preview June 19, 2024 10:20 View deployment

dbkr added this pull request to the merge queue Jun 19, 2024

Merged via the queue into develop with commit 819fc75 Jun 19, 2024
27 checks passed

dbkr deleted the dbkr/fetch_capabilities_background branch June 19, 2024 10:47

github-merge-queue bot pushed a commit to matrix-org/matrix-react-sdk that referenced this pull request Jun 19, 2024

Update for js-sdk interface change (#12638)

b2e9de1

It's a separate method to force a capabilities fetch as of matrix-org/matrix-js-sdk#4246

dbkr added a commit to matrix-org/matrix-react-sdk that referenced this pull request Jun 19, 2024

Disable profile controls if the HS doesn't allow them to be set

af6e6fe

Also updates to the js-sdk interface changes in matrix-org/matrix-js-sdk#4246

dbkr mentioned this pull request Jun 19, 2024

Disable profile controls if the HS doesn't allow them to be set matrix-org/matrix-react-sdk#12652

Merged

4 tasks

dbkr added a commit to matrix-org/matrix-react-sdk that referenced this pull request Jun 20, 2024

Disable profile controls if the HS doesn't allow them to be set

8a159bf

Also updates to the js-sdk interface changes in matrix-org/matrix-js-sdk#4246

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fetch capabilities in the background #4246

Fetch capabilities in the background #4246

dbkr commented Jun 17, 2024 •

edited

Loading

t3chguy Jun 18, 2024

dbkr Jun 18, 2024

t3chguy left a comment

dbkr commented Jun 18, 2024

BillCarsonFr left a comment

BillCarsonFr Jun 19, 2024

dbkr Jun 19, 2024

BillCarsonFr Jun 19, 2024

dbkr Jun 19, 2024

Fetch capabilities in the background #4246

Fetch capabilities in the background #4246

Conversation

dbkr commented Jun 17, 2024 • edited Loading

Checklist

t3chguy Jun 18, 2024

Choose a reason for hiding this comment

dbkr Jun 18, 2024

Choose a reason for hiding this comment

t3chguy left a comment

Choose a reason for hiding this comment

dbkr commented Jun 18, 2024

BillCarsonFr left a comment

Choose a reason for hiding this comment

BillCarsonFr Jun 19, 2024

Choose a reason for hiding this comment

dbkr Jun 19, 2024

Choose a reason for hiding this comment

BillCarsonFr Jun 19, 2024

Choose a reason for hiding this comment

dbkr Jun 19, 2024

Choose a reason for hiding this comment

dbkr commented Jun 17, 2024 •

edited

Loading