Skip to content

Support alternate for UDP allocation#546

Draft
JoTurk wants to merge 1 commit intomasterfrom
alternate
Draft

Support alternate for UDP allocation#546
JoTurk wants to merge 1 commit intomasterfrom
alternate

Conversation

@JoTurk
Copy link
Member

@JoTurk JoTurk commented Jan 27, 2026

Description

This is a draft for supporting alternate addresses, this works for UDP.

two things:

  1. to support alternate for TCP allocation we'll have to add a connection-factory helper or something so users can provide connections for alternate addresses or create the connections our-self, i feel like v5 was a missed opportunity to fix this.
  2. I reviewed the libwebrtc code and a few other libraries and couldn't find any explicit limit on the number of alternatives they follow. idk that felt risky to me, so I hard-coded a limit of 64, which I think is very generous.

Reference issue

Fixes #544

@JoTurk JoTurk force-pushed the alternate branch 2 times, most recently from 3adcb47 to 7a36c2a Compare January 27, 2026 22:54
@codecov
Copy link

codecov bot commented Jan 27, 2026

Codecov Report

❌ Patch coverage is 84.28571% with 11 lines in your changes missing coverage. Please review.
✅ Project coverage is 81.87%. Comparing base (aa56100) to head (32280ba).

Files with missing lines Patch % Lines
client.go 84.28% 8 Missing and 3 partials ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master     #546      +/-   ##
==========================================
+ Coverage   81.57%   81.87%   +0.30%     
==========================================
  Files          46       46              
  Lines        3191     3244      +53     
==========================================
+ Hits         2603     2656      +53     
- Misses        385      387       +2     
+ Partials      203      201       -2     
Flag Coverage Δ
go 81.87% <84.28%> (+0.30%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@JoTurk
Copy link
Member Author

JoTurk commented Jan 27, 2026

@sean-bauer @rg0now @adrianosela I'm going to keep this a draft until we decide what to do with TCP, I think our options are: either add a dial/connection-factory in config so AllocateTCP() can re-connect, or keep this UDP-only for now and have TCP either fail on 300 or return the alternate addr so callers can retry with a new client.

@rg0now
Copy link
Contributor

rg0now commented Feb 4, 2026

Interesting. Kept me a while to think through.

One option is to change the turn.Client to receive a net.Dialer instead of a net.PacketConn (this would be your connection factory?) and then let Allocate create the server connections for itself and encapsulate the retry logic. The net.Dialer could enforce the redirect limit by failing to create new connections over a certain quota, or we could make the quota explicit in the config. If the caller wants glorified sockets (e.g., enriched with Prom counters), they'd need to pass us a net.Dialer that generates such connections. This covers both TCP and UDP. The cost is that we would have to break the API (again) and change the entire client logic: the client now expects a net.PacketConn and uses WriteTo/ReadFrom to send to/receive from the server. However, net.Dialer produces a net.Conn that is already connected to the server so internally we should use Write/Read for sending/receiving stuff. I fail to see why this would go wrong; in fact a connected socket feels more correct since we always speak to the same TURN server (minus TryAlternate).

The second option is to rely on the caller to track the alternates and implement the retry logic. I mean we may want to fail on 300 AND return the alternate addr in the err so callers can retry with a new client. This looks simpler, at the cost of the caller having to create multiple clients/connections until they succeed (which sux). I don't think we can make the same client reusable for multiple client.Allocate calls, or at least I think we cannot reliably reuse a TCP server connection that we get from the user.

The third option is implement the redirect logic for UDP and fail on 300 for TCP. I guess this is what you're doing here, right?

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

Labels

None yet

Development

Successfully merging this pull request may close these issues.

Error code 300 gets ignored

2 participants