Skip to content

Fixes ICE candidate issue #3

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.

Sign up for GitHub

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
martian56 merged 1 commit into from
Nov 7, 2025
Merged

Fixes ICE candidate issue #3

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 21 additions & 12 deletions backend/config.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,12 +1,18 @@
import os
import re
from pydantic_settings import BaseSettings
from pydantic_settings import BaseSettings, SettingsConfigDict
from dotenv import load_dotenv

load_dotenv()


class Settings(BaseSettings):
model_config = SettingsConfigDict(
env_file=".env",
case_sensitive=False,
extra="ignore" # Ignore extra environment variables like CORS_ORIGINS
Copy link

Copilot AI Nov 7, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The comment suggests CORS_ORIGINS is an extra environment variable to be ignored, but it's actually being used by the cors_origins_str property method via os.getenv(). This comment is misleading since CORS_ORIGINS is a required configuration, not an extra variable to ignore.

Suggested change
extra="ignore" # Ignore extra environment variables like CORS_ORIGINS
extra="ignore" # Ignore extra environment variables not defined in the model

Copilot uses AI. Check for mistakes.
)

# Database
database_url: str = os.getenv("DATABASE_URL", "postgresql://localhost/videocall")

Expand All @@ -18,20 +24,23 @@ def async_database_url(self) -> str:
# Security
secret_key: str = os.getenv("SECRET_KEY", "dev-secret-key-change-in-production")

# CORS
cors_origins: list[str] = [
origin.strip()
for origin in os.getenv("CORS_ORIGINS", "http://localhost:5173,https://linkup.ufazien.com").split(",")
if origin.strip()
]
@property
def cors_origins_str(self) -> str:
"""Get CORS origins as string from environment"""
return os.getenv("CORS_ORIGINS", "http://localhost:5173,https://linkup.ufazien.com")

@property
def cors_origins(self) -> list[str]:
"""Get CORS origins as a list"""
return [
origin.strip()
for origin in self.cors_origins_str.split(",")
if origin.strip()
]

# Application
app_name: str = "VideoCall"
app_name: str = "LinkUp"
debug: bool = os.getenv("DEBUG", "False").lower() == "true"

class Config:
env_file = ".env"
case_sensitive = False


settings = Settings()
Expand Down
27 changes: 25 additions & 2 deletions frontend/src/hooks/useWebRTC.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,9 +41,12 @@ export const useWebRTC = (options?: UseWebRTCOptions) => {

// Add local stream tracks
if (localStreamRef.current) {
console.log('Adding local tracks to peer connection for:', clientId);
localStreamRef.current.getTracks().forEach((track) => {
pc.addTrack(track, localStreamRef.current!);
});
} else {
console.warn('No local stream available when creating peer connection for:', clientId);
}

// Handle remote stream
Expand Down Expand Up @@ -96,15 +99,35 @@ export const useWebRTC = (options?: UseWebRTCOptions) => {
}, [createPeerConnection]);

