fix(voip): dial relays on the web client port - #254
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (4)
🚧 Files skipped from review as they are similar to previous changes (1)
Included review availability: Your plan includes up to 2 reviews per rolling hour; 0 remain after this review. 📝 WalkthroughWalkthroughThe VoIP relay configuration now uses the shared ChangesVoIP relay port handling
Estimated code review effort: 2 (Simple) | ~10 minutes Merge Risk: 🟡 Moderate · up to The current behavior still forces relay connections to port 3478, which can make relays advertising another port unreachable and collapse distinct same-host relay endpoints. The PR is not merge-ready until advertised ports are preserved and the fallback is applied only when no port is provided. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
ecf0129 to
714cfdf
Compare
`connectRelays` pinned every relay connection to 3478. A relay answers only on the port it advertises, and reaching one on 3478 completes the handshake and carries the uplink without ever forwarding the peer's stream back, so the call is silently one way with no error anywhere. WhatsApp Web's own constants name 3478 `FAUX_WEB_CLIENT_RELAY_PORT` and 3480 `TRUE_WEB_CLIENT_RELAY_PORT`, and it builds its connection map with `portOverride: (port) => shouldUseOriginalRelayPort() ? port : TRUE`, so by default it dials every candidate on 3480 and keeps the advertised one only as `originalPort`. Overriding the advertised port is therefore correct; the value was not. `useOriginalRelayPort` mirrors the other side of that gate. It defaults to `false`, so nothing changes for a caller that does not ask, and it is the only way to reach a relay that answers on the port it advertises, which no amount of configuration outside the plugin could do before.
714cfdf to
c1d1775
Compare
Why
connectRelayspins every relay connection to 3478. A relay answers only on the port it advertises, and reaching one on 3478 completes the handshake and carries the uplink without ever forwarding the peer's stream back, so the call is silently one way and nothing surfaces as an error.WhatsApp Web names the two ports itself:
and builds its connection map with
portOverride: (port) => shouldUseOriginalRelayPort() ? port : TRUE_WEB_CLIENT_RELAY_PORT, so by default it dials every candidate on 3480 and keeps the advertised port only asoriginalPort.Overriding the advertised port is therefore right, which is why this is not a change of shape. The value was the faux one, and 3480 is already in the package as
CONFIG.TRUE_WEB_CLIENT_RELAY_PORT, the fallbackconfigureRelaysapplies to an endpoint that carries no port.The gate is worth mirroring too. As it stands a relay is only ever reachable on one fixed port, however the
<te2>block addresses it, and nothing outside the plugin can change that.What
port: TRUE_WEB_CLIENT_RELAY_PORT, and the generated name follows it. The constant moves next to the behaviour it explains and is exported, soCONFIGand the media session share one definition instead of two literals.useOriginalRelayPort, defaultfalse, dials the port each endpoint advertises instead. It threads down the same pathmaxConcurrentCallsalready takes, and the doc comment says plainly that it is the wrong choice against WhatsApp's own relays.Notes
The port policy could instead live in
WaSctpRelay, whoseconfigureRelaysalready appliesrelay.port || CONFIG.TRUE_WEB_CLIENT_RELAY_PORT, with the session passingep.portthrough. That puts one place in charge of the dialled port. Happy to move it if you prefer that shape.Tests
Five in
packages/voip/src/call/__tests__/media-session-relays.test.ts, on what reachesconfigureRelays: relays are dialled on the web client port, an endpoint advertising the faux port is dialled there too, an unnamed relay is named for the address it dials, the escape hatch dials the advertised port, and two ports on one host stay two relays under it. The first three fail onmaster.npm testinpackages/voippasses (77 tests);tsc --noEmit, ESLint and Prettier clean.