-
-
Notifications
You must be signed in to change notification settings - Fork 2.7k
Rtc: Let the widget driver decide to fallback to well-known or not for rtc discovery. #34581
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Changes from all commits
Commits
Show all changes
22 commits
Select commit
Hold shift + click to select a range
aafe840
rtc: fallback to well-known only if config allows
BillCarsonFr fd4d19b
Use existing rtc transports cached by client
BillCarsonFr 746702d
post rebase fixes
BillCarsonFr 5328a8f
post rebase test update
BillCarsonFr ef1b6ba
cleanup callstore tests
BillCarsonFr 0e79e41
Lint + fmt fixes
BillCarsonFr 20e98f0
postrebase: update useCall tests
BillCarsonFr d34ebee
fixup: fix mocking on useRoomCall task
BillCarsonFr 9dc1437
fix lint
BillCarsonFr 98143ef
fixup lint
BillCarsonFr 0a280b4
Merge branch 'develop' into valere/rtc/cache_dicovery
BillCarsonFr 3b08260
fix missing act in react test
BillCarsonFr 6412799
fix client mocks
BillCarsonFr 60ed948
fix test mocking for rtc discovery
BillCarsonFr 191ed48
fix test mocking
BillCarsonFr 1f8eac0
fix Callstore testing
BillCarsonFr 47375c1
lint:js-fix
BillCarsonFr f8e99c5
Merge branch 'develop' into valere/rtc/cache_dicovery
BillCarsonFr 9e30637
add missing mocks
BillCarsonFr 7aa1db1
Merge branch 'develop' into valere/rtc/cache_dicovery
robintown 52ff54a
Fix duplicate ElementWidgetDriver test cases
robintown e98c60d
Avoid logging warning about well-known foci when they are undefined
robintown File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -8,16 +8,16 @@ | |
|
|
||
| import { logger } from "matrix-js-sdk/src/logger"; | ||
| import { type MatrixRTCSession, MatrixRTCSessionManagerEvents, type Transport } from "matrix-js-sdk/src/matrixrtc"; | ||
| import { MatrixError, type EmptyObject, type Room } from "matrix-js-sdk/src/matrix"; | ||
| import { ClientEvent, type EmptyObject, type Room } from "matrix-js-sdk/src/matrix"; | ||
|
|
||
| import defaultDispatcher from "../dispatcher/dispatcher"; | ||
| import { UPDATE_EVENT } from "./AsyncStore"; | ||
| import { AsyncStoreWithClient } from "./AsyncStoreWithClient"; | ||
| import WidgetStore from "./WidgetStore"; | ||
| import SettingsStore from "../settings/SettingsStore"; | ||
| import { SettingLevel } from "../settings/SettingLevel"; | ||
| import SdkConfig from "../SdkConfig"; | ||
| import { Call, CallEvent, ConnectionState } from "../models/Call"; | ||
| import SdkConfig from "../SdkConfig.ts"; | ||
|
|
||
| export enum CallStoreEvent { | ||
| // Signals a change in the call associated with a given room | ||
|
|
@@ -41,8 +41,6 @@ | |
| return this._instance; | ||
| } | ||
|
|
||
| private readonly configuredMatrixRTCTransports = new Set<Transport>(); | ||
|
|
||
| private constructor() { | ||
| super(defaultDispatcher); | ||
| this.setMaxListeners(100); // One for each RoomTile | ||
|
|
@@ -52,44 +50,15 @@ | |
| // nothing to do | ||
| } | ||
|
|
||
| /** | ||
| * Fetch transports used by MatrixRTC services, such as Element Call. | ||
| * This function is called once during Store startup which means we don't refetch | ||
| * transports every time we need to check for Element Call support. | ||
| */ | ||
| protected async fetchTransports(): Promise<void> { | ||
| if (!this.matrixClient) return; | ||
| this.configuredMatrixRTCTransports.clear(); | ||
| // Prefer checking the proper endpoint for transports. | ||
| try { | ||
| const transports = await this.matrixClient._unstable_getRTCTransports(); | ||
| transports.forEach((t) => this.configuredMatrixRTCTransports.add(t)); | ||
| } catch (ex) { | ||
| // Expected, MSC not implemented. | ||
| // | ||
| // Homeservers will return a 404 M_UNRECOGNIZED matrix error if they | ||
| // don't implement a requested endpoint. | ||
| if (ex instanceof MatrixError === false || ex.errcode !== "M_UNRECOGNIZED") { | ||
| logger.warn("Unexpected error when trying to fetch RTC transports", ex); | ||
| } | ||
| } | ||
| // See https://github.com/matrix-org/matrix-spec-proposals/blob/d61969a9a3696b6c54d7987b1643b5bc03670927/proposals/4143-matrix-rtc.md#discovery-of-foci-using-well-knownmatrixclient | ||
| // This well-known option has since been removed from the spec but is still widely deployed. | ||
| // Reading it can be disabled via config; the modern endpoint above is unaffected. | ||
| if (SdkConfig.get("enable_client_well_known_lookups")) { | ||
| await this.matrixClient.waitForClientWellKnown(); | ||
| const foci = this.matrixClient.getClientWellKnown()?.["org.matrix.msc4143.rtc_foci"]; | ||
| if (Array.isArray(foci)) { | ||
| foci.forEach((foci) => this.configuredMatrixRTCTransports.add(foci)); | ||
| } | ||
| } | ||
| this.emit(CallStoreEvent.TransportsUpdated); | ||
| } | ||
|
|
||
| protected async onReady(): Promise<any> { | ||
| if (!this.matrixClient) return; | ||
| // Fetch transports, but don't await the result. | ||
| void this.fetchTransports(); | ||
| this.matrixClient.cachedRtcTransports.wait().catch(() => { | ||
| if (SdkConfig.get("enable_client_well_known_lookups")) { | ||
| void this.matrixClient?.waitForClientWellKnown(); | ||
| } | ||
| }); | ||
|
|
||
| // We assume that the calls present in a room are a function of room | ||
| // widgets and group calls, so we initialize the room map here and then | ||
| // update it whenever those change | ||
|
|
@@ -99,6 +68,8 @@ | |
| this.matrixClient.matrixRTC.on(MatrixRTCSessionManagerEvents.SessionStarted, this.onRTCSessionStart); | ||
| WidgetStore.instance.on(UPDATE_EVENT, this.onWidgets); | ||
|
|
||
| this.matrixClient.on(ClientEvent.RtcTransportsUpdated, this.onRTCTransportsUpdated); | ||
|
|
||
| // If the room ID of a previously connected call is still in settings at | ||
| // this time, that's a sign that we failed to disconnect from it | ||
| // properly, and need to clean up after ourselves | ||
|
|
@@ -125,9 +96,9 @@ | |
| this.callListeners.clear(); | ||
| this.calls.clear(); | ||
| this._connectedCalls.clear(); | ||
| this.configuredMatrixRTCTransports.clear(); | ||
|
|
||
| this.matrixClient?.matrixRTC.off(MatrixRTCSessionManagerEvents.SessionStarted, this.onRTCSessionStart); | ||
| this.matrixClient?.off(ClientEvent.RtcTransportsUpdated, this.onRTCTransportsUpdated); | ||
| WidgetStore.instance.off(UPDATE_EVENT, this.onWidgets); | ||
| } | ||
|
|
||
|
|
@@ -138,6 +109,7 @@ | |
| public get connectedCalls(): Set<Call> { | ||
| return this._connectedCalls; | ||
| } | ||
|
|
||
| private set connectedCalls(value: Set<Call>) { | ||
| const prevValue = this._connectedCalls; | ||
| this._connectedCalls = value; | ||
|
|
@@ -156,6 +128,7 @@ | |
| private callListeners = new Map<Call, Map<CallEvent, (...args: unknown[]) => unknown>>(); | ||
|
|
||
| private inUpdateRoom = false; | ||
|
|
||
| private updateRoom(room: Room): void { | ||
| // XXX: This method is guarded with the flag this.inUpdateRoom because | ||
| // we need to block this method from calling itself recursively. That | ||
|
|
@@ -242,10 +215,26 @@ | |
| }; | ||
|
|
||
| public getConfiguredRTCTransports(): Transport[] { | ||
| return [...this.configuredMatrixRTCTransports]; | ||
| const rtcTransports = this.matrixClient?.cachedRtcTransports.get(); | ||
| const enableClientWellKnownLookups = SdkConfig.get("enable_client_well_known_lookups"); | ||
| if (rtcTransports || !enableClientWellKnownLookups) { | ||
| return rtcTransports ?? []; | ||
| } | ||
| const wellKnown = this.matrixClient?.getClientWellKnown(); | ||
| const foci = wellKnown?.["org.matrix.msc4143.rtc_foci"]; | ||
| if (foci !== undefined) { | ||
| if (Array.isArray(foci)) | ||
| return foci; // Contents assumed to be valid Transports | ||
| else logger.warn(`org.matrix.msc4143.rtc_foci is not an array in .well-known`); | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Surprised this passes linting, I thought we always used curly braces for |
||
| } | ||
| return []; | ||
| } | ||
|
|
||
| private onRTCSessionStart = (roomId: string, session: MatrixRTCSession): void => { | ||
| this.updateRoom(session.room); | ||
| }; | ||
|
|
||
| private onRTCTransportsUpdated = (transports: Transport[]): void => { | ||
| this.emit(CallStoreEvent.TransportsUpdated, transports); | ||
| }; | ||
| } | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Noting we've changed this to prefill the cache rather than mocking the response, is that going to be more prone to breakage if the behaviour of
_unstable_getRTCTransportschanges?