Skip to content

🔧 Refactor: Improve WebSocket error handling, reliability, and recovery #310

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

Open
SoorejS wants to merge 5 commits into
base: main
Choose a base branch
from
Open

🔧 Refactor: Improve WebSocket error handling, reliability, and recovery #310

Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
4de47c5
Fix: improve WebSocket error handling and recovery
SoorejS Feb 8, 2026
5a39ce6
fix(ws): improve WebSocket error handling and lifecycle management
SoorejS Mar 13, 2026
446cd10
fix(ws): undo previous file change
SoorejS Mar 13, 2026
c442354
fix(ws): coderabbitai suggestions considered 1
SoorejS Mar 13, 2026
63069ef
fix(ws): coderabbitai suggestions considered 2 and remove retry logic…
SoorejS Mar 13, 2026
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
131 changes: 104 additions & 27 deletions frontend/src/hooks/useDebateWS.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,10 +10,13 @@ import {
presenceAtom,
spectatorHashAtom,
lastEventIdAtom,
wsErrorAtom, // <-- NEW
PollInfo,
} from '../atoms/debateAtoms';
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Comment on lines +13 to 15

@coderabbitai coderabbitai Bot Mar 13, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Verify that wsErrorAtom is exported from debateAtoms.ts

ATOM_FILE=$(fd 'debateAtoms\.ts$' | head -n1)
echo "=== Checking for wsErrorAtom in ${ATOM_FILE} ==="

# Check for export declaration
rg -n 'export\s+(const\s+)?wsErrorAtom' "${ATOM_FILE}" || echo "No direct export found"

# Check for atom declaration (might be exported elsewhere)
rg -n 'wsErrorAtom' "${ATOM_FILE}" || echo "wsErrorAtom not found in file"

Repository: AOSSIE-Org/DebateAI

Length of output: 185

wsErrorAtom is not exported from debateAtoms.ts.

The import at line 13 will fail at runtime. Add the export to frontend/src/atoms/debateAtoms.ts:

export const wsErrorAtom = atom<string | null>(null);

Or verify that wsErrorAtom is defined elsewhere if this import path is incorrect.

🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@frontend/src/hooks/useDebateWS.ts` around lines 13 - 15, The import of
wsErrorAtom in useDebateWS.ts fails because wsErrorAtom is not exported from
debateAtoms; add an export for wsErrorAtom (e.g., export const wsErrorAtom =
atom<string | null>(null)) inside the module that defines the debate atoms
(debateAtoms.ts) or, if wsErrorAtom is defined elsewhere, correct the import
path in useDebateWS.ts to point to the module that actually exports wsErrorAtom;
ensure the symbol name matches exactly and that the module exports it so
useDebateWS.ts can import wsErrorAtom successfully.

import ReconnectingWebSocket from 'reconnecting-websocket';

const ERROR_THROTTLE_MS = 5000;

interface Event {
type: string;
payload: any;
Expand All @@ -30,7 +33,10 @@ export const useDebateWS = (debateId: string | null) => {
const [, setPresence] = useAtom(presenceAtom);
const [, setLastEventId] = useAtom(lastEventIdAtom);
const [spectatorHash] = useAtom(spectatorHashAtom);
const [, setWsError] = useAtom(wsErrorAtom);

const wsRef = useRef<ReconnectingWebSocket | null>(null);
const lastErrorTime = useRef(0);

useEffect(() => {
if (!debateId) return;
Expand Down Expand Up @@ -62,11 +68,12 @@ export const useDebateWS = (debateId: string | null) => {
}

setWsStatus('connecting');
setWsError(null); // clear old errors when reconnecting

// Get WebSocket URL
const protocol = window.location.protocol === 'https:' ? 'wss:' : 'ws:';
const apiUrl = import.meta.env.VITE_API_URL;
let host = window.location.host;

if (apiUrl) {
try {
host = new URL(apiUrl).host;
Expand All @@ -87,7 +94,7 @@ export const useDebateWS = (debateId: string | null) => {

const rws = new ReconnectingWebSocket(wsUrl, [], {
connectionTimeout: 4000,
maxRetries: Infinity,
maxRetries: 3,
maxReconnectionDelay: 10000,
minReconnectionDelay: 1000,
reconnectionDelayGrowFactor: 1.3,
Expand All @@ -97,23 +104,48 @@ export const useDebateWS = (debateId: string | null) => {
setWs(rws as unknown as WebSocket);
let ownsConnection = true;

const handleError = (error: unknown, context = 'WebSocket error') => {
const now = Date.now();
if (now - lastErrorTime.current > ERROR_THROTTLE_MS) { // 5 seconds throttle
const errorMessage = error instanceof Error
? `${context}: ${error.message}`
: `${context}: An unknown error occurred`;

console.error(errorMessage, error);
setWsError(errorMessage);
setWsStatus('error'); // Move inside throttle guard
lastErrorTime.current = now;
}
};

rws.onopen = () => {
// Reset retry counter on successful connection
setWsStatus('connected');
setWsError(null);

const spectatorHashValue = spectatorHash || localStorage.getItem('spectatorHash') || '';

const spectatorHashValue =
spectatorHash || localStorage.getItem('spectatorHash') || '';
const joinMessage = {
type: 'join',
payload: {
spectatorHash: spectatorHashValue,
},
};
rws.send(JSON.stringify(joinMessage));
try {
const joinMessage = {
type: 'join',
payload: { spectatorHash: spectatorHashValue },
};
rws.send(JSON.stringify(joinMessage));
} catch (error) {
handleError(error, 'Failed to send join message');
}
};

rws.onmessage = (event) => {
try {
if (!event.data) {
throw new Error('Received empty message');
}

const eventData: Event = JSON.parse(event.data);

// Clear error state on successful message
setWsError(null);

if (eventData.type !== 'poll_snapshot' && eventData.timestamp) {
setLastEventId(String(eventData.timestamp));
Expand All @@ -123,36 +155,45 @@ export const useDebateWS = (debateId: string | null) => {
case 'poll_snapshot': {
const payload = eventData.payload || {};
const pollsPayload = payload.polls;

if (Array.isArray(pollsPayload)) {
const nextState: Record<string, PollInfo> = {};

pollsPayload.forEach((poll) => {
if (!poll) return;

const pollId =
typeof poll.pollId === 'string'
? poll.pollId
: String(poll.pollId ?? '');

if (!pollId) return;

const countsRaw = poll.counts || {};
const counts: Record<string, number> = {};

if (countsRaw && typeof countsRaw === 'object') {
Object.entries(countsRaw).forEach(([option, value]) => {
const numericValue =
counts[option] =
typeof value === 'number'
? value
: Number(value ?? 0) || 0;
counts[option] = numericValue;
});
}

let options: string[] = [];

if (Array.isArray(poll.options)) {
options = poll.options
.map((opt: unknown) => String(opt ?? '').trim())
.filter((opt: string) => opt.length > 0);
}

if (options.length === 0) {
options = Object.keys(counts);
}
const info: PollInfo = {

nextState[pollId] = {
pollId,
question:
typeof poll.question === 'string' ? poll.question : '',
Expand All @@ -163,18 +204,20 @@ export const useDebateWS = (debateId: string | null) => {
? poll.voters
: Number(poll.voters ?? 0) || 0,
};
nextState[pollId] = info;
});

setPollState(nextState);
} else if (payload.pollState) {
// Backwards compatibility: convert legacy structure
const legacyState = payload.pollState as Record<
string,
Record<string, number>
>;

const legacyResult: Record<string, PollInfo> = {};

Object.entries(legacyState).forEach(([pollId, counts]) => {
const options = Object.keys(counts || {});

legacyResult[pollId] = {
pollId,
question: '',
Expand All @@ -186,6 +229,7 @@ export const useDebateWS = (debateId: string | null) => {
: 0,
};
});

setPollState(legacyResult);
}
break;
Expand All @@ -195,11 +239,14 @@ export const useDebateWS = (debateId: string | null) => {
setPollState((prev) => {
const pollId = eventData.payload?.pollId;
const option = eventData.payload?.option;

if (typeof pollId !== 'string' || typeof option !== 'string') {
return prev;
}

const nextState = { ...prev };
const existing = nextState[pollId];

if (!existing) {
nextState[pollId] = {
pollId,
Expand All @@ -210,44 +257,54 @@ export const useDebateWS = (debateId: string | null) => {
};
return nextState;
}

const nextCounts = { ...existing.counts };
nextCounts[option] = (nextCounts[option] || 0) + 1;

const nextOptions = existing.options.includes(option)
? existing.options
: [...existing.options, option];

nextState[pollId] = {
...existing,
options: nextOptions,
counts: nextCounts,
};

return nextState;
});
break;

case 'poll_created': {
const poll = eventData.payload;

if (poll && poll.pollId) {
setPollState((prev) => {
const pollId = String(poll.pollId);
const countsRaw = poll.counts || {};
const counts: Record<string, number> = {};

Object.entries(countsRaw).forEach(([option, value]) => {
counts[option] =
typeof value === 'number'
? value
: Number(value ?? 0) || 0;
});

const options = Array.isArray(poll.options)
? poll.options
.map((opt: unknown) => String(opt ?? '').trim())
.filter((opt: string) => opt.length > 0)
: Object.keys(counts);

return {
...prev,
[pollId]: {
pollId,
question:
typeof poll.question === 'string' ? poll.question : '',
typeof poll.question === 'string'
? poll.question
: '',
options,
counts,
voters:
Expand Down Expand Up @@ -293,29 +350,49 @@ export const useDebateWS = (debateId: string | null) => {
default:
}
} catch (error) {
handleError(error, 'Error processing WebSocket message');
}
};

rws.onerror = () => {
setWsStatus('error');
rws.onerror = (error) => {
handleError(error, 'WebSocket connection error');
};

rws.onclose = () => {
setWsStatus('disconnected');
rws.onclose = (event) => {
// Don't treat normal closure as error
if (event.code !== 1000) { // 1000 is normal closure
handleError(new Error(`Connection closed with code ${event.code}: ${event.reason || 'Unknown reason'}`));
} else {
setWsStatus('disconnected');
setWsError(null);
}

setWs(null);

if (wsRef.current === rws) {
wsRef.current = null;
}
};

// Cleanup function
return () => {
if (ownsConnection) {
rws.close();
if (wsRef.current === rws) {
wsRef.current = null;
try {
if (rws.readyState === WebSocket.OPEN) {
rws.close(1000, 'Component unmounting');
} else {
rws.close();
}
} catch (error) {
console.error('Error during WebSocket cleanup:', error);
} finally {
if (wsRef.current === rws) {
wsRef.current = null;
}
setWs(null);
setWsStatus('disconnected');
setWsError(null);
}
setWs(null);
setWsStatus('disconnected');
}
ownsConnection = false;
};
Expand All @@ -331,6 +408,7 @@ export const useDebateWS = (debateId: string | null) => {
setWsStatus,
setLastEventId,
setPresence,
setWsError,
]);

const sendMessage = (type: string, payload: any) => {
Expand All @@ -345,4 +423,3 @@ export const useDebateWS = (debateId: string | null) => {
ws: wsRef.current,
};
};