Fix client reconnection after page refresh#3117
Conversation
WalkthroughAdd persistent-ID–based reconnection: server maps persistentIDs to clientIDs, orchestrates socket/listener swap on reconnect, records kicked persistentIDs, and assigns clientIDs in lobby_info; clients stop sending clientID and host creation uses JWT (Authorization) for creator identity. Changes
Sequence DiagramsequenceDiagram
participant Client
participant Worker
participant GameServer
participant GameState
Client->>Worker: WS CONNECT (includes token / persistentID)
Worker->>GameServer: getOrCreateClientId(gameID, persistentID)
alt persistentID mapped (reconnect)
Worker->>GameServer: reconnectExistingClient(persistentID, ws)
GameServer->>GameServer: close old ws, swap socket, update lastPing, mark connected
GameServer->>GameServer: reattach listeners
GameServer->>Client: send lobby_info (yourClientID)
alt game already started
GameServer->>GameState: request last turn/state
GameState-->>GameServer: state
GameServer->>Client: send start-game + state
end
else new client
GameServer->>GameServer: create new clientID, map persistentID→clientID
GameServer->>Client: send lobby_info (yourClientID)
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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 |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@src/server/GameServer.ts`:
- Around line 162-186: The reconnect branch in joinClient skips the persistentID
check that rejoinClient enforces, allowing session hijack; update the reconnect
path in joinClient to validate that incoming client.persistentID matches
existingClient.persistentID, and if it does not, log a warning and close/reject
the new socket (do not replace existingClient.ws or add to activeClients); if it
matches, continue with updating existingClient.ws, lastPing,
markClientDisconnected(..., false), addListeners(existingClient) and use
existingClient.ws when calling sendStartGameMsg.
- Around line 169-171: When replacing an existing client's WebSocket (the code
that sets existingClient.ws = client.ws and updates existingClient.lastPing),
first grab the previous WebSocket (e.g., const previousWs = existingClient.ws),
and if it exists and is not already CLOSED/closing, call previousWs.close()
(optionally with a normal close code) to avoid leaking the old connection; do
NOT call this.websockets.delete(previousWs) — leave Set cleanup to the class
end() method. Ensure you then assign existingClient.ws = client.ws and update
lastPing as before.
🧹 Nitpick comments (1)
src/server/GameServer.ts (1)
182-184: UseexistingClient.wsfor clarity.After line 170, both
client.wsandexistingClient.wspoint to the same WebSocket. UsingexistingClient.wshere would be more consistent and easier to follow sinceexistingClientis what's added toactiveClients.✏️ Suggested change
// Send start message if game has already started if (this._hasStarted) { - this.sendStartGameMsg(client.ws, 0); + this.sendStartGameMsg(existingClient.ws, 0); }
There was a problem hiding this comment.
Pull request overview
This PR implements client reconnection functionality after page refresh by modifying the joinClient method in GameServer.ts. Previously, when a client attempted to rejoin with the same clientID, a warning was logged and the client was rejected. The new code now handles reconnection by updating the WebSocket reference, restoring the client's connected status, and re-sending game state if needed.
Changes:
- Modified
joinClientto detect and handle reconnection scenarios when a client with the same clientID already exists - Added logic to update WebSocket references, mark client as connected, restore to activeClients, and resend game start information
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
f5aea08 to
5479459
Compare
There was a problem hiding this comment.
this logic looks pretty simiilar to rejoin. maybe instead we have the client call rejoin if it has already joined the server once?
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/client/HostLobbyModal.ts (1)
638-666:⚠️ Potential issue | 🟡 MinorSet lobbyCreatorClientID after joining.
It now stays empty, so lobby-player-view cannot show the host or “you”. Consider setting it from the first client returned in pollPlayers (same as JoinLobbyModal) or from lobby_info.
✅ Suggested fix (pollPlayers)
- .then((data: GameInfo) => { - this.clients = data.clients ?? []; - }); + .then((data: GameInfo) => { + this.clients = data.clients ?? []; + if (!this.lobbyCreatorClientID && this.clients.length > 0) { + this.lobbyCreatorClientID = this.clients[0].clientID; + } + });src/server/Worker.ts (1)
1-1:⚠️ Potential issue | 🟡 MinorFix Prettier formatting before merge.
The CI pipeline reports formatting issues in this file. Run
npx prettier --write src/server/Worker.tsto fix.
🤖 Fix all issues with AI agents
In `@src/client/ClientGameRunner.ts`:
- Around line 85-98: The file fails Prettier formatting; run the project's
formatter and commit the formatted changes to satisfy CI. Open
src/client/ClientGameRunner.ts (look for symbols like onconnect, onmessage,
terrainLoad, lobbyConfig) and apply the project's Prettier configuration (or run
npm/yarn script such as "npm run format" or "yarn format") to reformat the file,
then stage and push the updated file so the prettier check passes.
In `@src/client/JoinLobbyModal.ts`:
- Around line 346-350: startTrackingLobby currently clears this.currentClientID
and nothing sets it afterward; locate the handler that processes the server's
lobby_info message (or wherever yourClientID is received) and assign that value
into this.currentClientID (e.g., set this.currentClientID = message.yourClientID
or the equivalent field) so the lobby-player-view can identify the local player;
ensure the assignment happens after startTrackingLobby is called and consider
emitting/updating any dependent state or view update methods so the UI
re-renders with the new client id.
| const onconnect = () => { | ||
| if (hasJoined) { | ||
| console.log("rejoining game"); | ||
| transport.rejoinGame(0); | ||
| } else { | ||
| hasJoined = true; | ||
| console.log(`Joining game lobby ${lobbyConfig.gameID}`); | ||
| transport.joinGame(); | ||
| } | ||
| // Always send join - server will detect reconnection via persistentID | ||
| console.log(`Joining game lobby ${lobbyConfig.gameID}`); | ||
| transport.joinGame(); | ||
| }; | ||
| let terrainLoad: Promise<TerrainMapData> | null = null; | ||
|
|
||
| const onmessage = (message: ServerMessage) => { | ||
| if (message.type === "lobby_info") { | ||
| // Server tells us our assigned clientID | ||
| if (message.yourClientID) { | ||
| lobbyConfig.clientID = message.yourClientID; | ||
| console.log(`Received server-assigned clientID: ${message.yourClientID}`); | ||
| } |
There was a problem hiding this comment.
Prettier check failed for this file.
Please run the formatter so CI passes.
🤖 Prompt for AI Agents
In `@src/client/ClientGameRunner.ts` around lines 85 - 98, The file fails Prettier
formatting; run the project's formatter and commit the formatted changes to
satisfy CI. Open src/client/ClientGameRunner.ts (look for symbols like
onconnect, onmessage, terrainLoad, lobbyConfig) and apply the project's Prettier
configuration (or run npm/yarn script such as "npm run format" or "yarn format")
to reformat the file, then stage and push the updated file so the prettier check
passes.
| private startTrackingLobby(lobbyId: string, lobbyInfo?: GameInfo) { | ||
| this.currentLobbyId = lobbyId; | ||
| this.currentClientID = getClientIDForGame(lobbyId); | ||
| // clientID will be assigned by server via lobby_info message | ||
| this.currentClientID = ""; | ||
| this.gameConfig = null; |
There was a problem hiding this comment.
Update currentClientID once the server assigns it.
currentClientID is reset to empty and never updated, so lobby-player-view cannot mark the local player. Please wire yourClientID from lobby_info (or another shared state) into currentClientID.
🤖 Prompt for AI Agents
In `@src/client/JoinLobbyModal.ts` around lines 346 - 350, startTrackingLobby
currently clears this.currentClientID and nothing sets it afterward; locate the
handler that processes the server's lobby_info message (or wherever yourClientID
is received) and assign that value into this.currentClientID (e.g., set
this.currentClientID = message.yourClientID or the equivalent field) so the
lobby-player-view can identify the local player; ensure the assignment happens
after startTrackingLobby is called and consider emitting/updating any dependent
state or view update methods so the UI re-renders with the new client id.
Description:
Fix client reconnection after page refresh
Please complete the following:
Please put your Discord username so you can be contacted if a bug or regression is found:
w.o.n