Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR addresses WebSocket connection management and stability issues by improving reconnection logic, error handling, and race condition prevention. It also adds fallback configuration values and health check endpoint support.
- Improved WebSocket reconnection logic to prevent multiple concurrent connections
- Fixed race condition in host assignment by checking participant count before creating records
- Added error handling and dead connection cleanup in broadcast functionality
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| frontend/src/hooks/useWebSocket.ts | Enhanced WebSocket cleanup with mounted state tracking and proper null assignment on disconnect |
| frontend/src/pages/MeetingRoom.tsx | Added localStream and chatOpen to dependency array for WebSocket message handler |
| frontend/src/config.ts | Added fallback values for API and WebSocket URLs when environment variables are not set |
| backend/main.py | Fixed race condition in host assignment, added HEAD method support for health endpoint, improved broadcast error handling, and enhanced connection cleanup |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
| useEffect(() => { | ||
| connect(); | ||
| let isMounted = true; |
There was a problem hiding this comment.
The isMounted variable is declared but never used. It's set to true on line 93 and to false on line 96 in the cleanup function, but there are no checks against this variable anywhere in the code. Either remove this unused variable or add logic to check isMounted before reconnecting in the ws.onclose handler to prevent reconnection attempts after component unmount.
| user_agent = websocket.headers.get("user-agent", "Unknown") | ||
|
|
||
| # Get or create meeting in database | ||
| meeting = None |
There was a problem hiding this comment.
The meeting variable is initialized to None before the database block and checked again after at line 234. However, if the meeting is not found, the code already returns at line 188, making the check at line 234 redundant. The initialization at line 177 and the check at lines 234-235 can be removed to simplify the code flow.
This pull request introduces several backend and frontend improvements focused on reliability, error handling, and developer experience. The backend now ensures safer participant host assignment and more robust error handling during WebSocket connections. The frontend improves WebSocket reconnection logic and provides fallback defaults for API configuration, making local development easier.
Backend reliability and error handling:
broadcast_to_meetingfunction to safely remove dead WebSocket connections, preventing issues from modifying the connection list during iteration./healthendpoint for better health checks.Frontend configuration and connection handling:
API_BASE_URLandWS_BASE_URLinconfig.ts, simplifying local development when environment variables are missing.useWebSocket.tsto prevent unwanted reconnections and memory leaks by checking component mount status and cleaning up references. [1] [2]MeetingRoom.tsxto includelocalStreamandchatOpen, ensuring correct effect execution.