Add support for external TURN servers - #14
Conversation
- Modifies the Go signaling server (`config.go`, `web.go`, `handler.go`, `signalling.go`) to optionally disable the built-in UDP TURN server. - Adds configurations for static external ICE servers or dynamic Cloudflare TURN API credentials. - Defaults to Metered OpenRelay `turn:openrelay.metered.ca` if enabled but unconfigured. - Updates the frontend `Room.ts` `initWS` method and WebRTC instantiation logic to properly handle dynamic ICE server payload structures sent over WebSockets. - Prevents containerized platforms (like Railway) from failing due to lack of UDP port bindings, enabling 100% free serverless compatibility. Co-authored-by: meghrathod <23073422+meghrathod@users.noreply.github.com>
|
👋 Jules, reporting for duty! I'm here to lend a hand with this pull request. When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down. I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job! For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with New to Jules? Learn more at jules.google/docs. For security, I will only act on instructions from the user who triggered this task. |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
|
Overall Grade Focus Area: Reliability |
Security Reliability Complexity Hygiene |
Code Review Summary
| Analyzer | Status | Updated (UTC) | Details |
|---|---|---|---|
| Shell | Mar 3, 2026 3:23a.m. | Review ↗ | |
| JavaScript | Mar 3, 2026 3:23a.m. | Review ↗ | |
| Go | Mar 3, 2026 3:23a.m. | Review ↗ |
There was a problem hiding this comment.
Pull request overview
Adds external TURN server support to the signalling backend and updates the frontend WebRTC setup to accept TURN/ICE configuration returned as JSON during the WebSocket handshake. This enables deployments on platforms that don’t allow inbound UDP and optionally integrates Cloudflare’s TURN credential API.
Changes:
- Add config fields for external ICE servers and Cloudflare TURN API credentials.
- Update WS handshake to return either the internal TURN host:port (string) or external ICE server configs (JSON).
- Update frontend peer connection creation to merge dynamic ICE servers (from handshake) into the RTC configuration.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| src/connection/room.ts | Accept TURN/ICE config as JSON and merge into RTCPeerConnection ICE servers. |
| signalling/web/web.go | Skip internal TURN initialization when external TURN is enabled. |
| signalling/web/signalling.go | Send TURN/ICE data to clients via WS handshake (string or JSON). |
| signalling/web/handler.go | Add external TURN credential retrieval (Cloudflare + caching) and fallback ICE server selection. |
| signalling/kabootar.example.toml | Document new external TURN / ICE server configuration options. |
| signalling/config/config.go | Add config schema for external ICE server entries and Cloudflare credentials. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| import ( | ||
| "errors" | ||
| "net" | ||
| "strconv" | ||
| "strings" | ||
|
|
||
| "github.com/gargakshit/kabootar/signalling/config" | ||
| "github.com/gargakshit/kabootar/signalling/util" | ||
| "github.com/gofiber/websocket/v2" | ||
| "github.com/pion/turn/v2" | ||
| "github.com/puzpuzpuz/xsync" | ||
| "bytes" | ||
| "encoding/json" | ||
| "errors" | ||
| "net" | ||
| "net/http" | ||
| "strconv" | ||
| "strings" | ||
| "sync" | ||
| "time" |
There was a problem hiding this comment.
This file also appears to be not gofmt-formatted (indentation in the import block and newHandler differs from other Go files). Please run gofmt to keep formatting consistent and avoid noisy diffs in future changes.
| IceServers interface{} `json:"iceServers"` | ||
| } | ||
|
|
||
| func (h *handler) getExternalTurnCredentials() (interface{}, error) { | ||
| if h.cfg.CloudflareTurnKeyID != "" && h.cfg.CloudflareTurnAPIToken != "" { |
There was a problem hiding this comment.
getExternalTurnCredentials returns (interface{}, error), but the function never returns a non-nil error and relies on interface{} for the Cloudflare payload. This makes the call-site logic (err == nil) misleading and removes compile-time guarantees about the JSON shape. Consider returning a concrete type like []config.ICEServer (or []map[string]any) and only returning nil/an error when Cloudflare fetch fails and no static external servers are configured.
| return []map[string]interface{}{ | ||
| { | ||
| "urls": []string{"turn:openrelay.metered.ca:80", "turn:openrelay.metered.ca:443", "turn:openrelay.metered.ca:443?transport=tcp"}, | ||
| "username": "openrelayproject", | ||
| "credential": "openrelayproject", |
There was a problem hiding this comment.
When no Cloudflare/static ICE servers are configured, this falls back to the public openrelay.metered.ca TURN servers. That has operational/privacy implications (traffic relayed via a third-party) and is a surprising default for a self-hosted signalling service. Consider requiring explicit configuration for external TURN (and returning an error if none is configured), or at least gating this fallback behind a config flag and documenting it clearly.
| private clientKey: string, | ||
| public file: Master extends true ? File : undefined, | ||
| eventDispatcher: RoomEventDispatcher<Master>, | ||
| turnServer: string, | ||
| turnServer: any, | ||
| public pin?: string, |
There was a problem hiding this comment.
turnServer is widened to any, which loses type-safety across the whole connection stack (Room/initWS/handlers) and makes it easy to accidentally pass a shape that RTCPeerConnection rejects at runtime. Consider defining an explicit union type (e.g., string | RTCIceServer | RTCIceServer[]) plus a small normalizer function so the rest of the code can stay strongly typed.
| if (typeof turnServer === "string") { | ||
| dynamicIceServers = [ | ||
| { | ||
| urls: `turn:${turnServer}?transport=tcp`, | ||
| username: roomID, |
There was a problem hiding this comment.
dynamicIceServers is currently typed as any[], even though RTCPeerConnection expects RTCIceServer[]. Typing this as RTCIceServer[] (and narrowing the input during normalization) will prevent accidentally passing an unsupported shape to the WebRTC API and make future refactors safer.
| this.dispatcher.connectionStatusChanged(false); | ||
|
|
||
| let dynamicIceServers: any[] = []; | ||
| if (typeof turnServer === "string") { | ||
| dynamicIceServers = [ | ||
| { | ||
| urls: `turn:${turnServer}?transport=tcp`, | ||
| username: id, |
There was a problem hiding this comment.
The TURN/ICE normalization logic is duplicated here and in ClientHandler. Consider extracting a shared helper (e.g., normalizeTurnToIceServers(turnServer, username, credential)) to avoid future divergence when adding more supported shapes (Cloudflare, static list, etc.).
| import ( | ||
| "time" | ||
| "time" | ||
|
|
||
| "github.com/gofiber/fiber/v2" | ||
| "github.com/gofiber/websocket/v2" | ||
| "github.com/gofiber/fiber/v2" | ||
| "github.com/gofiber/websocket/v2" |
There was a problem hiding this comment.
This file appears to have lost gofmt formatting (spaces instead of tabs in the import block and throughout HandleWS). Please run gofmt on this file to match the formatting used elsewhere in the Go codebase (e.g., signalling/web/web.go).
- Modifies the Go signaling server (`config.go`, `web.go`, `handler.go`, `signalling.go`) to optionally disable the built-in UDP TURN server. - Adds configurations for static external ICE servers or dynamic Cloudflare TURN API credentials. - Defaults to Metered OpenRelay `turn:openrelay.metered.ca` if enabled but unconfigured. - Updates the frontend `Room.ts` `initWS` method and WebRTC instantiation logic to properly handle dynamic ICE server payload structures sent over WebSockets. - Prevents containerized platforms (like Railway) from failing due to lack of UDP port bindings, enabling 100% free serverless compatibility. Co-authored-by: meghrathod <23073422+meghrathod@users.noreply.github.com>
- Modifies the Go signaling server (`config.go`, `web.go`, `handler.go`, `signalling.go`) to optionally disable the built-in UDP TURN server.
- Uses `config.ICEServer` struct arrays properly rather than unconstrained `interface{}` to type external payloads in Go.
- Adds configurations for static external ICE servers or dynamic Cloudflare TURN API credentials.
- Defaults to Metered OpenRelay `turn:openrelay.metered.ca` if enabled but unconfigured.
- Removes `any` from `src/connection/room.ts` and uses `RTCIceServer` and `RTCIceServer[]` for strict typings in TypeScript.
- Updates the frontend `Room.ts` `initWS` method and WebRTC instantiation logic to properly handle dynamic ICE server payload structures sent over WebSockets.
- Formatted Go code with `gofmt`.
- Cleaned up scratchpad files and build artifacts from workspace.
Co-authored-by: meghrathod <23073422+meghrathod@users.noreply.github.com>
This PR introduces the ability to configure external TURN servers in the Go signaling backend, which allows the application to be deployed on cloud platforms that do not support inbound UDP traffic (like Railway, Render). It also seamlessly integrates Cloudflare's TURN API if configured. The frontend logic is adjusted to gracefully accept JSON configurations instead of strings from the WebSocket handshake.
PR created automatically by Jules for task 1915659550273635745 started by @meghrathod
Note
Medium Risk
Introduces conditional TURN behavior and a new WebSocket handshake payload shape (string vs ICE server JSON), which can break connectivity if clients or configs are mismatched. Also adds outbound HTTP calls and caching for Cloudflare TURN credentials, affecting runtime behavior and reliability.
Overview
Adds support for running the signalling service without the built-in TURN server and instead returning external ICE/TURN servers to clients.
Backend now accepts new TOML config (
use_external_turn,external_ice_servers, and optional Cloudflare TURN API credentials) and, when enabled, skipsInitTurn()and sends either static ICE servers or Cloudflare-generated credentials (with a 12-hour in-memory cache) in the initial WebSocket"-1"handshake message.Frontend updates
Roomconnection logic to accept the handshake TURN data as either the legacyhost:portstring orRTCIceServer(s), and builds theRTCPeerConnectioniceServerslist accordingly.Written by Cursor Bugbot for commit 9e04894. This will update automatically on new commits. Configure here.