const handleOffer = useCallback(async (clientId: string, offer: RTCSessionDescriptionInit): Promise<RTCSessionDescriptionInit | null> => {
const pc = createPeerConnection(clientId);
// Check if we already have a peer connection for this client
let pc = peerConnectionsRef.current.get(clientId);
if (!pc) {
console.log('Creating new peer connection for offer from:', clientId);
pc = createPeerConnection(clientId);
} else {
console.log('Reusing existing peer connection for offer from:', clientId);
// If remote description is already set, we might be handling a duplicate offer
if (pc.remoteDescription) {
console.warn('Remote description already set for:', clientId, 'current state:', pc.signalingState);
// If we're in a state where we can set it again, continue, otherwise return
if (pc.signalingState === 'stable') {
// We can set it again
} else {
console.error('Cannot set remote description, signaling state:', pc.signalingState);
return null;
}
}
Comment on lines +113 to +119
Copy link

Copilot AI Nov 7, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The empty if block when pc.signalingState === 'stable' serves no purpose and reduces code clarity. If the condition is meant to allow fall-through to the try block below, consider removing the empty branch and inverting the logic:

if (pc.remoteDescription && pc.signalingState !== 'stable') {
  console.error('Cannot set remote description, signaling state:', pc.signalingState);
  return null;
}

This makes the intent clearer and eliminates the confusing empty block.

Copilot uses AI. Check for mistakes.
}

try {
await pc.setRemoteDescription(new RTCSessionDescription(offer));
console.log('Set remote description for:', clientId, 'signaling state:', pc.signalingState);
const answer = await pc.createAnswer();
await pc.setLocalDescription(answer);
console.log('Created and set local answer for:', clientId);
return answer;
} catch (error) {
console.error('Error handling offer:', error);
console.error('Error handling offer:', error, 'signaling state:', pc?.signalingState);
return null;
}
}, [createPeerConnection]);
Expand Down
11 changes: 9 additions & 2 deletions frontend/src/hooks/useWebSocket.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,12 @@ export const useWebSocket = (
const [error, setError] = useState<Error | null>(null);
const wsRef = useRef<WebSocket | null>(null);
const reconnectTimeoutRef = useRef<ReturnType<typeof setTimeout> | undefined>(undefined);
const onMessageRef = useRef(onMessage);

// Keep the ref updated without causing reconnections
useEffect(() => {
onMessageRef.current = onMessage;
}, [onMessage]);

const connect = useCallback(() => {
try {
Expand All @@ -51,7 +57,8 @@ export const useWebSocket = (
try {
const message: WSMessage = JSON.parse(event.data);
setLastMessage(message);
onMessage?.(message);
// Use ref to avoid dependency issues
onMessageRef.current?.(message);
} catch (err) {
console.error('Error parsing WebSocket message:', err);
}
Expand All @@ -77,7 +84,7 @@ export const useWebSocket = (
console.error('Error creating WebSocket:', err);
setError(err as Error);
}
}, [meetingCode, clientId, onMessage]);
}, [meetingCode, clientId]);

useEffect(() => {
connect();
Expand Down
82 changes: 51 additions & 31 deletions frontend/src/pages/MeetingRoom.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,11 +51,14 @@ export default function MeetingRoom() {
} = useWebRTC({
onIceCandidate: (clientId: string, candidate: RTCIceCandidate) => {
if (sendMessageRef.current) {
console.log('Sending ICE candidate to:', clientId);
sendMessageRef.current({
type: WSMessageType.ICE_CANDIDATE,
target: clientId,
data: candidate.toJSON(),
});
} else {
console.warn('sendMessageRef is null, cannot send ICE candidate to:', clientId);
Comment on lines +54 to +61
Copy link

Copilot AI Nov 7, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Excessive debug logging has been added throughout the WebRTC connection flow. While useful for development, this amount of console logging in production code can:

  1. Impact performance with frequent console operations
  2. Clutter production logs making it harder to find actual issues
  3. Potentially expose sensitive connection details in production logs

Consider either:

  • Using a proper logging library with configurable log levels
  • Wrapping these logs in a debug flag check
  • Removing non-critical logs before production deployment

Copilot uses AI. Check for mistakes.
}
},
});
Expand All @@ -78,22 +81,38 @@ export default function MeetingRoom() {

// WebSocket message handler
const handleWebSocketMessage = useCallback((message: WSMessage) => {
console.log('Received WebSocket message:', message.type);
console.log('Received WebSocket message:', message.type, 'Full message:', message);

switch (message.type) {
case WSMessageType.USER_JOINED:
if (message.clientId && message.clientId !== clientId) {
setOtherParticipants((prev) => [...prev, message.clientId!]);
// Create offer to new participant
createOffer(message.clientId).then((offer) => {
if (offer && sendMessageRef.current) {
sendMessageRef.current({
type: WSMessageType.OFFER,
target: message.clientId,
data: offer,
});
console.log('User joined:', message.clientId);
setOtherParticipants((prev) => {
if (!prev.includes(message.clientId!)) {
return [...prev, message.clientId!];
}
return prev;
});
// Create offer to new participant only if we don't already have a connection
if (!hasPeerConnection(message.clientId) && sendMessageRef.current) {
console.log('Creating offer to:', message.clientId);
createOffer(message.clientId).then((offer) => {
if (offer && sendMessageRef.current) {
console.log('Sending offer to:', message.clientId);
sendMessageRef.current({
type: WSMessageType.OFFER,
target: message.clientId,
data: offer,
});
} else {
console.error('Failed to create offer or sendMessageRef is null');
}
}).catch((error) => {
console.error('Error creating offer:', error);
});
} else {
console.log('Already have peer connection with:', message.clientId);
}
}
break;

Expand All @@ -114,52 +133,49 @@ export default function MeetingRoom() {
const others = message.participants.filter((id: string) => id !== clientId);
setOtherParticipants(others);

// Create offers to existing participants when we first join
// Use hasPeerConnection to avoid creating duplicate offers
if (!isInitialized && others.length > 0) {
others.forEach((otherId: string) => {
// Only create offer if we don't already have a peer connection
// This prevents duplicate offers if we already received one from this peer
if (!hasPeerConnection(otherId) && sendMessageRef.current) {
createOffer(otherId).then((offer) => {
if (offer && sendMessageRef.current) {
sendMessageRef.current({
type: WSMessageType.OFFER,
target: otherId,
data: offer,
});
}
});
}
});
}
// When we first join, we receive the list of existing participants
// We should wait for existing participants to send us offers (via USER_JOINED on their side)
// Don't create offers here - let existing participants initiate via USER_JOINED
setIsInitialized(true);
}
break;

case WSMessageType.OFFER:
if (message.from && message.data) {
console.log('Received offer from:', message.from, 'localStream ready:', !!localStream);
// Handle offer - the useWebRTC hook will check if local stream is ready
handleOffer(message.from, message.data).then((answer) => {
if (answer && sendMessageRef.current) {
console.log('Sending answer to:', message.from);
sendMessageRef.current({
type: WSMessageType.ANSWER,
target: message.from,
data: answer,
});
} else {
console.error('Failed to create answer or sendMessageRef is null');
}
}).catch((error) => {
console.error('Error handling offer:', error);
});
}
break;

case WSMessageType.ANSWER:
if (message.from && message.data) {
handleAnswer(message.from, message.data);
console.log('Received answer from:', message.from);
handleAnswer(message.from, message.data).catch((error) => {
console.error('Error handling answer:', error);
});
}
break;

case WSMessageType.ICE_CANDIDATE:
if (message.from && message.data) {
handleRemoteIceCandidate(message.from, message.data);
console.log('Received ICE candidate from:', message.from);
handleRemoteIceCandidate(message.from, message.data).catch((error) => {
console.error('Error handling ICE candidate:', error);
});
}
break;

Expand Down Expand Up @@ -208,6 +224,10 @@ export default function MeetingRoom() {
});
}
break;

default:
console.warn('Unhandled WebSocket message type:', message.type, message);
break;
}
}, [clientId, createOffer, handleOffer, handleAnswer, handleRemoteIceCandidate, removePeer, hasPeerConnection, isInitialized]);

Expand Down
Loading