Skip to content

Support MSC4143 RTC Transport endpoint#3629

Merged
Half-Shot merged 19 commits intolivekitfrom
hs/support-homeserver-rtc-transport
Dec 29, 2025
Merged

Support MSC4143 RTC Transport endpoint#3629
Half-Shot merged 19 commits intolivekitfrom
hs/support-homeserver-rtc-transport

Conversation

@Half-Shot
Copy link
Copy Markdown
Member

This implements MSC4143's backend endpoint for transports, and uses matrix-org/matrix-js-sdk#5104 to handle it.

This changes makeTransport to:

  • Try all listed transports, not just the first livekit one.
  • Test all backend transports, then all well-known transports, then all config transports.

@Half-Shot Half-Shot requested a review from a team as a code owner December 11, 2025 13:56
@Half-Shot Half-Shot requested a review from AndrewFerr December 11, 2025 13:56
@Half-Shot Half-Shot added the T-Enhancement New features, changes in functionality, performance boosts, user-facing improvements label Dec 11, 2025
@Half-Shot Half-Shot added PR-Improvement Release note category. A PR that improves EC's performance or stability. and removed T-Enhancement New features, changes in functionality, performance boosts, user-facing improvements labels Dec 11, 2025
@toger5
Copy link
Copy Markdown
Contributor

toger5 commented Dec 11, 2025

This needs more coverage.
It looks like throwing in a test that uses the new endpoint would get you almost to 100% since it would run thorugh both "throw levels"

Copy link
Copy Markdown
Contributor

@toger5 toger5 left a comment

Choose a reason for hiding this comment

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

This looks sensible.

If one of them fails we always run into a hard error. Do we want automatically check if we suceed in loading one of the lower prio ones?

@Half-Shot
Copy link
Copy Markdown
Member Author

This needs more coverage. It looks like throwing in a test that uses the new endpoint would get you almost to 100% since it would run thorugh both "throw levels"

Yup! Working through it

@toger5
Copy link
Copy Markdown
Contributor

toger5 commented Dec 11, 2025

In other words: I am not sure if:

Test all backend transports, then all well-known transports, then all config transports.

is actually happening. Are we really continuing to check well-known transports if backend transports fail?
edit
at least in the
ex instanceof FailToGetOpenIdToken case.
edit
Oh which is an issue with our HS and entirely independent of the sfu/jwt.
Liking this more and more while looking into it :)

@Half-Shot
Copy link
Copy Markdown
Member Author

Oh which is an issue with our HS and entirely independent of the sfu/jwt.
Liking this more and more while looking into it :)

That was my intention yes, but seeing as it wasn't clear this needs more comments on the PR.

// https://github.com/element-hq/element-call/issues/3344
// The app used to request a new jwt token then to reconnect to the SFU
expect(wsConnectionCount).toBe(1);
expect(sfuGetCallCount).toBe(2 /* the first one is for the warmup */);
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.

Removed this test because livekit/jwt/sfu/get is called whenever we just want to validate a transport, which unfortunately happens every time we instantiate LocalTransport, which seems to happen a few times over this call.

The websocket connection test should be enough here as it still tests we aren't making any reconnections.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

PR-Improvement Release note category. A PR that improves EC's performance or stability.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants