Fix: player session never recovers when server resets by karyao · Pull Request #18 · CSSS/pacmacro2

karyao · 2026-08-19T01:43:38Z

Closes #12

Description:

Server restarting previously left players stuck on "Reconnecting.,,"because the browser still had player ID cookie, but the restarted server no longer had the corresponding player session. To fix this:

on graceful shutdown, server notifies connected clients before exiting. Players are returned to the registration page, old player ID is cleared, and previous entered name is pre-filled.
if a player's websocket connection fails 3 times, the client treats the saved player session as expired automatically re-registers the player again.
if automatic registration fails, then old credentials are cleared and players are sent to the registration page.
shutdown and critical socket updates are prioritized so they are not blocked behind queued map updates.

… connecting fails 3 times

karyao · 2026-08-19T09:32:34Z

@jbriones1

jbriones1

The test comments can probably be ignored, but when tests are sus I'm sus of the code.

jbriones1 · 2026-08-20T00:54:22Z

+  });
+
+  it('re-registers with the saved name and reconnects', async () => {
+    playerName.get.mockReturnValue('Odin');


Doxxing my son.

jbriones1 · 2026-08-20T01:34:06Z

+
+    const startCalls = gameSocket.start.mock.calls;
+    const onConnected = startCalls[startCalls.length - 1][1] as () => void;
+    expect(startCalls[startCalls.length - 1][0]).toBe('NEWID');


nit: this doesn't check the third argument of the start function. In the future, if someone refactors and messes up and doesn't add a third parameter on reconnect this test would fail.

jbriones1 · 2026-08-20T02:17:22Z


+  protected override shouldReconnect(closeEvent: CloseEvent): boolean {
+    if (this.mode !== 'player') {
+      return true;


I don't think players should reconnect if the closeEvent was graceful (status 1001). I don't think our server is capable of having a good shutdown, so maybe this could be added.

With this logic, if server is shut down and someone never closed their browser tab, they would reconnect if the server was turned back and they opened their tab. Would be kinda weird.

Server shutdown now sends an explicit shutdown command soclient can now distinguish between intentional game shutdown from the other 1001 graceful closes

karyao · 2026-08-20T05:25:43Z

Will take a look at how to get the server to explicitly send signals instead of frontend guessing when to reconnect so we're not retrying connection even on graceful server shutdowns!

…ration

…ting

jbriones1

Maybe I don't understand it, the control functions should only be used for shutdowns, not anything else. Also, I'll need to confirm the Websocket 1001 close message, because I'm not sure this does it.

jbriones1 · 2026-08-27T08:45:02Z

 		message, ok := informMessage(player, coordinate)
 		if ok {
-			h.broadcast(message, nil, onlyViewers)
+			h.broadcastControl(message, nil, onlyViewers)


This seems dangerous. Other player messages could be waiting in the queue to be written, but someone disconnecting could remove all those messages. I think this should be a normal broadcast.

jbriones1 · 2026-08-27T08:47:32Z

 				outgoing = message
 			}
-			if !h.enqueue(connection, outgoing) {
+			if !h.enqueueControl(connection, outgoing) {


Same as above, other messages can still be relevant.

jbriones1 · 2026-08-27T08:47:45Z

 		message, ok := informMessage(player, coordinate)
 		if ok {
-			h.broadcast(message, nil, onlyViewers)
+			h.broadcastControl(message, nil, onlyViewers)


Same as above, other messages can still be relevant.

jbriones1 · 2026-08-27T09:05:32Z

+	// Block until SIGINT (Ctrl+C) or SIGTERM (systemd stop/restart).
+	quit := make(chan os.Signal, 1)
+	signal.Notify(quit, syscall.SIGINT, syscall.SIGTERM)
+	<-quit
+
+	fmt.Println("Shutdown signal received. Notifying players...")
+	sock.BroadcastShutDown(api.CMD_SHUTDOWN)
+
+	// Give write pumps ~1 second to flush the shutdown message before exiting.
+	time.Sleep(1 * time.Second)
+	fmt.Println("Server exiting.")


This seems suspicious since:

The HTTP listener is still active while shutting down, so new connections can be accepted while attempting to shut down.

It sleeps for 1 second, which is arbitrary, a deadline for WriteControl should be used instead.

It doesn't wait for the write pumps to complete.

It doesn't hit the admin or leader websocket, I think.

I've never used this websocket library before, but I don't think users get 1001 when the server shuts down. I'll need to double-check this.

karyao · 2026-09-10T21:30:20Z

closing this bc im a dumbass

karyao marked this pull request as draft August 19, 2026 01:43

karyao added 2 commits August 19, 2026 00:37

feat: reregister users when server restarts and also clears cookie if…

1285d74

… connecting fails 3 times

style: code ordering

7a8c616

karyao force-pushed the fix-12-player-session-never-recovers-after-server-restart branch from 2c598a9 to 7a8c616 Compare August 19, 2026 07:38

karyao added 4 commits August 19, 2026 01:33

test: add unit tests for re-registration after server restart

fe5a036

refactor: extract socket connect to deduplicate socket start callback

9e60e14

refactor: simplify session-expiry handling so it can't double-fire

20cc73b

fix: restore frontend/angular.json file

f72fbea

karyao force-pushed the fix-12-player-session-never-recovers-after-server-restart branch from a149dc3 to f72fbea Compare August 19, 2026 09:22

karyao marked this pull request as ready for review August 19, 2026 09:27

karyao requested a review from jbriones1 August 19, 2026 09:31

jbriones1 requested changes Aug 20, 2026

View reviewed changes

feat: handle server restart and graceful shutdown with auto re-regist…

9093e93

…ration

karyao marked this pull request as draft August 24, 2026 04:52

karyao added 3 commits August 23, 2026 21:56

refactor: move player service to credentials

ecc9e08

feat: end client sessions on server shutdown instead of auto-reconnec…

bc62000

…ting

fix: brittle unit tests with timers

461f103

karyao force-pushed the fix-12-player-session-never-recovers-after-server-restart branch from bdb0c24 to 461f103 Compare August 25, 2026 22:54

karyao added 3 commits August 25, 2026 17:09

fix: remove duplicate code that duplicated player reset behaviour

2d9edf5

fix: clean up socket timer tests and trailing whitespace

f4b375b

fix: remove dead code

6c0f83c

karyao marked this pull request as ready for review August 26, 2026 22:00

karyao requested a review from jbriones1 August 26, 2026 23:31

jbriones1 requested changes Aug 27, 2026

View reviewed changes

karyao added 3 commits September 9, 2026 15:57

fix: prevent control broadcasts from dropping queued updates

8fc083c

fix: handle server shutdown without automatic reconnection

f93c4db

fix: coordinate HTTP and WebSocket shutdown

c655a12

karyao closed this Sep 10, 2026

Uh oh!

Conversation

karyao commented Aug 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description:

Uh oh!

karyao commented Aug 19, 2026

Uh oh!

jbriones1 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

karyao commented Aug 20, 2026

Uh oh!

jbriones1 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

karyao commented Sep 10, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

karyao commented Aug 19, 2026 •

edited

Loading