Skip to content

Conversation

@Half-Shot
Copy link
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
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
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
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
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
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.

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.

4 participants