feat: implement 1v1 quiz battle with real-time sync and auto-start#153
Open
hansikareddy29 wants to merge 1 commit intoOpenLake:mainfrom
Open
feat: implement 1v1 quiz battle with real-time sync and auto-start#153hansikareddy29 wants to merge 1 commit intoOpenLake:mainfrom
hansikareddy29 wants to merge 1 commit intoOpenLake:mainfrom
Conversation
🎉 Thanks for Your Contribution to CanonForces!
|
WalkthroughThe changes refactor the 1v1 quiz battle matchmaking system to use Redis queue-based operations instead of direct room creation, reorganize socket event handlers with simplified architecture, and update corresponding UI flows. The API endpoints are restructured with a new Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Socket as Socket.IO Server
participant Redis
participant Firestore
participant API as API Routes
Client->>+Socket: emit join_queue
Socket->>+Redis: spop(quiz:queue)
Redis-->>-Socket: opponentId or null
alt Opponent Found
Socket->>+API: (internally)
Socket->>+Firestore: getBankedQuestions('DSA','medium',5)
Firestore-->>-Socket: questions[]
Socket->>+Redis: set quiz:room${roomId} with questions+TTL
Redis-->>-Socket: OK
Socket->>Client: emit match_found {roomId}
Socket->>Client: (other socket) emit match_found {roomId}
Client->>+API: GET /api/quiz/questions/${roomId}
API->>+Redis: get quiz:room${roomId}
Redis-->>-API: questions JSON
API-->>-Client: {questions}
Client->>+Socket: emit join_battle {roomId}
Client->>Client: Render battle UI
Client->>Client: Answer questions, emit submit_answer
Socket->>Socket: Track opponent_update events
else No Opponent
Socket->>+Redis: sadd(quiz:queue, socket.id)
Redis-->>-Socket: OK
Socket->>Client: (client continues waiting)
end
sequenceDiagram
participant Host as Host Client
participant Joiner as Joiner Client
participant Socket as Socket.IO Server
participant Redis
participant Firestore
Host->>+Socket: emit join_private_room {roomId}
Socket->>+Redis: Store player in private room
Redis-->>-Socket: OK
Joiner->>+Socket: emit join_private_room {roomId}
Socket->>Socket: players.length >= 2, trigger countdown
Socket->>Host: emit opponent_joined
Socket->>Joiner: emit opponent_joined
Note over Host,Joiner: Countdown timer (5 seconds)
Socket->>+Firestore: getBankedQuestions('DSA','medium',5)
Firestore-->>-Socket: questions[]
Socket->>+Redis: set quiz:room${roomId} with questions+TTL
Redis-->>-Socket: OK
Socket->>Host: emit battle_starting {roomId}
Socket->>Joiner: emit battle_starting {roomId}
Host->>Host: Navigate to /quiz/battle/${roomId}
Joiner->>Joiner: Navigate to /quiz/battle/${roomId}
Estimated code review effort🎯 4 (Complex) | ⏱️ ~65 minutes Possibly related issues
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Warning Review ran into problems🔥 ProblemsTimed out fetching pipeline failures after 30000ms 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 |
|
@hansikareddy29 is attempting to deploy a commit to the aviralsaxena16's projects Team on Vercel. A member of the Team first needs to authorize it. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 9
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/pages/quiz/lobby/[roomId].tsx (1)
32-37:⚠️ Potential issue | 🟠 MajorThis lobby state is inferred locally, so the countdown isn't actually synchronized.
Every browser enters this effect with
players.length === 0, so both clients initially mark themselves as host. The second player also never receives aplayer_joinedevent for the first player, which leaves them stuck inwaitingwhile only the first client sees the countdown. Send authoritative room state (hostId,playerCount,startAt) from the server and derivewaiting/countdownfrom that instead of local guesses.Also applies to: 41-50, 60-73
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/pages/quiz/lobby/`[roomId].tsx around lines 32 - 37, The current lobby effect infers host and player list locally (using players.length === 0, setIsHost, setPlayers) which causes both clients to mark themselves host and prevents proper countdown sync; instead stop guessing and request/subscribe to authoritative room state from the server (including hostId, playerCount/players[], startAt) when joining and on server events (e.g. "room_state" or "player_joined"/"player_left"), update local state from that payload and derive waiting/countdown from room.hostId === currentUser.uid and whether startAt is set (or playerCount meets threshold), and remove the local-only host/players initialization logic so setIsHost and setPlayers are driven by the server state rather than players.length checks.
🧹 Nitpick comments (5)
src/pages/api/quiz/battle/create.ts (3)
10-10: Use201 Createdfor successful room creation.Line 10 currently returns
200;201is a better semantic match for create operations.Suggested patch
- return res.status(200).json({ roomId }); + return res.status(201).json({ roomId });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/pages/api/quiz/battle/create.ts` at line 10, Change the response status for successful room creation from 200 to 201: locate the handler that returns res.status(200).json({ roomId }) and update it to use res.status(201).json({ roomId }) so the create operation returns the proper HTTP 201 Created semantics (leave the JSON body unchanged).
6-6: SetAllowheader for 405 responses.Line 6 returns
405correctly, but addingAllow: POSTmakes the endpoint behavior explicit for clients and tooling.Suggested patch
- if (req.method !== 'POST') return res.status(405).json({ error: 'Method not allowed' }); + if (req.method !== 'POST') { + res.setHeader('Allow', 'POST'); + return res.status(405).json({ error: 'Method not allowed' }); + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/pages/api/quiz/battle/create.ts` at line 6, The 405 response branch in the API handler that checks req.method !== 'POST' should include an Allow header so clients know allowed methods; modify the branch in the create handler to set the response header (e.g., res.setHeader('Allow', 'POST') or similar) before returning res.status(405).json(...) so the endpoint explicitly advertises allowed methods for the POST-only route.
11-12: Add minimal server-side error telemetry in the catch path.Line 12 returns a good generic client error; logging the failure server-side will make production debugging much easier.
Suggested patch
} catch (error) { + console.error('Failed to create quiz lobby', { error }); return res.status(500).json({ error: 'Failed to create room' }); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/pages/api/quiz/battle/create.ts` around lines 11 - 12, The catch block in the create room route currently returns a generic 500 but doesn't record the error; update the catch path in src/pages/api/quiz/battle/create.ts to emit minimal server-side telemetry by logging the caught error and contextual info before returning res.status(500).json(...). Specifically, call your telemetry/log utility (or console.error) with the caught error variable and a short message referencing this handler (e.g., "quiz/battle create handler failed") and include any relevant request identifiers or payload if available, then keep the existing res.status(500).json({ error: 'Failed to create room' }) response.src/pages/quiz/battle/[roomId].tsx (2)
101-103: No error state for failed question fetch.If the API call fails, users will see "Loading battle..." indefinitely with no indication of the error. Consider adding an error state to display a retry option or error message.
💡 Suggested approach
+ const [error, setError] = useState<string | null>(null);In the fetch catch block:
.catch(err => { - console.error('Failed to fetch battle questions:', err); + if (err.name !== 'AbortError') { + console.error('Failed to fetch battle questions:', err); + setError('Failed to load questions. Please try again.'); + } });Then add error rendering:
+ if (error) { + return ( + <div className={styles['battle-container']}> + <p>{error}</p> + <button onClick={() => router.push('/quiz')}>Back to Quiz</button> + </div> + ); + } + if (questions.length === 0) {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/pages/quiz/battle/`[roomId].tsx around lines 101 - 103, The component currently only checks questions.length === 0 and shows "Loading battle..." so failed API calls leave users stuck; add an error state (e.g., fetchError / setFetchError) and set it in the questions fetch catch block inside this page component (the existing fetchQuestions or useEffect that populates questions), then update the render logic that uses questions.length === 0 to first check fetchError and render an error message with a retry control that calls the same fetchQuestions function (and clear fetchError before retry), ensuring successful fetch clears fetchError and updates questions.
17-17: Unuseduservariable.
useris declared but never used in this component. This creates an unnecessary context subscription. Remove if not needed, or implement the intended functionality.- const user = useContext(UserContext);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/pages/quiz/battle/`[roomId].tsx at line 17, The declared but unused variable `user` from `useContext(UserContext)` should be removed to avoid an unnecessary context subscription; locate the `const user = useContext(UserContext);` line and delete it, or if the component actually needs the current user, replace its usage by referencing `user` where required (e.g., in rendering or logic) so the variable is used; ensure all imports and references remain consistent after removing or using `user`.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/components/quiz/StartScreen.tsx`:
- Around line 57-79: After emitting 'join_queue' in handleFindMatch, ensure we
clean up abandoned matchmaking by emitting 'leave_queue' and resetting
isMatchmaking: update the useEffect that listens to socket to (1) on component
unmount return handler also emit 'leave_queue' with user.uid and
setIsMatchmaking(false), (2) add a cancel method (e.g., a new handleCancelMatch
function) that emits 'leave_queue' and sets isMatchmaking(false), and (3) listen
for socket disconnect/timeout events in the same useEffect to
setIsMatchmaking(false) and emit 'leave_queue' when appropriate; reference
socket, handleFindMatch, setIsMatchmaking, user.uid, 'join_queue' and
'leave_queue' to locate and modify the code.
In `@src/context/SocketContext.tsx`:
- Around line 13-35: The async initSocket inside the useEffect may create an
orphan socket after the effect is torn down; modify initSocket to use an
AbortController (or a local cancelled boolean) and check it after the await
fetch('/api/socket') before constructing io(), attaching listeners, or calling
setSocket/setIsConnected; also call controller.abort() (or set cancelled = true)
in the effect cleanup and ensure you disconnect any created socketInstance there
so no listeners remain after unmount.
In `@src/pages/api/socket.ts`:
- Around line 50-52: The socket handler for the 'submit_answer' event is
trusting the client's isCorrect flag; instead, accept the submitted
questionId/index and selectedOption from the client, look up the room's
server-side question set (room state or Rooms map) inside the 'submit_answer'
handler, validate the selected option against the canonical answer, compute the
score delta, enforce one submission per player per question (track submissions
in the room state or a per-player answeredQuestions set), and only then emit
'opponent_update' with the server-computed isCorrect/scoreDelta; also handle and
emit errors for invalid question ids or duplicate submissions.
- Around line 28-43: The current matchmaking flow removes an opponent from the
queue with redis.spop but if getBankedQuestions or redis.set fails the popped
opponent is lost; update the logic so after a successful spop(opponent) you wrap
the room setup (nanoid, getBankedQuestions, redis.set, emits) in its own
try/catch and if that inner block throws you re-add the popped opponent back
into the queue with redis.sadd(QUEUE_KEY, opponent) and emit an error event
(e.g., 'match_error') to both socket.id and opponent so clients aren’t left
hanging; reference redis.spop, redis.sadd, getBankedQuestions, redis.set,
ROOM_PREFIX, nanoid, socket.id and opponent when making the change.
- Around line 57-66: The server currently allows unlimited joins and any client
to trigger start_private_battle; fix by maintaining server-side per-room
metadata (host id and player count/state) in Redis (or in-memory) alongside
ROOM_PREFIX data: when handling join_private_room, if no metadata exists set
host = socket.id and count = 1 and save with TTL, otherwise fetch metadata and
reject the join (emit an error event and do not call socket.join) if count >= 2,
and if accepted increment count and save (emit player_joined to room); when
handling start_private_battle, fetch the room metadata and only proceed if
metadata.host === socket.id and metadata.count === 2 (otherwise emit a
permission or not_ready error), then atomically set the question bank into
`${ROOM_PREFIX}${roomId}` and emit battle_starting — ensure all updates use the
same room key naming and respect TTL and emit clear error events for rejected
joins/unauthorized starts.
In `@src/pages/quiz/battle/`[roomId].tsx:
- Around line 28-57: The effect in useEffect that joins the room and fetches
questions uses roomId, socket and checks isConnected but the dependency array
only lists [roomId, socket], and the fetch has no AbortController, risking stale
state updates; update the dependency array to include isConnected, create an
AbortController before fetch and pass its signal to fetch, check signal/abort in
the promise chain (or ignore setQuestions when aborted), and in the effect
cleanup call controller.abort() in addition to socket.off('opponent_update') to
ensure the fetch is cancelled and listeners removed; keep the existing
socket.on('opponent_update', ...) handler and state setters (setQuestions,
setOpponentScore, setOpponentProgress) untouched but guard them from running
after abort/unmount.
- Around line 72-79: The setTimeout started after answering (the block that
calls setIndex, setSelectedAnswer, setIsFinished) can run after the component
unmounts; store its ID (e.g., in a ref like timeoutRef) when you call setTimeout
and clearTimeout(timeoutRef.current) in a useEffect cleanup to avoid state
updates on an unmounted component. Update the handler that creates the timeout
(the function that currently calls setTimeout and updates
setIndex/setSelectedAnswer/setIsFinished) to save the ID into the ref and ensure
you clear it on unmount (and before creating a new timeout) so no stale timer
fires.
In `@src/services/quizServer.ts`:
- Around line 1-2: This server module currently imports the client web SDK (db,
collection, query, where, getDocs, writeBatch, doc) and performs reads/writes
against "quiz_bank"; change it to use the Admin SDK instance instead: import
adminDb (from src/lib/firebase_admin.ts) and replace web-SDK calls with Admin
SDK equivalents — use adminDb.collection('quiz_bank').where(...).get() for reads
(instead of query/getDocs) and use const batch = adminDb.batch(),
adminDb.doc(...) (or adminDb.collection('quiz_bank').doc(...)) and
batch.commit() for writes (instead of writeBatch/doc from the web client).
Update all usages in functions that read/write quiz_bank to reference adminDb
and its .collection/.doc/.where/.get/.batch API.
In `@src/styles/lobby.module.css`:
- Around line 194-205: The keyframe name "fadeIn" violates the project's
keyframes-name-pattern; rename the keyframe (e.g., "countdownFadeIn" or
"lobbyFadeIn") and update the reference in the .countdown-container's animation
property so the `@keyframes` block and the animation: fadeIn 0.5s ease; line use
the new name (ensure both the `@keyframes` identifier and the animation usage in
.countdown-container are changed).
---
Outside diff comments:
In `@src/pages/quiz/lobby/`[roomId].tsx:
- Around line 32-37: The current lobby effect infers host and player list
locally (using players.length === 0, setIsHost, setPlayers) which causes both
clients to mark themselves host and prevents proper countdown sync; instead stop
guessing and request/subscribe to authoritative room state from the server
(including hostId, playerCount/players[], startAt) when joining and on server
events (e.g. "room_state" or "player_joined"/"player_left"), update local state
from that payload and derive waiting/countdown from room.hostId ===
currentUser.uid and whether startAt is set (or playerCount meets threshold), and
remove the local-only host/players initialization logic so setIsHost and
setPlayers are driven by the server state rather than players.length checks.
---
Nitpick comments:
In `@src/pages/api/quiz/battle/create.ts`:
- Line 10: Change the response status for successful room creation from 200 to
201: locate the handler that returns res.status(200).json({ roomId }) and update
it to use res.status(201).json({ roomId }) so the create operation returns the
proper HTTP 201 Created semantics (leave the JSON body unchanged).
- Line 6: The 405 response branch in the API handler that checks req.method !==
'POST' should include an Allow header so clients know allowed methods; modify
the branch in the create handler to set the response header (e.g.,
res.setHeader('Allow', 'POST') or similar) before returning
res.status(405).json(...) so the endpoint explicitly advertises allowed methods
for the POST-only route.
- Around line 11-12: The catch block in the create room route currently returns
a generic 500 but doesn't record the error; update the catch path in
src/pages/api/quiz/battle/create.ts to emit minimal server-side telemetry by
logging the caught error and contextual info before returning
res.status(500).json(...). Specifically, call your telemetry/log utility (or
console.error) with the caught error variable and a short message referencing
this handler (e.g., "quiz/battle create handler failed") and include any
relevant request identifiers or payload if available, then keep the existing
res.status(500).json({ error: 'Failed to create room' }) response.
In `@src/pages/quiz/battle/`[roomId].tsx:
- Around line 101-103: The component currently only checks questions.length ===
0 and shows "Loading battle..." so failed API calls leave users stuck; add an
error state (e.g., fetchError / setFetchError) and set it in the questions fetch
catch block inside this page component (the existing fetchQuestions or useEffect
that populates questions), then update the render logic that uses
questions.length === 0 to first check fetchError and render an error message
with a retry control that calls the same fetchQuestions function (and clear
fetchError before retry), ensuring successful fetch clears fetchError and
updates questions.
- Line 17: The declared but unused variable `user` from
`useContext(UserContext)` should be removed to avoid an unnecessary context
subscription; locate the `const user = useContext(UserContext);` line and delete
it, or if the component actually needs the current user, replace its usage by
referencing `user` where required (e.g., in rendering or logic) so the variable
is used; ensure all imports and references remain consistent after removing or
using `user`.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: a99d1d24-6bc0-427b-bacf-8c79bfe7e498
📒 Files selected for processing (12)
src/components/quiz/StartScreen.tsxsrc/context/SocketContext.tsxsrc/pages/api/quiz/battle/create.tssrc/pages/api/quiz/battle/getQuestions.tssrc/pages/api/quiz/questions/[roomId].tssrc/pages/api/socket.tssrc/pages/quiz/battle/[battleId].tsxsrc/pages/quiz/battle/[roomId].tsxsrc/pages/quiz/lobby/[roomId].tsxsrc/services/quizServer.tssrc/styles/lobby.module.csstsconfig.json
💤 Files with no reviewable changes (2)
- src/pages/api/quiz/battle/getQuestions.ts
- src/pages/quiz/battle/[battleId].tsx
Comment on lines
+57
to
79
| useEffect(() => { | ||
| if (!socket) return; | ||
|
|
||
| const handleMatchFound = (roomId: string) => { | ||
| router.push(`/quiz/battle/${roomId}`); | ||
| }; | ||
|
|
||
| socket.on('match_found', handleMatchFound); | ||
|
|
||
| return () => { | ||
| socket.off('match_found', handleMatchFound); | ||
| }; | ||
| }, [socket, router]); | ||
|
|
||
| const handleFindMatch = () => { | ||
| if (!socket || !isConnected || !user) { | ||
| console.error('Socket not connected or user not logged in.'); | ||
| return; | ||
| } | ||
|
|
||
| setIsMatchmaking(true); | ||
| socket.emit('join_random_queue', user.uid); | ||
|
|
||
| socket.on('match_found', (roomId: string) => { | ||
| router.push(`/quiz/battle/${roomId}`); | ||
| }); | ||
| socket.emit('join_queue', user.uid); | ||
| }; |
There was a problem hiding this comment.
Add a dequeue path for abandoned matchmaking attempts.
Once join_queue is emitted, this component never removes the socket from matchmaking unless the socket fully disconnects. Because the socket is held in app-level context, navigating away from this screen can leave a live queue entry behind; a later match_found can then redirect the wrong user or strand the opponent in an empty room. Emit a leave_queue on unmount/cancel and reset isMatchmaking on disconnect/timeout.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/components/quiz/StartScreen.tsx` around lines 57 - 79, After emitting
'join_queue' in handleFindMatch, ensure we clean up abandoned matchmaking by
emitting 'leave_queue' and resetting isMatchmaking: update the useEffect that
listens to socket to (1) on component unmount return handler also emit
'leave_queue' with user.uid and setIsMatchmaking(false), (2) add a cancel method
(e.g., a new handleCancelMatch function) that emits 'leave_queue' and sets
isMatchmaking(false), and (3) listen for socket disconnect/timeout events in the
same useEffect to setIsMatchmaking(false) and emit 'leave_queue' when
appropriate; reference socket, handleFindMatch, setIsMatchmaking, user.uid,
'join_queue' and 'leave_queue' to locate and modify the code.
Comment on lines
+13
to
+35
| useEffect(() => { | ||
| let socketInstance: Socket | null = null; | ||
|
|
||
| // Initialize socket server by calling the API route once | ||
| const initSocket = async () => { | ||
| try { | ||
| await fetch('/api/socket'); | ||
| socketInstance = io({ path: '/api/socket', addTrailingSlash: false }); | ||
|
|
||
| socketInstance.on('connect', () => setIsConnected(true)); | ||
| socketInstance.on('disconnect', () => setIsConnected(false)); | ||
|
|
||
| setSocket(socketInstance); | ||
| } catch (err) { | ||
| console.error("Socket initialization failed:", err); | ||
| } | ||
| }; | ||
|
|
||
| initSocket(); | ||
|
|
||
| return () => { | ||
| if (socketInstance) socketInstance.disconnect(); | ||
| }; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
fd -t f "SocketContext" --extension tsxRepository: OpenLake/canonforces
Length of output: 93
🏁 Script executed:
cat -n src/context/SocketContext.tsxRepository: OpenLake/canonforces
Length of output: 1843
Guard async socket init against effect teardown.
The fetch('/api/socket') call resolves after cleanup can run. If the component unmounts during the await, cleanup sees socketInstance as null and cannot disconnect. The async continuation will then create an orphan socket with active listeners that never get cleaned up. Add a cancellation guard using an AbortController and cancelled flag to prevent socket creation and state updates after cleanup.
Suggested fix
useEffect(() => {
+ let cancelled = false;
let socketInstance: Socket | null = null;
+ const controller = new AbortController();
// Initialize socket server by calling the API route once
const initSocket = async () => {
try {
- await fetch('/api/socket');
+ const response = await fetch('/api/socket', { signal: controller.signal });
+ if (!response.ok || cancelled) return;
socketInstance = io({ path: '/api/socket', addTrailingSlash: false });
+ if (cancelled) {
+ socketInstance.disconnect();
+ return;
+ }
socketInstance.on('connect', () => setIsConnected(true));
socketInstance.on('disconnect', () => setIsConnected(false));
setSocket(socketInstance);
} catch (err) {
- console.error("Socket initialization failed:", err);
+ if (!cancelled) {
+ console.error("Socket initialization failed:", err);
+ }
}
};
initSocket();
return () => {
+ cancelled = true;
+ controller.abort();
if (socketInstance) socketInstance.disconnect();
};
}, []);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/context/SocketContext.tsx` around lines 13 - 35, The async initSocket
inside the useEffect may create an orphan socket after the effect is torn down;
modify initSocket to use an AbortController (or a local cancelled boolean) and
check it after the await fetch('/api/socket') before constructing io(),
attaching listeners, or calling setSocket/setIsConnected; also call
controller.abort() (or set cancelled = true) in the effect cleanup and ensure
you disconnect any created socketInstance there so no listeners remain after
unmount.
Comment on lines
+28
to
+43
| try { | ||
| const opponent = await redis.spop<string>(QUEUE_KEY); | ||
| if (opponent && opponent !== socket.id) { | ||
| console.log("Match found between", socket.id, "and", opponent); | ||
| const roomId = nanoid(10); | ||
| const questions = await getBankedQuestions('DSA', 'medium', 5); | ||
| await redis.set(`${ROOM_PREFIX}${roomId}`, JSON.stringify(questions), { ex: 3600 }); | ||
| io.to(socket.id).emit('match_found', roomId); | ||
| io.to(opponent).emit('match_found', roomId); | ||
| } else { | ||
| console.log("No opponent found, adding to queue:", socket.id); | ||
| await redis.sadd(QUEUE_KEY, socket.id); | ||
| } | ||
| } catch (error) { | ||
| console.error("Matchmaking error:", error); | ||
| } |
There was a problem hiding this comment.
Rollback the queue pop when room setup fails.
After spop succeeds, any failure in getBankedQuestions or redis.set just logs and returns. The popped opponent is already gone from the queue, the caller is not requeued, and neither client gets an error event, so both users can hang in matchmaking indefinitely.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/pages/api/socket.ts` around lines 28 - 43, The current matchmaking flow
removes an opponent from the queue with redis.spop but if getBankedQuestions or
redis.set fails the popped opponent is lost; update the logic so after a
successful spop(opponent) you wrap the room setup (nanoid, getBankedQuestions,
redis.set, emits) in its own try/catch and if that inner block throws you re-add
the popped opponent back into the queue with redis.sadd(QUEUE_KEY, opponent) and
emit an error event (e.g., 'match_error') to both socket.id and opponent so
clients aren’t left hanging; reference redis.spop, redis.sadd,
getBankedQuestions, redis.set, ROOM_PREFIX, nanoid, socket.id and opponent when
making the change.
Comment on lines
+50
to
+52
| socket.on('submit_answer', (roomId: string, isCorrect: boolean) => { | ||
| socket.to(roomId).emit('opponent_update', { isCorrect }); | ||
| }); |
There was a problem hiding this comment.
Don't trust isCorrect from the client.
Any client can emit submit_answer(roomId, true) repeatedly and manufacture score updates because the server never validates the answer or enforces one submission per question. The battle result needs to be server-authoritative: accept the selected option/question index, validate against the room's question set, then broadcast the score delta.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/pages/api/socket.ts` around lines 50 - 52, The socket handler for the
'submit_answer' event is trusting the client's isCorrect flag; instead, accept
the submitted questionId/index and selectedOption from the client, look up the
room's server-side question set (room state or Rooms map) inside the
'submit_answer' handler, validate the selected option against the canonical
answer, compute the score delta, enforce one submission per player per question
(track submissions in the room state or a per-player answeredQuestions set), and
only then emit 'opponent_update' with the server-computed isCorrect/scoreDelta;
also handle and emit errors for invalid question ids or duplicate submissions.
Comment on lines
+57
to
+66
| socket.on('join_private_room',async (roomId:string)=>{ | ||
| socket.join(roomId); | ||
| socket.to(roomId).emit('player_joined',socket.id); | ||
| console.log("player joined private room",roomId); | ||
| }); | ||
| socket.on('start_private_battle',async (roomId:string)=>{ | ||
| const questions=await getBankedQuestions('DSA','medium',5); | ||
| await redis.set(`${ROOM_PREFIX}${roomId}`,JSON.stringify(questions),{ex:3600}); | ||
| io.to(roomId).emit('battle_starting',roomId); | ||
| console.log(`Battle Starting in room: ${roomId}`); |
There was a problem hiding this comment.
Enforce the 1v1 room contract on the server.
join_private_room accepts unlimited sockets, and start_private_battle can be triggered by any connection that knows the roomId. That lets a guest or third party overfill a room or force-start a battle early. Track host/player count server-side, reject joins after two players, and only allow the host to start once the room is full.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/pages/api/socket.ts` around lines 57 - 66, The server currently allows
unlimited joins and any client to trigger start_private_battle; fix by
maintaining server-side per-room metadata (host id and player count/state) in
Redis (or in-memory) alongside ROOM_PREFIX data: when handling
join_private_room, if no metadata exists set host = socket.id and count = 1 and
save with TTL, otherwise fetch metadata and reject the join (emit an error event
and do not call socket.join) if count >= 2, and if accepted increment count and
save (emit player_joined to room); when handling start_private_battle, fetch the
room metadata and only proceed if metadata.host === socket.id and metadata.count
=== 2 (otherwise emit a permission or not_ready error), then atomically set the
question bank into `${ROOM_PREFIX}${roomId}` and emit battle_starting — ensure
all updates use the same room key naming and respect TTL and emit clear error
events for rejected joins/unauthorized starts.
Comment on lines
+28
to
+57
| useEffect(() => { | ||
| if (!roomId || !socket || !isConnected) return; | ||
|
|
||
| console.log("Joining battle room:", roomId); | ||
| // Join the socket room so you receive opponent updates | ||
| socket.emit('join_battle', roomId as string); | ||
|
|
||
| // Fetch the questions from the server | ||
| fetch(`/api/quiz/questions/${roomId}`) | ||
| .then(res => { | ||
| if (!res.ok) throw new Error(`HTTP error! status: ${res.status}`); | ||
| return res.json(); | ||
| }) | ||
| .then(data => { | ||
| console.log("Fetched questions:", data); | ||
| if (data.questions) setQuestions(data.questions); | ||
| else console.error("No questions in response data:", data); | ||
| }) | ||
| .catch(err => console.error('Failed to fetch battle questions:', err)); | ||
|
|
||
| // Listen for opponent answering a question | ||
| socket.on('opponent_update', ({ isCorrect }: { isCorrect: boolean }) => { | ||
| if (isCorrect) setOpponentScore(prev => prev + 1); | ||
| setOpponentProgress(prev => prev + 1); | ||
| }); | ||
|
|
||
| return () => { | ||
| socket.off('opponent_update'); | ||
| }; | ||
| }, [roomId, socket]); |
There was a problem hiding this comment.
Add missing isConnected dependency and abort controller for fetch cleanup.
-
The linter correctly identifies that
isConnectedis checked on line 29 but missing from the dependency array. If connection state changes, this effect won't re-run. -
The fetch lacks an abort controller, which can cause state updates on unmounted components.
🔧 Proposed fix
useEffect(() => {
if (!roomId || !socket || !isConnected) return;
+ const abortController = new AbortController();
+
console.log("Joining battle room:", roomId);
socket.emit('join_battle', roomId as string);
- fetch(`/api/quiz/questions/${roomId}`)
+ fetch(`/api/quiz/questions/${roomId}`, { signal: abortController.signal })
.then(res => {
if (!res.ok) throw new Error(`HTTP error! status: ${res.status}`);
return res.json();
})
.then(data => {
console.log("Fetched questions:", data);
if (data.questions) setQuestions(data.questions);
else console.error("No questions in response data:", data);
})
- .catch(err => console.error('Failed to fetch battle questions:', err));
+ .catch(err => {
+ if (err.name !== 'AbortError') {
+ console.error('Failed to fetch battle questions:', err);
+ }
+ });
socket.on('opponent_update', ({ isCorrect }: { isCorrect: boolean }) => {
if (isCorrect) setOpponentScore(prev => prev + 1);
setOpponentProgress(prev => prev + 1);
});
return () => {
+ abortController.abort();
socket.off('opponent_update');
};
- }, [roomId, socket]);
+ }, [roomId, socket, isConnected]);🧰 Tools
🪛 GitHub Check: lint
[warning] 57-57:
React Hook useEffect has a missing dependency: 'isConnected'. Either include it or remove the dependency array
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/pages/quiz/battle/`[roomId].tsx around lines 28 - 57, The effect in
useEffect that joins the room and fetches questions uses roomId, socket and
checks isConnected but the dependency array only lists [roomId, socket], and the
fetch has no AbortController, risking stale state updates; update the dependency
array to include isConnected, create an AbortController before fetch and pass
its signal to fetch, check signal/abort in the promise chain (or ignore
setQuestions when aborted), and in the effect cleanup call controller.abort() in
addition to socket.off('opponent_update') to ensure the fetch is cancelled and
listeners removed; keep the existing socket.on('opponent_update', ...) handler
and state setters (setQuestions, setOpponentScore, setOpponentProgress)
untouched but guard them from running after abort/unmount.
Comment on lines
+72
to
+79
| setTimeout(() => { | ||
| if (index + 1 < questions.length) { | ||
| setIndex(prev => prev + 1); | ||
| setSelectedAnswer(null); | ||
| } else { | ||
| setIsFinished(true); | ||
| } | ||
| }, 1000); |
There was a problem hiding this comment.
Missing cleanup for setTimeout can cause state updates on unmounted component.
If the user navigates away during the 1-second delay, setIndex, setSelectedAnswer, or setIsFinished will attempt to update state on an unmounted component. Store the timeout ID and clear it on unmount.
🔧 Proposed fix using a ref
Add a ref to track the timeout:
+ const timeoutRef = useRef<NodeJS.Timeout | null>(null);
+
+ useEffect(() => {
+ return () => {
+ if (timeoutRef.current) clearTimeout(timeoutRef.current);
+ };
+ }, []);Then in handleAnswer:
- setTimeout(() => {
+ timeoutRef.current = setTimeout(() => {
if (index + 1 < questions.length) {
setIndex(prev => prev + 1);
setSelectedAnswer(null);
} else {
setIsFinished(true);
}
}, 1000);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/pages/quiz/battle/`[roomId].tsx around lines 72 - 79, The setTimeout
started after answering (the block that calls setIndex, setSelectedAnswer,
setIsFinished) can run after the component unmounts; store its ID (e.g., in a
ref like timeoutRef) when you call setTimeout and
clearTimeout(timeoutRef.current) in a useEffect cleanup to avoid state updates
on an unmounted component. Update the handler that creates the timeout (the
function that currently calls setTimeout and updates
setIndex/setSelectedAnswer/setIsFinished) to save the ID into the ref and ensure
you clear it on unmount (and before creating a new timeout) so no stale timer
fires.
Comment on lines
+1
to
+2
| import { db } from "../lib/firebase"; | ||
| import { collection, query, where, getDocs, writeBatch, doc } from "firebase/firestore"; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify whether the shared Firebase helper is a web-SDK client and whether an admin helper still exists.
fd 'firebase\.(ts|tsx|js|jsx)$' src --exec sed -n '1,200p' {}
rg -n --type=ts --type=tsx 'firebase/firestore|firebase-admin|adminDb' srcRepository: OpenLake/canonforces
Length of output: 6225
🏁 Script executed:
# Read quizServer.ts to see the implementation
cat -n src/services/quizServer.tsRepository: OpenLake/canonforces
Length of output: 2634
🏁 Script executed:
# Search for firebase-admin imports anywhere in the codebase
rg 'firebase-admin|initializeApp.*admin|getDatabase.*admin' --type=ts --type=jsRepository: OpenLake/canonforces
Length of output: 260
🏁 Script executed:
# Check where quizServer.ts is imported from
rg 'from.*quizServer|import.*quizServer' --type=ts --type=tsx --type=jsRepository: OpenLake/canonforces
Length of output: 91
🏁 Script executed:
# Check if there's an admin SDK helper file that might exist
fd -e ts -e js | grep -i admin | head -20Repository: OpenLake/canonforces
Length of output: 89
🏁 Script executed:
# Read the firebase_admin.ts file
cat -n src/lib/firebase_admin.tsRepository: OpenLake/canonforces
Length of output: 855
🏁 Script executed:
# Check src/pages/api/quiz/save.ts to see how it uses the admin SDK
cat -n src/pages/api/quiz/save.ts | head -50Repository: OpenLake/canonforces
Length of output: 2159
Use the Admin SDK for quiz bank operations in this server-side module.
Lines 16–17 write to quiz_bank using the web-SDK client, and lines 37–45 read from it the same way. Since this module is server-only, import and use adminDb from src/lib/firebase_admin.ts instead—it's already configured and in use by src/pages/api/quiz/save.ts for the same operations. Using the web-SDK client here requires quiz_bank to be writable by unauthenticated users or fails in production.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/services/quizServer.ts` around lines 1 - 2, This server module currently
imports the client web SDK (db, collection, query, where, getDocs, writeBatch,
doc) and performs reads/writes against "quiz_bank"; change it to use the Admin
SDK instance instead: import adminDb (from src/lib/firebase_admin.ts) and
replace web-SDK calls with Admin SDK equivalents — use
adminDb.collection('quiz_bank').where(...).get() for reads (instead of
query/getDocs) and use const batch = adminDb.batch(), adminDb.doc(...) (or
adminDb.collection('quiz_bank').doc(...)) and batch.commit() for writes (instead
of writeBatch/doc from the web client). Update all usages in functions that
read/write quiz_bank to reference adminDb and its
.collection/.doc/.where/.get/.batch API.
Comment on lines
+194
to
+205
| .countdown-container { | ||
| display: flex; | ||
| flex-direction: column; | ||
| align-items: center; | ||
| gap: 1rem; | ||
| animation: fadeIn 0.5s ease; | ||
| } | ||
|
|
||
| @keyframes fadeIn { | ||
| from { opacity: 0; transform: translateY(10px); } | ||
| to { opacity: 1; transform: translateY(0); } | ||
| } |
There was a problem hiding this comment.
Rename this keyframe to satisfy the lint rule.
fadeIn violates the configured keyframes-name-pattern, so this stylesheet will keep failing stylelint.
Suggested fix
.countdown-container {
display: flex;
flex-direction: column;
align-items: center;
gap: 1rem;
- animation: fadeIn 0.5s ease;
+ animation: fade-in 0.5s ease;
}
-@keyframes fadeIn {
+@keyframes fade-in {
from { opacity: 0; transform: translateY(10px); }
to { opacity: 1; transform: translateY(0); }
}🧰 Tools
🪛 Stylelint (17.5.0)
[error] 202-202: Expected keyframe name "fadeIn" to be kebab-case (keyframes-name-pattern)
(keyframes-name-pattern)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/styles/lobby.module.css` around lines 194 - 205, The keyframe name
"fadeIn" violates the project's keyframes-name-pattern; rename the keyframe
(e.g., "countdownFadeIn" or "lobbyFadeIn") and update the reference in the
.countdown-container's animation property so the `@keyframes` block and the
animation: fadeIn 0.5s ease; line use the new name (ensure both the `@keyframes`
identifier and the animation usage in .countdown-container are changed).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
🚀 Real-Time 1v1 Quiz Battle System
✅ Features Implemented
1. Real-time Infrastructure
/api/socketto manage:2. Private Lobby & Invite System
/quiz/lobby/[roomId]) to invite friends.3. Automatic Battle Flow
4. Battle Experience
🛠 Bug Fixes & Improvements
roomIdguard during lobby initialization.BattlePageto prevent crashes while questions are loading.🚧 Remaining Work
Pending Features
UI & Technical Improvements
📌 Summary
This PR lays the foundation for the real-time multiplayer quiz system including private lobbies, automatic matchmaking flow, synchronized battles, and Redis-based question caching. Remaining work mainly involves leaderboard features, timers, and invite link fixes.