feat: collect ICE servers from all relays - #8486
Conversation
6ee7cef to
26ee796
Compare
| .values() | ||
| .flat_map(|metadata| metadata.ice_servers.clone()) | ||
| .collect(); | ||
| unresolved_ice_servers.sort(); |
There was a problem hiding this comment.
Because dedup on the next line only works after sorting. E.g. if we have multiple relays not returning the TURN server, they will all return a fallback which should be deduplicated.
hpk42
left a comment
hpk42
left a comment
There was a problem hiding this comment.
lgtm, though i am a little uneasy about the lack of tests.
I don't know what can be tested here without actually having multiple test relays. Maybe we actually need a second CI relay. |
calling the |
WofWca
left a comment
WofWca
left a comment
There was a problem hiding this comment.
Also see relevant forum post: https://support.delta.chat/t/calls-use-all-relays-turn-servers-not-just-one/5374?u=wofwca.
I didn't dive deep into the code, but it looks like metadata includes all relays, i.e. also unpublished ones? I think it would make sense to filter out those relays. But I guess it's not critical, the call isn't gonna fail because of that.
(This was brought up in https://support.delta.chat/t/relay-is-not-removed-from-connections-after-deletion-android-2-56/5587/7?u=wofwca).
|
On Thu, Jul 30, 2026 at 07:37 -0700, WofWca wrote:
@WofWca commented on this pull request.
I didn't dive deep into the code, but it looks like `metadata` includes all relays, i.e. also unpublished ones? I think it would make sense to filter out those relays. But I guess it's not critical, the call isn't gonna fail because of that.
definitely makes sense to only use published relays for the set of ICE servers.
also did this in #8490 for determining iroh relay candidates from relays.
Everything else is suprising especially since the UI action is "remove" these days,
and the hidden ICE relay would be used for a long time still.
|
|
(can be addressed in a follow-up if you ask me. The current main behavior seems to be basically "pick a TURN server from a random relay") |
|
26ee796 to
9a40455
Compare
4 participants
Relays that don't have a TURN server in the metadata still return a fallback relay. Not changing it currently, maybe we want to get rid of the fallback completely. Otherwise will need to move the call to
create_fallback_ice_servers()out of IMAP loop, likely intoice_servers()call.