From 1285d74245125ce5e1effabebf15cfe9f8962004 Mon Sep 17 00:00:00 2001 From: Karen Yao Date: Tue, 18 Aug 2026 18:00:51 -0700 Subject: [PATCH 01/16] feat: reregister users when server restarts and also clears cookie if connecting fails 3 times --- frontend/src/app/core/credentials.service.ts | 9 +++ .../src/app/core/player-name.service.spec.ts | 79 +++++++++++++++++++ frontend/src/app/core/player-name.service.ts | 26 ++++++ .../app/core/sockets/game-socket.service.ts | 29 ++++++- .../game-page/game-page.component.spec.ts | 8 +- .../pages/game-page/game-page.component.ts | 47 +++++++++++ .../register-page/register-page.component.ts | 3 + 7 files changed, 197 insertions(+), 4 deletions(-) create mode 100644 frontend/src/app/core/player-name.service.spec.ts create mode 100644 frontend/src/app/core/player-name.service.ts diff --git a/frontend/src/app/core/credentials.service.ts b/frontend/src/app/core/credentials.service.ts index 1c0b881..8c40233 100644 --- a/frontend/src/app/core/credentials.service.ts +++ b/frontend/src/app/core/credentials.service.ts @@ -47,4 +47,13 @@ export class CredentialsService { const attributes = `; Path=/; SameSite=Lax${secure}`; this.document.cookie = `id=${encodeURIComponent(credentials.id)}${attributes}`; } + + clear(): void { + if (!this.browserWindow) { + return; + } + + const secure = this.browserWindow.location.protocol === 'https:' ? '; Secure' : ''; + this.document.cookie = `id=; Path=/; SameSite=Lax${secure}; Max-Age=0`; + } } diff --git a/frontend/src/app/core/player-name.service.spec.ts b/frontend/src/app/core/player-name.service.spec.ts new file mode 100644 index 0000000..6c70f4e --- /dev/null +++ b/frontend/src/app/core/player-name.service.spec.ts @@ -0,0 +1,79 @@ +import { TestBed } from '@angular/core/testing'; + +import { PAC_WINDOW } from './browser-window.token'; +import { PlayerNameService } from './player-name.service'; + +describe('PlayerNameService', () => { + let service: PlayerNameService; + let mockStorage: Record; + + beforeEach(() => { + mockStorage = {}; + const mockWindow = { + localStorage: { + getItem: (key: string) => mockStorage[key] ?? null, + setItem: (key: string, value: string) => { + mockStorage[key] = value; + }, + }, + }; + + TestBed.configureTestingModule({ + providers: [PlayerNameService, { provide: PAC_WINDOW, useValue: mockWindow }], + }); + service = TestBed.inject(PlayerNameService); + }); + + it('returns an empty string when no name has been saved', () => { + expect(service.get()).toBe(''); + }); + + it('saves and retrieves a player name', () => { + service.save('Odin'); + expect(service.get()).toBe('Odin'); + }); + + it('returns an empty string when localStorage throws on get', () => { + TestBed.resetTestingModule(); + const throwingWindow = { + localStorage: { + getItem: () => { + throw new Error('unavailable'); + }, + setItem: () => {}, + }, + }; + TestBed.configureTestingModule({ + providers: [PlayerNameService, { provide: PAC_WINDOW, useValue: throwingWindow }], + }); + service = TestBed.inject(PlayerNameService); + expect(service.get()).toBe(''); + }); + + it('silently ignores localStorage errors on save', () => { + TestBed.resetTestingModule(); + const throwingWindow = { + localStorage: { + getItem: () => null, + setItem: () => { + throw new Error('quota exceeded'); + }, + }, + }; + TestBed.configureTestingModule({ + providers: [PlayerNameService, { provide: PAC_WINDOW, useValue: throwingWindow }], + }); + service = TestBed.inject(PlayerNameService); + expect(() => service.save('Odin')).not.toThrow(); + }); + + it('returns an empty string when PAC_WINDOW is null', () => { + TestBed.resetTestingModule(); + TestBed.configureTestingModule({ + providers: [PlayerNameService, { provide: PAC_WINDOW, useValue: null }], + }); + service = TestBed.inject(PlayerNameService); + expect(service.get()).toBe(''); + expect(() => service.save('Odin')).not.toThrow(); + }); +}); diff --git a/frontend/src/app/core/player-name.service.ts b/frontend/src/app/core/player-name.service.ts new file mode 100644 index 0000000..4bde22a --- /dev/null +++ b/frontend/src/app/core/player-name.service.ts @@ -0,0 +1,26 @@ +import { inject, Service, signal } from '@angular/core'; +import { PAC_WINDOW } from './browser-window.token'; + +const PLAYER_NAME_KEY = 'playerName'; + +@Service() +export class PlayerNameService { + private readonly browserWindow = inject(PAC_WINDOW); + private readonly statusMessage = signal(null); + + get(): string { + try { + return this.browserWindow?.localStorage.getItem(PLAYER_NAME_KEY) ?? ''; + } catch { + return ''; + } + } + + save(name: string): void { + try { + this.browserWindow?.localStorage.setItem(PLAYER_NAME_KEY, name); + } catch { + this.statusMessage.set('Could not save your name for auto-re-registration.'); + } + } +} diff --git a/frontend/src/app/core/sockets/game-socket.service.ts b/frontend/src/app/core/sockets/game-socket.service.ts index 15a74f8..b969854 100644 --- a/frontend/src/app/core/sockets/game-socket.service.ts +++ b/frontend/src/app/core/sockets/game-socket.service.ts @@ -32,15 +32,19 @@ export class GameSocketService extends WebSocketService { private reconnecting = false; private suspendedReason = 'Paused while the browser is offline.'; private readonly statusMessage = signal(null); - + private consecutiveFailures = 0; readonly players = signal>({}); readonly isFlagFound = signal(false); + readonly sessionExpired = signal(false); + readonly MAX_FAILED_ATTEMPTS = 3; start(id: string, onConnected: () => void): void { this.stop(); this.mode = 'player'; this.playerId = id; this.onConnected = onConnected; + this.consecutiveFailures = 0; + this.sessionExpired.set(false); this.resume(); } @@ -74,6 +78,8 @@ export class GameSocketService extends WebSocketService { this.onConnected = null; this.reconnecting = false; this.statusMessage.set(null); + this.consecutiveFailures = 0; + this.sessionExpired.set(false); this.disconnect(); } @@ -94,8 +100,9 @@ export class GameSocketService extends WebSocketService { // Every connection receives a fresh list of active players. Clearing the // cache removes disconnects that may have been missed while unavailable. this.players.set({}); - this.reconnecting = false; this.statusMessage.set(null); + this.reconnecting = false; + this.consecutiveFailures = 0; this.onConnected?.(); } @@ -103,6 +110,24 @@ export class GameSocketService extends WebSocketService { this.reconnecting = true; } + protected override shouldReconnect(closeEvent: CloseEvent): boolean { + if (this.mode !== 'player') { + return true; + } + + this.consecutiveFailures++; + + if (this.consecutiveFailures >= this.MAX_FAILED_ATTEMPTS) { + this.sessionExpired.set(true); + this.statusMessage.set( + 'Session has expired as game server restarted.', + ); + return false; + } + + return true; + } + protected override onSocketError(): void { this.statusMessage.set( this.mode === 'viewer' diff --git a/frontend/src/app/pages/game-page/game-page.component.spec.ts b/frontend/src/app/pages/game-page/game-page.component.spec.ts index 5d040de..5f7a145 100644 --- a/frontend/src/app/pages/game-page/game-page.component.spec.ts +++ b/frontend/src/app/pages/game-page/game-page.component.spec.ts @@ -10,6 +10,7 @@ import { GeolocationService } from '../../core/geolocation.service'; import { MapInfo, PlayerStatus, PlayerType } from '../../core/game.models'; import { WakeLockService } from '../../core/wake-lock.service'; import { GamePageComponent } from './game-page.component'; +import { PlayerNameService } from '../../core/player-name.service'; describe('GamePageComponent leader link', () => { let fixture: ComponentFixture; @@ -34,6 +35,7 @@ describe('GamePageComponent leader link', () => { }), status: signal('Connected.'), isFlagFound: signal(false), + sessionExpired: signal(false), start: vi.fn(), stop: vi.fn(), resume: vi.fn(), @@ -69,7 +71,8 @@ describe('GamePageComponent leader link', () => { imports: [GamePageComponent], providers: [ { provide: ApiService, useValue: { getMap: vi.fn(() => of(map)) } }, - { provide: CredentialsService, useValue: { get: () => ({ id: 'SELF' }) } }, + { provide: CredentialsService, useValue: { get: () => ({ id: 'SELF' }), save: vi.fn(), clear: vi.fn() } }, + { provide: PlayerNameService, useValue: { get: vi.fn(() => ''), save: vi.fn() } }, { provide: Router, useValue: { navigateByUrl: vi.fn() } }, ], }) @@ -110,6 +113,7 @@ describe('GamePageComponent leader link', () => { it('does not show the leader link to a non-leader', async () => { const page = await render(PlayerType.Ghost); - expect(page.querySelector('.game-page__leader-link')).toBeNull(); + const link = page.querySelector('.game-page__leader-link a'); + expect(link?.style.visibility).toBe('hidden'); }); }); diff --git a/frontend/src/app/pages/game-page/game-page.component.ts b/frontend/src/app/pages/game-page/game-page.component.ts index b435cb3..918184b 100644 --- a/frontend/src/app/pages/game-page/game-page.component.ts +++ b/frontend/src/app/pages/game-page/game-page.component.ts @@ -4,6 +4,7 @@ import { Component, computed, DestroyRef, + effect, inject, signal, } from '@angular/core'; @@ -19,6 +20,7 @@ import { isLeaderType, MapInfo, typeLabel } from '../../core/game.models'; import { WakeLockService } from '../../core/wake-lock.service'; import { GameCanvasComponent } from '../../game/game-canvas/game-canvas.component'; import { BrandHeaderComponent } from '../../shared/brand-header/brand-header.component'; +import { PlayerNameService } from '../../core/player-name.service'; @Component({ selector: 'pac-game-page', @@ -34,6 +36,7 @@ export class GamePageComponent { private readonly credentials = inject(CredentialsService); private readonly destroyRef = inject(DestroyRef); private readonly router = inject(Router); + private readonly playerName = inject(PlayerNameService); protected readonly socket = inject(GameSocketService); protected readonly geolocation = inject(GeolocationService); @@ -41,6 +44,8 @@ export class GamePageComponent { protected readonly map = signal(null); protected readonly selfId = signal(''); protected readonly pageStatus = signal('Loading the game map…'); + + private readonly reregistering = signal(false); protected readonly selfSummary = computed(() => { const player = this.socket.players()[this.selfId()]?.player; return player ? `${player.name} (${player.id}) is ${typeLabel(player.type)}` : ''; @@ -62,12 +67,54 @@ export class GamePageComponent { constructor() { afterNextRender(() => void this.initialize()); this.destroyRef.onDestroy(() => this.cleanup()); + + effect(() => { + if (this.socket.sessionExpired() && !this.reregistering()) { + void this.autoReregister(); + } + }); } protected async toggleWakeLock(event: Event): Promise { await this.wakeLock.setEnabled((event.target as HTMLInputElement).checked); } + private async autoReregister(): Promise { + const name = this.playerName.get(); + if (!name) { + this.credentials.clear(); + this.socket.stop(); + await this.router.navigateByUrl('/register'); + return; + } + + this.reregistering.set(true); + this.pageStatus.set('Re-registering…'); + + try { + const response = await firstValueFrom(this.api.registerPlayer(name)); + const id = response.id.trim(); + if (!id) { + throw new Error('The API returned an empty player ID.'); + } + + this.credentials.save({ id }); + this.selfId.set(id); + this.pageStatus.set('Re-registered. Reconnecting…'); + this.socket.start(id, () => { + this.pageStatus.set('Connected to PacMacro.'); + this.geolocation.start((coordinate) => this.socket.sendCoordinate(coordinate)); + }); + } catch { + this.credentials.clear(); + this.socket.stop(); + this.pageStatus.set('Could not re-register. Redirecting…'); + await this.router.navigateByUrl('/register'); + } finally { + this.reregistering.set(false); + } + } + private async initialize(): Promise { if (!this.browserWindow) { return; diff --git a/frontend/src/app/pages/register-page/register-page.component.ts b/frontend/src/app/pages/register-page/register-page.component.ts index c04ca41..6b251ca 100644 --- a/frontend/src/app/pages/register-page/register-page.component.ts +++ b/frontend/src/app/pages/register-page/register-page.component.ts @@ -12,6 +12,7 @@ import { firstValueFrom } from 'rxjs'; import { ApiService } from '../../core/api.service'; import { CredentialsService } from '../../core/credentials.service'; import { BrandHeaderComponent } from '../../shared/brand-header/brand-header.component'; +import { PlayerNameService } from '../../core/player-name.service'; interface RegistrationModel { name: string; @@ -28,6 +29,7 @@ export class RegisterPageComponent { private readonly api = inject(ApiService); private readonly credentials = inject(CredentialsService); private readonly router = inject(Router); + private readonly playerName = inject(PlayerNameService); protected readonly registrationModel = signal({ name: '', @@ -62,6 +64,7 @@ export class RegisterPageComponent { throw new Error('The API returned an empty player ID.'); } this.credentials.save({ id }); + this.playerName.save(trimmedName); await this.router.navigateByUrl('/'); } catch (error) { this.status.set('Registration failed. Check your details and the API connection.'); From 7a8c616331c2621e2b962a7ed42871cca6dc82d6 Mon Sep 17 00:00:00 2001 From: Karen Yao Date: Tue, 18 Aug 2026 18:04:44 -0700 Subject: [PATCH 02/16] style: code ordering --- frontend/src/app/core/sockets/game-socket.service.ts | 5 +++-- frontend/src/app/pages/game-page/game-page.component.ts | 2 +- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/frontend/src/app/core/sockets/game-socket.service.ts b/frontend/src/app/core/sockets/game-socket.service.ts index b969854..cd80361 100644 --- a/frontend/src/app/core/sockets/game-socket.service.ts +++ b/frontend/src/app/core/sockets/game-socket.service.ts @@ -31,8 +31,9 @@ export class GameSocketService extends WebSocketService { private onConnected: (() => void) | null = null; private reconnecting = false; private suspendedReason = 'Paused while the browser is offline.'; - private readonly statusMessage = signal(null); private consecutiveFailures = 0; + private readonly statusMessage = signal(null); + readonly players = signal>({}); readonly isFlagFound = signal(false); readonly sessionExpired = signal(false); @@ -100,8 +101,8 @@ export class GameSocketService extends WebSocketService { // Every connection receives a fresh list of active players. Clearing the // cache removes disconnects that may have been missed while unavailable. this.players.set({}); - this.statusMessage.set(null); this.reconnecting = false; + this.statusMessage.set(null); this.consecutiveFailures = 0; this.onConnected?.(); } diff --git a/frontend/src/app/pages/game-page/game-page.component.ts b/frontend/src/app/pages/game-page/game-page.component.ts index 918184b..7f0b548 100644 --- a/frontend/src/app/pages/game-page/game-page.component.ts +++ b/frontend/src/app/pages/game-page/game-page.component.ts @@ -37,6 +37,7 @@ export class GamePageComponent { private readonly destroyRef = inject(DestroyRef); private readonly router = inject(Router); private readonly playerName = inject(PlayerNameService); + private readonly reregistering = signal(false); protected readonly socket = inject(GameSocketService); protected readonly geolocation = inject(GeolocationService); @@ -45,7 +46,6 @@ export class GamePageComponent { protected readonly selfId = signal(''); protected readonly pageStatus = signal('Loading the game map…'); - private readonly reregistering = signal(false); protected readonly selfSummary = computed(() => { const player = this.socket.players()[this.selfId()]?.player; return player ? `${player.name} (${player.id}) is ${typeLabel(player.type)}` : ''; From fe5a0368dab387a342986811370d7db1b1d63007 Mon Sep 17 00:00:00 2001 From: Karen Yao Date: Wed, 19 Aug 2026 01:33:41 -0700 Subject: [PATCH 03/16] test: add unit tests for re-registration after server restart --- .../src/app/core/credentials.service.spec.ts | 53 +++- .../core/sockets/game-socket.service.spec.ts | 96 ++++++++ .../game-page/game-page.component.spec.ts | 232 +++++++++++++----- .../register-page.component.spec.ts | 8 +- 4 files changed, 324 insertions(+), 65 deletions(-) diff --git a/frontend/src/app/core/credentials.service.spec.ts b/frontend/src/app/core/credentials.service.spec.ts index 3b3ddae..90edebb 100644 --- a/frontend/src/app/core/credentials.service.spec.ts +++ b/frontend/src/app/core/credentials.service.spec.ts @@ -1,4 +1,8 @@ -import { readCookie } from './credentials.service'; +import { DOCUMENT } from '@angular/common'; +import { TestBed } from '@angular/core/testing'; + +import { PAC_WINDOW } from './browser-window.token'; +import { CredentialsService, readCookie } from './credentials.service'; describe('readCookie', () => { it('reads and decodes an exact cookie name', () => { @@ -10,3 +14,50 @@ describe('readCookie', () => { expect(readCookie('theme=dark', 'id')).toBe(''); }); }); + +describe('CredentialsService', () => { + let service: CredentialsService; + let mockDocument: { cookie: string }; + let mockWindow: { location: { protocol: string } }; + + beforeEach(() => { + mockDocument = { cookie: '' }; + mockWindow = { location: { protocol: 'http:' } }; + + TestBed.configureTestingModule({ + providers: [ + CredentialsService, + { provide: DOCUMENT, useValue: mockDocument }, + { provide: PAC_WINDOW, useValue: mockWindow }, + ], + }); + service = TestBed.inject(CredentialsService); + }); + + it.each(['http:', 'https:'] as const)('expires the id cookie over %s', (protocol) => { + mockWindow.location.protocol = protocol; + mockDocument.cookie = 'id=ABC; theme=dark'; + + service.clear(); + + const secure = protocol === 'https:' ? '; Secure' : ''; + expect(mockDocument.cookie).toBe(`id=; Path=/; SameSite=Lax${secure}; Max-Age=0`); + }); + + it('leaves the cookie untouched when PAC_WINDOW is null', () => { + TestBed.resetTestingModule(); + TestBed.configureTestingModule({ + providers: [ + CredentialsService, + { provide: DOCUMENT, useValue: mockDocument }, + { provide: PAC_WINDOW, useValue: null }, + ], + }); + service = TestBed.inject(CredentialsService); + mockDocument.cookie = 'id=ABC'; + + service.clear(); + + expect(mockDocument.cookie).toBe('id=ABC'); + }); +}); diff --git a/frontend/src/app/core/sockets/game-socket.service.spec.ts b/frontend/src/app/core/sockets/game-socket.service.spec.ts index e0b6040..d8f3b20 100644 --- a/frontend/src/app/core/sockets/game-socket.service.spec.ts +++ b/frontend/src/app/core/sockets/game-socket.service.spec.ts @@ -297,4 +297,100 @@ describe('GameSocketService', () => { expect(service.state()).toBe('error'); expect(service.status()).toContain('Register as admin again in this browser'); }); + + it('expires the session after three consecutive failed player connections', () => { + vi.useFakeTimers(); + vi.spyOn(console, 'log').mockImplementation(() => undefined); + vi.spyOn(console, 'warn').mockImplementation(() => undefined); + service.start('ABCD', () => undefined); + + MockGameWebSocket.instances[0].serverClose(false); + vi.advanceTimersByTime(1000); + MockGameWebSocket.instances[1].serverClose(false); + vi.advanceTimersByTime(2000); + MockGameWebSocket.instances[2].serverClose(false); + vi.runAllTimers(); + + expect(service.sessionExpired()).toBe(true); + expect(service.state()).toBe('error'); + expect(service.status()).toContain('Session has expired as game server restarted.'); + expect(MockGameWebSocket.instances).toHaveLength(3); + }); + + it('keeps reconnecting a player socket after fewer than three failures', () => { + vi.useFakeTimers(); + vi.spyOn(console, 'log').mockImplementation(() => undefined); + vi.spyOn(console, 'warn').mockImplementation(() => undefined); + service.start('ABCD', () => undefined); + + MockGameWebSocket.instances[0].serverClose(false); + vi.advanceTimersByTime(1000); + MockGameWebSocket.instances[1].serverClose(false); + vi.advanceTimersByTime(2000); + + expect(service.sessionExpired()).toBe(false); + expect(service.state()).toBe('connecting'); + expect(MockGameWebSocket.instances).toHaveLength(3); + }); + + it('resets the failure counter when a player connection succeeds', () => { + vi.useFakeTimers(); + vi.spyOn(console, 'log').mockImplementation(() => undefined); + vi.spyOn(console, 'warn').mockImplementation(() => undefined); + service.start('ABCD', () => undefined); + + MockGameWebSocket.instances[0].serverClose(false); + vi.advanceTimersByTime(1000); + MockGameWebSocket.instances[1].serverClose(false); + vi.advanceTimersByTime(2000); + + MockGameWebSocket.instances[2].open(); + MockGameWebSocket.instances[2].serverClose(false); + vi.advanceTimersByTime(4000); + MockGameWebSocket.instances[3].serverClose(false); + vi.runAllTimers(); + + expect(service.sessionExpired()).toBe(false); + expect(MockGameWebSocket.instances).toHaveLength(5); + }); + + it('never expires the session for a viewer socket', () => { + vi.useFakeTimers(); + vi.spyOn(console, 'log').mockImplementation(() => undefined); + vi.spyOn(console, 'warn').mockImplementation(() => undefined); + service.startViewer(); + + MockGameWebSocket.instances[0].serverClose(false); + vi.advanceTimersByTime(1000); + MockGameWebSocket.instances[1].serverClose(false); + vi.advanceTimersByTime(2000); + MockGameWebSocket.instances[2].serverClose(false); + vi.runAllTimers(); + + expect(service.sessionExpired()).toBe(false); + expect(MockGameWebSocket.instances.length).toBeGreaterThan(3); + }); + + it('start and stop clear an expired session', () => { + vi.useFakeTimers(); + vi.spyOn(console, 'log').mockImplementation(() => undefined); + vi.spyOn(console, 'warn').mockImplementation(() => undefined); + service.start('ABCD', () => undefined); + + MockGameWebSocket.instances[0].serverClose(false); + vi.advanceTimersByTime(1000); + MockGameWebSocket.instances[1].serverClose(false); + vi.advanceTimersByTime(2000); + MockGameWebSocket.instances[2].serverClose(false); + vi.runAllTimers(); + + expect(service.sessionExpired()).toBe(true); + + service.stop(); + expect(service.sessionExpired()).toBe(false); + + service.start('ABCD', () => undefined); + expect(service.sessionExpired()).toBe(false); + expect(MockGameWebSocket.instances).toHaveLength(4); + }); }); diff --git a/frontend/src/app/pages/game-page/game-page.component.spec.ts b/frontend/src/app/pages/game-page/game-page.component.spec.ts index 5f7a145..3d69963 100644 --- a/frontend/src/app/pages/game-page/game-page.component.spec.ts +++ b/frontend/src/app/pages/game-page/game-page.component.spec.ts @@ -1,7 +1,7 @@ import { signal } from '@angular/core'; import { ComponentFixture, TestBed } from '@angular/core/testing'; import { Router } from '@angular/router'; -import { of } from 'rxjs'; +import { of, throwError } from 'rxjs'; import { ApiService } from '../../core/api.service'; import { CredentialsService } from '../../core/credentials.service'; @@ -12,51 +12,88 @@ import { WakeLockService } from '../../core/wake-lock.service'; import { GamePageComponent } from './game-page.component'; import { PlayerNameService } from '../../core/player-name.service'; +const map: MapInfo = { + min: { latitude: 49.27, longitude: -122.92 }, + max: { latitude: 49.28, longitude: -122.9 }, + width: 32, + height: 32, + isFlagFound: false, +}; +const api = { + getMap: vi.fn(() => of(map)), + registerPlayer: vi.fn(() => of({ id: 'NEWID' })), +}; +const credentials = { + get: vi.fn(() => ({ id: 'SELF' })), + save: vi.fn(), + clear: vi.fn(), +}; +const playerName = { + get: vi.fn(() => ''), + save: vi.fn(), +}; +const router = { navigateByUrl: vi.fn() }; +const gameSocket = { + players: signal({ + SELF: { + coordinate: { latitude: 49.275, longitude: -122.91 }, + player: { + id: 'SELF', + name: 'Leader', + type: PlayerType.Ghost, + status: PlayerStatus.Connected, + }, + }, + }), + status: signal('Connected.'), + isFlagFound: signal(false), + sessionExpired: signal(false), + start: vi.fn((_id: string, _onConnected: () => void) => gameSocket.sessionExpired.set(false)), + stop: vi.fn(() => gameSocket.sessionExpired.set(false)), + resume: vi.fn(), + suspend: vi.fn(), + sendCoordinate: vi.fn(), + setInitialState: vi.fn(), +}; +const geolocation = { + status: signal('Ready.'), + start: vi.fn(), + stop: vi.fn(), +}; +const wakeLock = { + supported: signal(true), + enabled: signal(false), + status: signal('Screen wake lock is off.'), + initialize: vi.fn(), + setEnabled: vi.fn(async () => undefined), + handleVisibilityChange: vi.fn(async () => undefined), + release: vi.fn(async () => undefined), +}; + +async function configureTestBed(): Promise { + await TestBed.configureTestingModule({ + imports: [GamePageComponent], + providers: [ + { provide: ApiService, useValue: api }, + { provide: CredentialsService, useValue: credentials }, + { provide: PlayerNameService, useValue: playerName }, + { provide: Router, useValue: router }, + ], + }) + .overrideComponent(GamePageComponent, { + set: { + providers: [ + { provide: GameSocketService, useValue: gameSocket }, + { provide: GeolocationService, useValue: geolocation }, + { provide: WakeLockService, useValue: wakeLock }, + ], + }, + }) + .compileComponents(); +} + describe('GamePageComponent leader link', () => { let fixture: ComponentFixture; - const map: MapInfo = { - min: { latitude: 49.27, longitude: -122.92 }, - max: { latitude: 49.28, longitude: -122.9 }, - width: 32, - height: 32, - isFlagFound: false, - }; - const gameSocket = { - players: signal({ - SELF: { - coordinate: { latitude: 49.275, longitude: -122.91 }, - player: { - id: 'SELF', - name: 'Leader', - type: PlayerType.Ghost, - status: PlayerStatus.Connected, - }, - }, - }), - status: signal('Connected.'), - isFlagFound: signal(false), - sessionExpired: signal(false), - start: vi.fn(), - stop: vi.fn(), - resume: vi.fn(), - suspend: vi.fn(), - sendCoordinate: vi.fn(), - setInitialState: vi.fn(), - }; - const geolocation = { - status: signal('Ready.'), - start: vi.fn(), - stop: vi.fn(), - }; - const wakeLock = { - supported: signal(true), - enabled: signal(false), - status: signal('Screen wake lock is off.'), - initialize: vi.fn(), - setEnabled: vi.fn(async () => undefined), - handleVisibilityChange: vi.fn(async () => undefined), - release: vi.fn(async () => undefined), - }; beforeEach(async () => { gameSocket.players.update((players) => ({ @@ -67,25 +104,7 @@ describe('GamePageComponent leader link', () => { }, })); - await TestBed.configureTestingModule({ - imports: [GamePageComponent], - providers: [ - { provide: ApiService, useValue: { getMap: vi.fn(() => of(map)) } }, - { provide: CredentialsService, useValue: { get: () => ({ id: 'SELF' }), save: vi.fn(), clear: vi.fn() } }, - { provide: PlayerNameService, useValue: { get: vi.fn(() => ''), save: vi.fn() } }, - { provide: Router, useValue: { navigateByUrl: vi.fn() } }, - ], - }) - .overrideComponent(GamePageComponent, { - set: { - providers: [ - { provide: GameSocketService, useValue: gameSocket }, - { provide: GeolocationService, useValue: geolocation }, - { provide: WakeLockService, useValue: wakeLock }, - ], - }, - }) - .compileComponents(); + await configureTestBed(); }); async function render(playerType: PlayerType): Promise { @@ -117,3 +136,90 @@ describe('GamePageComponent leader link', () => { expect(link?.style.visibility).toBe('hidden'); }); }); + +describe('GamePageComponent re-registration', () => { + let fixture: ComponentFixture; + + beforeEach(async () => { + vi.clearAllMocks(); + gameSocket.sessionExpired.set(false); + await configureTestBed(); + }); + + async function render(): Promise { + fixture = TestBed.createComponent(GamePageComponent); + fixture.detectChanges(); + await new Promise((resolve) => setTimeout(resolve, 0)); + fixture.detectChanges(); + return fixture.nativeElement as HTMLElement; + } + + async function flushReRegistration(): Promise { + fixture.detectChanges(); + await new Promise((resolve) => setTimeout(resolve, 0)); + fixture.detectChanges(); + } + + it('clears credentials and redirects to /register when no name is saved', async () => { + playerName.get.mockReturnValue(''); + await render(); + + gameSocket.sessionExpired.set(true); + await flushReRegistration(); + + expect(credentials.clear).toHaveBeenCalled(); + expect(gameSocket.stop).toHaveBeenCalled(); + expect(router.navigateByUrl).toHaveBeenCalledWith('/register'); + }); + + it('re-registers with the saved name and reconnects', async () => { + playerName.get.mockReturnValue('Odin'); + api.registerPlayer.mockReturnValue(of({ id: 'NEWID' })); + const page = await render(); + + gameSocket.sessionExpired.set(true); + await flushReRegistration(); + + expect(api.registerPlayer).toHaveBeenCalledWith('Odin'); + expect(credentials.save).toHaveBeenCalledWith({ id: 'NEWID' }); + expect(credentials.clear).not.toHaveBeenCalled(); + expect(page.textContent).toContain('Re-registered. Reconnecting…'); + + const startCalls = gameSocket.start.mock.calls; + const onConnected = startCalls[startCalls.length - 1][1] as () => void; + expect(startCalls[startCalls.length - 1][0]).toBe('NEWID'); + onConnected(); + fixture.detectChanges(); + + expect(geolocation.start).toHaveBeenCalledWith(expect.any(Function)); + expect(page.textContent).toContain('Connected to PacMacro.'); + }); + + it('clears credentials and redirects when re-registration fails', async () => { + playerName.get.mockReturnValue('Odin'); + api.registerPlayer.mockReturnValue(throwError(() => new Error('API is down'))); + const page = await render(); + + gameSocket.sessionExpired.set(true); + await flushReRegistration(); + + expect(credentials.clear).toHaveBeenCalled(); + expect(gameSocket.stop).toHaveBeenCalled(); + expect(router.navigateByUrl).toHaveBeenCalledWith('/register'); + expect(page.textContent).toContain('Could not re-register. Redirecting…'); + }); + + it('treats an empty player ID from the API as a failure', async () => { + playerName.get.mockReturnValue('Odin'); + api.registerPlayer.mockReturnValue(of({ id: ' ' })); + const page = await render(); + + gameSocket.sessionExpired.set(true); + await flushReRegistration(); + + expect(credentials.clear).toHaveBeenCalled(); + expect(gameSocket.stop).toHaveBeenCalled(); + expect(router.navigateByUrl).toHaveBeenCalledWith('/register'); + expect(page.textContent).toContain('Could not re-register. Redirecting…'); + }); +}); diff --git a/frontend/src/app/pages/register-page/register-page.component.spec.ts b/frontend/src/app/pages/register-page/register-page.component.spec.ts index e560a89..263c45b 100644 --- a/frontend/src/app/pages/register-page/register-page.component.spec.ts +++ b/frontend/src/app/pages/register-page/register-page.component.spec.ts @@ -5,6 +5,7 @@ import { of } from 'rxjs'; import { ApiService } from '../../core/api.service'; import { CredentialsService } from '../../core/credentials.service'; +import { PlayerNameService } from '../../core/player-name.service'; import { RegisterPageComponent } from './register-page.component'; describe('RegisterPageComponent', () => { @@ -13,6 +14,7 @@ describe('RegisterPageComponent', () => { registerPlayer: vi.fn(() => of({ id: 'ABCD' })), }; const credentials = { save: vi.fn() }; + const playerName = { save: vi.fn() }; const router = { navigateByUrl: vi.fn(() => Promise.resolve(true)) }; beforeEach(() => { @@ -22,6 +24,7 @@ describe('RegisterPageComponent', () => { providers: [ { provide: ApiService, useValue: api }, { provide: CredentialsService, useValue: credentials }, + { provide: PlayerNameService, useValue: playerName }, { provide: Router, useValue: router }, ], }); @@ -31,7 +34,7 @@ describe('RegisterPageComponent', () => { const component = TestBed.createComponent(RegisterPageComponent) .componentInstance as unknown as RegisterPageHarness; component.registrationModel.set({ - name: 'Test2', + name: ' Test2 ', }); await component.submit(submitEvent()); @@ -39,6 +42,7 @@ describe('RegisterPageComponent', () => { expect(api.registerPlayer).toHaveBeenCalledWith('Test2'); expect(api.registerAdmin).not.toHaveBeenCalled(); expect(credentials.save).toHaveBeenCalledWith({ id: 'ABCD' }); + expect(playerName.save).toHaveBeenCalledWith('Test2'); expect(router.navigateByUrl).toHaveBeenCalledWith('/'); }); @@ -52,6 +56,8 @@ describe('RegisterPageComponent', () => { await component.submit(submitEvent()); expect(api.registerPlayer).not.toHaveBeenCalled(); + expect(credentials.save).not.toHaveBeenCalled(); + expect(playerName.save).not.toHaveBeenCalled(); }); }); From 9e60e143d0e7a9ef74128bc507e48d1b5edd2585 Mon Sep 17 00:00:00 2001 From: Karen Yao Date: Wed, 19 Aug 2026 01:53:27 -0700 Subject: [PATCH 04/16] refactor: extract socket connect to deduplicate socket start callback --- .../src/app/pages/game-page/game-page.component.ts | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/frontend/src/app/pages/game-page/game-page.component.ts b/frontend/src/app/pages/game-page/game-page.component.ts index 7f0b548..e88f8f8 100644 --- a/frontend/src/app/pages/game-page/game-page.component.ts +++ b/frontend/src/app/pages/game-page/game-page.component.ts @@ -101,10 +101,7 @@ export class GamePageComponent { this.credentials.save({ id }); this.selfId.set(id); this.pageStatus.set('Re-registered. Reconnecting…'); - this.socket.start(id, () => { - this.pageStatus.set('Connected to PacMacro.'); - this.geolocation.start((coordinate) => this.socket.sendCoordinate(coordinate)); - }); + this.connectAs(id); } catch { this.credentials.clear(); this.socket.stop(); @@ -143,7 +140,11 @@ export class GamePageComponent { this.browserWindow.document.addEventListener('visibilitychange', this.onVisibilityChange); this.browserWindow.addEventListener('online', this.onOnline); this.browserWindow.addEventListener('offline', this.onOffline); - this.socket.start(credentials.id, () => { + this.connectAs(credentials.id); + } + + private connectAs(id: string): void { + this.socket.start(id, () => { this.pageStatus.set('Connected to PacMacro.'); this.geolocation.start((coordinate) => this.socket.sendCoordinate(coordinate)); }); From 20cc73bc5a550772c90832d030b47b4ec84bce53 Mon Sep 17 00:00:00 2001 From: Karen Yao Date: Wed, 19 Aug 2026 02:18:34 -0700 Subject: [PATCH 05/16] refactor: simplify session-expiry handling so it can't double-fire --- frontend/angular.json | 3 ++- .../core/sockets/game-socket.service.spec.ts | 20 +++++++++++++++- .../app/core/sockets/game-socket.service.ts | 6 ++++- .../game-page/game-page.component.spec.ts | 15 ++++++++---- .../pages/game-page/game-page.component.ts | 23 +++++++------------ 5 files changed, 44 insertions(+), 23 deletions(-) diff --git a/frontend/angular.json b/frontend/angular.json index 1d74f66..21e6be7 100644 --- a/frontend/angular.json +++ b/frontend/angular.json @@ -2,7 +2,8 @@ "$schema": "./node_modules/@angular/cli/lib/config/schema.json", "version": 1, "cli": { - "packageManager": "npm" + "packageManager": "npm", + "analytics": "132fecc1-3206-4f36-bf3c-2dcfd0f12560" }, "newProjectRoot": "projects", "projects": { diff --git a/frontend/src/app/core/sockets/game-socket.service.spec.ts b/frontend/src/app/core/sockets/game-socket.service.spec.ts index d8f3b20..6138836 100644 --- a/frontend/src/app/core/sockets/game-socket.service.spec.ts +++ b/frontend/src/app/core/sockets/game-socket.service.spec.ts @@ -302,7 +302,8 @@ describe('GameSocketService', () => { vi.useFakeTimers(); vi.spyOn(console, 'log').mockImplementation(() => undefined); vi.spyOn(console, 'warn').mockImplementation(() => undefined); - service.start('ABCD', () => undefined); + const onSessionExpired = vi.fn(); + service.start('ABCD', () => undefined, onSessionExpired); MockGameWebSocket.instances[0].serverClose(false); vi.advanceTimersByTime(1000); @@ -315,6 +316,23 @@ describe('GameSocketService', () => { expect(service.state()).toBe('error'); expect(service.status()).toContain('Session has expired as game server restarted.'); expect(MockGameWebSocket.instances).toHaveLength(3); + expect(onSessionExpired).toHaveBeenCalledOnce(); + }); + + it('does not invoke the session-expired callback after fewer than three failures', () => { + vi.useFakeTimers(); + vi.spyOn(console, 'log').mockImplementation(() => undefined); + vi.spyOn(console, 'warn').mockImplementation(() => undefined); + const onSessionExpired = vi.fn(); + service.start('ABCD', () => undefined, onSessionExpired); + + MockGameWebSocket.instances[0].serverClose(false); + vi.advanceTimersByTime(1000); + MockGameWebSocket.instances[1].serverClose(false); + vi.advanceTimersByTime(2000); + + expect(service.sessionExpired()).toBe(false); + expect(onSessionExpired).not.toHaveBeenCalled(); }); it('keeps reconnecting a player socket after fewer than three failures', () => { diff --git a/frontend/src/app/core/sockets/game-socket.service.ts b/frontend/src/app/core/sockets/game-socket.service.ts index cd80361..8f835b6 100644 --- a/frontend/src/app/core/sockets/game-socket.service.ts +++ b/frontend/src/app/core/sockets/game-socket.service.ts @@ -29,6 +29,7 @@ export class GameSocketService extends WebSocketService { private playerId: string | null = null; private mode: SocketMode | null = null; private onConnected: (() => void) | null = null; + private onSessionExpired: (() => void) | null = null; private reconnecting = false; private suspendedReason = 'Paused while the browser is offline.'; private consecutiveFailures = 0; @@ -39,11 +40,12 @@ export class GameSocketService extends WebSocketService { readonly sessionExpired = signal(false); readonly MAX_FAILED_ATTEMPTS = 3; - start(id: string, onConnected: () => void): void { + start(id: string, onConnected: () => void, onSessionExpired: () => void = () => undefined): void { this.stop(); this.mode = 'player'; this.playerId = id; this.onConnected = onConnected; + this.onSessionExpired = onSessionExpired; this.consecutiveFailures = 0; this.sessionExpired.set(false); this.resume(); @@ -77,6 +79,7 @@ export class GameSocketService extends WebSocketService { this.mode = null; this.playerId = null; this.onConnected = null; + this.onSessionExpired = null; this.reconnecting = false; this.statusMessage.set(null); this.consecutiveFailures = 0; @@ -123,6 +126,7 @@ export class GameSocketService extends WebSocketService { this.statusMessage.set( 'Session has expired as game server restarted.', ); + this.onSessionExpired?.(); return false; } diff --git a/frontend/src/app/pages/game-page/game-page.component.spec.ts b/frontend/src/app/pages/game-page/game-page.component.spec.ts index 3d69963..5b7dd2e 100644 --- a/frontend/src/app/pages/game-page/game-page.component.spec.ts +++ b/frontend/src/app/pages/game-page/game-page.component.spec.ts @@ -33,6 +33,7 @@ const playerName = { save: vi.fn(), }; const router = { navigateByUrl: vi.fn() }; +let triggerSessionExpired: (() => void) | null = null; const gameSocket = { players: signal({ SELF: { @@ -48,7 +49,10 @@ const gameSocket = { status: signal('Connected.'), isFlagFound: signal(false), sessionExpired: signal(false), - start: vi.fn((_id: string, _onConnected: () => void) => gameSocket.sessionExpired.set(false)), + start: vi.fn((_id: string, _onConnected: () => void, onSessionExpired: () => void) => { + triggerSessionExpired = onSessionExpired; + gameSocket.sessionExpired.set(false); + }), stop: vi.fn(() => gameSocket.sessionExpired.set(false)), resume: vi.fn(), suspend: vi.fn(), @@ -143,6 +147,7 @@ describe('GamePageComponent re-registration', () => { beforeEach(async () => { vi.clearAllMocks(); gameSocket.sessionExpired.set(false); + triggerSessionExpired = null; await configureTestBed(); }); @@ -164,7 +169,7 @@ describe('GamePageComponent re-registration', () => { playerName.get.mockReturnValue(''); await render(); - gameSocket.sessionExpired.set(true); + triggerSessionExpired?.(); await flushReRegistration(); expect(credentials.clear).toHaveBeenCalled(); @@ -177,7 +182,7 @@ describe('GamePageComponent re-registration', () => { api.registerPlayer.mockReturnValue(of({ id: 'NEWID' })); const page = await render(); - gameSocket.sessionExpired.set(true); + triggerSessionExpired?.(); await flushReRegistration(); expect(api.registerPlayer).toHaveBeenCalledWith('Odin'); @@ -200,7 +205,7 @@ describe('GamePageComponent re-registration', () => { api.registerPlayer.mockReturnValue(throwError(() => new Error('API is down'))); const page = await render(); - gameSocket.sessionExpired.set(true); + triggerSessionExpired?.(); await flushReRegistration(); expect(credentials.clear).toHaveBeenCalled(); @@ -214,7 +219,7 @@ describe('GamePageComponent re-registration', () => { api.registerPlayer.mockReturnValue(of({ id: ' ' })); const page = await render(); - gameSocket.sessionExpired.set(true); + triggerSessionExpired?.(); await flushReRegistration(); expect(credentials.clear).toHaveBeenCalled(); diff --git a/frontend/src/app/pages/game-page/game-page.component.ts b/frontend/src/app/pages/game-page/game-page.component.ts index e88f8f8..3574ae2 100644 --- a/frontend/src/app/pages/game-page/game-page.component.ts +++ b/frontend/src/app/pages/game-page/game-page.component.ts @@ -4,7 +4,6 @@ import { Component, computed, DestroyRef, - effect, inject, signal, } from '@angular/core'; @@ -37,7 +36,6 @@ export class GamePageComponent { private readonly destroyRef = inject(DestroyRef); private readonly router = inject(Router); private readonly playerName = inject(PlayerNameService); - private readonly reregistering = signal(false); protected readonly socket = inject(GameSocketService); protected readonly geolocation = inject(GeolocationService); @@ -67,12 +65,6 @@ export class GamePageComponent { constructor() { afterNextRender(() => void this.initialize()); this.destroyRef.onDestroy(() => this.cleanup()); - - effect(() => { - if (this.socket.sessionExpired() && !this.reregistering()) { - void this.autoReregister(); - } - }); } protected async toggleWakeLock(event: Event): Promise { @@ -88,7 +80,6 @@ export class GamePageComponent { return; } - this.reregistering.set(true); this.pageStatus.set('Re-registering…'); try { @@ -107,8 +98,6 @@ export class GamePageComponent { this.socket.stop(); this.pageStatus.set('Could not re-register. Redirecting…'); await this.router.navigateByUrl('/register'); - } finally { - this.reregistering.set(false); } } @@ -144,10 +133,14 @@ export class GamePageComponent { } private connectAs(id: string): void { - this.socket.start(id, () => { - this.pageStatus.set('Connected to PacMacro.'); - this.geolocation.start((coordinate) => this.socket.sendCoordinate(coordinate)); - }); + this.socket.start( + id, + () => { + this.pageStatus.set('Connected to PacMacro.'); + this.geolocation.start((coordinate) => this.socket.sendCoordinate(coordinate)); + }, + () => void this.autoReregister(), + ); } private cleanup(): void { From f72fbeacd1ff284565dd5cfa93c16e6889e9f498 Mon Sep 17 00:00:00 2001 From: Karen Yao Date: Wed, 19 Aug 2026 02:19:21 -0700 Subject: [PATCH 06/16] fix: restore frontend/angular.json file --- frontend/angular.json | 3 +-- .../app/pages/register-page/register-page.component.spec.ts | 2 +- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/frontend/angular.json b/frontend/angular.json index 21e6be7..1d74f66 100644 --- a/frontend/angular.json +++ b/frontend/angular.json @@ -2,8 +2,7 @@ "$schema": "./node_modules/@angular/cli/lib/config/schema.json", "version": 1, "cli": { - "packageManager": "npm", - "analytics": "132fecc1-3206-4f36-bf3c-2dcfd0f12560" + "packageManager": "npm" }, "newProjectRoot": "projects", "projects": { diff --git a/frontend/src/app/pages/register-page/register-page.component.spec.ts b/frontend/src/app/pages/register-page/register-page.component.spec.ts index 263c45b..9ff5f38 100644 --- a/frontend/src/app/pages/register-page/register-page.component.spec.ts +++ b/frontend/src/app/pages/register-page/register-page.component.spec.ts @@ -34,7 +34,7 @@ describe('RegisterPageComponent', () => { const component = TestBed.createComponent(RegisterPageComponent) .componentInstance as unknown as RegisterPageHarness; component.registrationModel.set({ - name: ' Test2 ', + name: 'Test2', }); await component.submit(submitEvent()); From 9093e93fa09c68ecd2120b858f3a76ce55f2efa4 Mon Sep 17 00:00:00 2001 From: Karen Yao Date: Sun, 23 Aug 2026 20:53:54 -0700 Subject: [PATCH 07/16] feat: handle server restart and graceful shutdown with auto re-registration --- api/admin.go | 1 + api/admin_socket.go | 2 ++ api/admin_test.go | 6 +++++ api/etc.go | 2 ++ api/hub.go | 22 ++++++++++++++++++- api/socket.go | 9 ++++++++ frontend/src/app/core/game.models.ts | 2 +- .../app/core/sockets/game-socket.service.ts | 17 ++++++++++++++ main.go | 21 +++++++++++++++++- 9 files changed, 79 insertions(+), 3 deletions(-) diff --git a/api/admin.go b/api/admin.go index 1834bf4..f066fe0 100644 --- a/api/admin.go +++ b/api/admin.go @@ -143,6 +143,7 @@ func (a *Admin) ServeReset(w http.ResponseWriter, r *http.Request) { if !a.authorizePost(w, r) { return } + a.sockets.BroadcastShutDown(CMD_RESET) changed := a.players.ResetNonLeaders() a.sockets.ClearOfflineLocations() for _, player := range changed { diff --git a/api/admin_socket.go b/api/admin_socket.go index b083ad3..7e0691d 100644 --- a/api/admin_socket.go +++ b/api/admin_socket.go @@ -11,6 +11,8 @@ const ( AdminEventSnapshot = "snapshot" AdminEventUpsert = "upsert" AdminEventFlag = "flag" + AdminEventShutdown = "shutdown" // server is shutting down gracefully; admin should reconnect + AdminEventReset = "reset" // game was reset; admin should reconnect ) type AdminSocketMessage struct { diff --git a/api/admin_test.go b/api/admin_test.go index e6bc556..1744e11 100644 --- a/api/admin_test.go +++ b/api/admin_test.go @@ -274,6 +274,12 @@ func TestAdminResetClearsOfflineLocationsButPreservesActiveCoordinates(t *testin if coordinate := sockets.hub.coordinates[activeID]; coordinate != activeCoordinate { t.Errorf("active coordinate after reset = %#v, want %#v", coordinate, activeCoordinate) } + // ServeReset now broadcasts CMD_RESET before wiping state so that + // connected players know to re-register. Consume that message first. + resetMsg := receiveTestMessage(t, viewer) + if resetMsg.Command != CMD_RESET { + t.Errorf("expected reset signal first, got command = %q", resetMsg.Command) + } message := receiveTestMessage(t, viewer) if message.Command != CMD_REMOVE || message.Data != string(offlineID) { t.Errorf("Admin reset location removal = %#v", message) diff --git a/api/etc.go b/api/etc.go index 15c1b9e..867d0d3 100644 --- a/api/etc.go +++ b/api/etc.go @@ -21,6 +21,8 @@ const ( CMD_INFORM = "inform" // inform another player change/connection CMD_REMOVE = "remove" // remove a player marker without disclosing a location CMD_STATE = "state" // inform clients of shared game state + CMD_SHUTDOWN = "shutdown" // inform clients of server shutdown + CMD_RESET = "reset" // inform clients of server reset // player type TypeHidden PlayerType = 0 diff --git a/api/hub.go b/api/hub.go index b55b4b7..da34e5a 100644 --- a/api/hub.go +++ b/api/hub.go @@ -23,6 +23,12 @@ type Hub struct { inform chan PlayerID state chan GameState clearOffline chan chan struct{} + shutdown chan shutdownEvent +} + +type shutdownEvent struct { + command string + done chan struct{} } func NewHub(players *Players, games ...*Game) *Hub { @@ -38,6 +44,7 @@ func NewHub(players *Players, games ...*Game) *Hub { inform: make(chan PlayerID), state: make(chan GameState), clearOffline: make(chan chan struct{}), + shutdown: make(chan shutdownEvent), } if len(games) > 0 { hub.game = games[0] @@ -61,8 +68,11 @@ func (h *Hub) Run() { case done := <-h.clearOffline: h.clearOfflineLocations() close(done) + case event := <-h.shutdown: + h.broadcastShutDown(event.command) + close(event.done) } - } + } } func isPrivateMapRole(playerType PlayerType) bool { @@ -420,3 +430,13 @@ func (h *Hub) clearOfflineLocations() { h.broadcastRemove(playerID, onlyViewers, nil) } } + +func (h *Hub) broadcastShutDown(command string) { + message, err := json.Marshal(Message{Command: command}) + if err != nil { + return + } + for connection := range h.connections { + h.enqueue(connection, message) + } +} \ No newline at end of file diff --git a/api/socket.go b/api/socket.go index 89c777e..3685fe1 100644 --- a/api/socket.go +++ b/api/socket.go @@ -36,6 +36,15 @@ type Sockets struct { hub *Hub } +// BroadcastShutDown signals every client.. +// CMD_RESET keeps socket open so players can re-register. +// CMD_SHUTDOWN closes everything. +func (s *Sockets) BroadcastShutDown(command string) { + done := make(chan struct{}) + s.hub.shutdown <- shutdownEvent{command: command, done: done} + <-done +} + func (s *Sockets) Init(players *Players, games ...*Game) { s.players = players s.hub = NewHub(players, games...) diff --git a/frontend/src/app/core/game.models.ts b/frontend/src/app/core/game.models.ts index 54a801c..681aa1b 100644 --- a/frontend/src/app/core/game.models.ts +++ b/frontend/src/app/core/game.models.ts @@ -52,7 +52,7 @@ export interface LivePlayer { export interface SocketMessage { coordinate?: Coordinate; - command: 'inform' | 'move' | 'remove' | 'state' | string; + command: 'inform' | 'move' | 'remove' | 'state' | 'shutdown' | 'reset' | string; data: string; } diff --git a/frontend/src/app/core/sockets/game-socket.service.ts b/frontend/src/app/core/sockets/game-socket.service.ts index 8f835b6..00ae3c5 100644 --- a/frontend/src/app/core/sockets/game-socket.service.ts +++ b/frontend/src/app/core/sockets/game-socket.service.ts @@ -200,6 +200,21 @@ export class GameSocketService extends WebSocketService { return; } + if (message.command === 'shutdown') { + // Server is restarting cleanly — let the existing retry pipeline + // reconnect automatically. Just show a friendly status message. + this.statusMessage.set('Server is restarting. Reconnecting automatically…'); + return; + } + + if (message.command === 'reset') { + // Game was reset — this player's ID is no longer valid. + this.statusMessage.set('The game was reset. Redirecting to registration…'); + this.sessionExpired.set(true); + this.onSessionExpired?.(); + return; + } + if (message.command === 'remove') { this.players.update((players) => { if (!(message.data in players)) { @@ -262,6 +277,8 @@ function isSocketMessage(value: unknown): value is SocketMessage { switch (value['command']) { case 'remove': + case 'shutdown': + case 'reset': return true; case 'inform': case 'move': diff --git a/main.go b/main.go index 5531ddb..3905dbb 100644 --- a/main.go +++ b/main.go @@ -5,6 +5,9 @@ import ( "log" "net/http" "os" + "os/signal" + "syscall" + "time" "github.com/joho/godotenv" "pacmacro/api" @@ -81,5 +84,21 @@ func main() { // PacMacro API is served on port 49152. // this should be proxied inside the web server used. - log.Fatal(http.ListenAndServe(port, nil)) + go func() { + if err := http.ListenAndServe(port, nil); err != nil && err != http.ErrServerClosed { + log.Fatal(err) + } + }() + + // 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.") } From ecc9e0883edf8e75fe3fd718602fe24e74c471af Mon Sep 17 00:00:00 2001 From: Karen Yao Date: Sun, 23 Aug 2026 21:50:53 -0700 Subject: [PATCH 08/16] refactor: move player service to credentials --- api/admin_socket.go | 4 +- .../src/app/core/credentials.service.spec.ts | 87 ++++++++++++++++++- frontend/src/app/core/credentials.service.ts | 18 ++++ .../src/app/core/player-name.service.spec.ts | 79 ----------------- frontend/src/app/core/player-name.service.ts | 26 ------ .../core/sockets/game-socket.service.spec.ts | 40 ++------- .../app/core/sockets/game-socket.service.ts | 1 - .../game-page/game-page.component.spec.ts | 16 ++-- .../pages/game-page/game-page.component.ts | 4 +- .../register-page.component.spec.ts | 9 +- .../register-page/register-page.component.ts | 4 +- 11 files changed, 123 insertions(+), 165 deletions(-) delete mode 100644 frontend/src/app/core/player-name.service.spec.ts delete mode 100644 frontend/src/app/core/player-name.service.ts diff --git a/api/admin_socket.go b/api/admin_socket.go index 7e0691d..da91fa8 100644 --- a/api/admin_socket.go +++ b/api/admin_socket.go @@ -11,8 +11,8 @@ const ( AdminEventSnapshot = "snapshot" AdminEventUpsert = "upsert" AdminEventFlag = "flag" - AdminEventShutdown = "shutdown" // server is shutting down gracefully; admin should reconnect - AdminEventReset = "reset" // game was reset; admin should reconnect + AdminEventShutdown = "shutdown" + AdminEventReset = "reset" ) type AdminSocketMessage struct { diff --git a/frontend/src/app/core/credentials.service.spec.ts b/frontend/src/app/core/credentials.service.spec.ts index 90edebb..f168d58 100644 --- a/frontend/src/app/core/credentials.service.spec.ts +++ b/frontend/src/app/core/credentials.service.spec.ts @@ -18,11 +18,27 @@ describe('readCookie', () => { describe('CredentialsService', () => { let service: CredentialsService; let mockDocument: { cookie: string }; - let mockWindow: { location: { protocol: string } }; + let mockStorage: Record; + let mockWindow: { + location: { protocol: string }; + localStorage: { + getItem: (key: string) => string | null; + setItem: (key: string, value: string) => void; + }; + }; beforeEach(() => { mockDocument = { cookie: '' }; - mockWindow = { location: { protocol: 'http:' } }; + mockStorage = {}; + mockWindow = { + location: { protocol: 'http:' }, + localStorage: { + getItem: (key) => mockStorage[key] ?? null, + setItem: (key, value) => { + mockStorage[key] = value; + }, + }, + }; TestBed.configureTestingModule({ providers: [ @@ -60,4 +76,71 @@ describe('CredentialsService', () => { expect(mockDocument.cookie).toBe('id=ABC'); }); + + it('saves and retrieves the player name for auto re-registration', () => { + service.savePlayerName('Odin'); + + expect(service.getPlayerName()).toBe('Odin'); + }); + + it('returns an empty player name when localStorage throws on get', () => { + TestBed.resetTestingModule(); + const throwingWindow = { + location: { protocol: 'http:' }, + localStorage: { + getItem: () => { + throw new Error('unavailable'); + }, + setItem: () => undefined, + }, + }; + TestBed.configureTestingModule({ + providers: [ + CredentialsService, + { provide: DOCUMENT, useValue: mockDocument }, + { provide: PAC_WINDOW, useValue: throwingWindow }, + ], + }); + service = TestBed.inject(CredentialsService); + + expect(service.getPlayerName()).toBe(''); + }); + + it('silently ignores localStorage errors when saving the player name', () => { + TestBed.resetTestingModule(); + const throwingWindow = { + location: { protocol: 'http:' }, + localStorage: { + getItem: () => null, + setItem: () => { + throw new Error('quota exceeded'); + }, + }, + }; + TestBed.configureTestingModule({ + providers: [ + CredentialsService, + { provide: DOCUMENT, useValue: mockDocument }, + { provide: PAC_WINDOW, useValue: throwingWindow }, + ], + }); + service = TestBed.inject(CredentialsService); + + expect(() => service.savePlayerName('Odin')).not.toThrow(); + }); + + it('returns an empty player name and ignores saves when PAC_WINDOW is null', () => { + TestBed.resetTestingModule(); + TestBed.configureTestingModule({ + providers: [ + CredentialsService, + { provide: DOCUMENT, useValue: mockDocument }, + { provide: PAC_WINDOW, useValue: null }, + ], + }); + service = TestBed.inject(CredentialsService); + + expect(service.getPlayerName()).toBe(''); + expect(() => service.savePlayerName('Odin')).not.toThrow(); + }); }); diff --git a/frontend/src/app/core/credentials.service.ts b/frontend/src/app/core/credentials.service.ts index 8c40233..e7fba9f 100644 --- a/frontend/src/app/core/credentials.service.ts +++ b/frontend/src/app/core/credentials.service.ts @@ -4,6 +4,8 @@ import { inject, Service } from '@angular/core'; import { PAC_WINDOW } from './browser-window.token'; import { Credentials } from './game.models'; +const PLAYER_NAME_KEY = 'playerName'; + export function readCookie(cookieHeader: string, name: string): string { const prefix = `${name}=`; const value = cookieHeader @@ -48,6 +50,22 @@ export class CredentialsService { this.document.cookie = `id=${encodeURIComponent(credentials.id)}${attributes}`; } + getPlayerName(): string { + try { + return this.browserWindow?.localStorage.getItem(PLAYER_NAME_KEY) ?? ''; + } catch { + return ''; + } + } + + savePlayerName(name: string): void { + try { + this.browserWindow?.localStorage.setItem(PLAYER_NAME_KEY, name); + } catch { + // Local storage may be unavailable or full; re-registration can fall back to the form. + } + } + clear(): void { if (!this.browserWindow) { return; diff --git a/frontend/src/app/core/player-name.service.spec.ts b/frontend/src/app/core/player-name.service.spec.ts deleted file mode 100644 index 6c70f4e..0000000 --- a/frontend/src/app/core/player-name.service.spec.ts +++ /dev/null @@ -1,79 +0,0 @@ -import { TestBed } from '@angular/core/testing'; - -import { PAC_WINDOW } from './browser-window.token'; -import { PlayerNameService } from './player-name.service'; - -describe('PlayerNameService', () => { - let service: PlayerNameService; - let mockStorage: Record; - - beforeEach(() => { - mockStorage = {}; - const mockWindow = { - localStorage: { - getItem: (key: string) => mockStorage[key] ?? null, - setItem: (key: string, value: string) => { - mockStorage[key] = value; - }, - }, - }; - - TestBed.configureTestingModule({ - providers: [PlayerNameService, { provide: PAC_WINDOW, useValue: mockWindow }], - }); - service = TestBed.inject(PlayerNameService); - }); - - it('returns an empty string when no name has been saved', () => { - expect(service.get()).toBe(''); - }); - - it('saves and retrieves a player name', () => { - service.save('Odin'); - expect(service.get()).toBe('Odin'); - }); - - it('returns an empty string when localStorage throws on get', () => { - TestBed.resetTestingModule(); - const throwingWindow = { - localStorage: { - getItem: () => { - throw new Error('unavailable'); - }, - setItem: () => {}, - }, - }; - TestBed.configureTestingModule({ - providers: [PlayerNameService, { provide: PAC_WINDOW, useValue: throwingWindow }], - }); - service = TestBed.inject(PlayerNameService); - expect(service.get()).toBe(''); - }); - - it('silently ignores localStorage errors on save', () => { - TestBed.resetTestingModule(); - const throwingWindow = { - localStorage: { - getItem: () => null, - setItem: () => { - throw new Error('quota exceeded'); - }, - }, - }; - TestBed.configureTestingModule({ - providers: [PlayerNameService, { provide: PAC_WINDOW, useValue: throwingWindow }], - }); - service = TestBed.inject(PlayerNameService); - expect(() => service.save('Odin')).not.toThrow(); - }); - - it('returns an empty string when PAC_WINDOW is null', () => { - TestBed.resetTestingModule(); - TestBed.configureTestingModule({ - providers: [PlayerNameService, { provide: PAC_WINDOW, useValue: null }], - }); - service = TestBed.inject(PlayerNameService); - expect(service.get()).toBe(''); - expect(() => service.save('Odin')).not.toThrow(); - }); -}); diff --git a/frontend/src/app/core/player-name.service.ts b/frontend/src/app/core/player-name.service.ts deleted file mode 100644 index 4bde22a..0000000 --- a/frontend/src/app/core/player-name.service.ts +++ /dev/null @@ -1,26 +0,0 @@ -import { inject, Service, signal } from '@angular/core'; -import { PAC_WINDOW } from './browser-window.token'; - -const PLAYER_NAME_KEY = 'playerName'; - -@Service() -export class PlayerNameService { - private readonly browserWindow = inject(PAC_WINDOW); - private readonly statusMessage = signal(null); - - get(): string { - try { - return this.browserWindow?.localStorage.getItem(PLAYER_NAME_KEY) ?? ''; - } catch { - return ''; - } - } - - save(name: string): void { - try { - this.browserWindow?.localStorage.setItem(PLAYER_NAME_KEY, name); - } catch { - this.statusMessage.set('Could not save your name for auto-re-registration.'); - } - } -} diff --git a/frontend/src/app/core/sockets/game-socket.service.spec.ts b/frontend/src/app/core/sockets/game-socket.service.spec.ts index 6138836..ea6e3bb 100644 --- a/frontend/src/app/core/sockets/game-socket.service.spec.ts +++ b/frontend/src/app/core/sockets/game-socket.service.spec.ts @@ -298,7 +298,7 @@ describe('GameSocketService', () => { expect(service.status()).toContain('Register as admin again in this browser'); }); - it('expires the session after three consecutive failed player connections', () => { + it('reconnects after two failures and expires the session after the third', () => { vi.useFakeTimers(); vi.spyOn(console, 'log').mockImplementation(() => undefined); vi.spyOn(console, 'warn').mockImplementation(() => undefined); @@ -309,6 +309,12 @@ describe('GameSocketService', () => { vi.advanceTimersByTime(1000); MockGameWebSocket.instances[1].serverClose(false); vi.advanceTimersByTime(2000); + + expect(service.sessionExpired()).toBe(false); + expect(service.state()).toBe('connecting'); + expect(MockGameWebSocket.instances).toHaveLength(3); + expect(onSessionExpired).not.toHaveBeenCalled(); + MockGameWebSocket.instances[2].serverClose(false); vi.runAllTimers(); @@ -319,38 +325,6 @@ describe('GameSocketService', () => { expect(onSessionExpired).toHaveBeenCalledOnce(); }); - it('does not invoke the session-expired callback after fewer than three failures', () => { - vi.useFakeTimers(); - vi.spyOn(console, 'log').mockImplementation(() => undefined); - vi.spyOn(console, 'warn').mockImplementation(() => undefined); - const onSessionExpired = vi.fn(); - service.start('ABCD', () => undefined, onSessionExpired); - - MockGameWebSocket.instances[0].serverClose(false); - vi.advanceTimersByTime(1000); - MockGameWebSocket.instances[1].serverClose(false); - vi.advanceTimersByTime(2000); - - expect(service.sessionExpired()).toBe(false); - expect(onSessionExpired).not.toHaveBeenCalled(); - }); - - it('keeps reconnecting a player socket after fewer than three failures', () => { - vi.useFakeTimers(); - vi.spyOn(console, 'log').mockImplementation(() => undefined); - vi.spyOn(console, 'warn').mockImplementation(() => undefined); - service.start('ABCD', () => undefined); - - MockGameWebSocket.instances[0].serverClose(false); - vi.advanceTimersByTime(1000); - MockGameWebSocket.instances[1].serverClose(false); - vi.advanceTimersByTime(2000); - - expect(service.sessionExpired()).toBe(false); - expect(service.state()).toBe('connecting'); - expect(MockGameWebSocket.instances).toHaveLength(3); - }); - it('resets the failure counter when a player connection succeeds', () => { vi.useFakeTimers(); vi.spyOn(console, 'log').mockImplementation(() => undefined); diff --git a/frontend/src/app/core/sockets/game-socket.service.ts b/frontend/src/app/core/sockets/game-socket.service.ts index 00ae3c5..64e0c97 100644 --- a/frontend/src/app/core/sockets/game-socket.service.ts +++ b/frontend/src/app/core/sockets/game-socket.service.ts @@ -129,7 +129,6 @@ export class GameSocketService extends WebSocketService { this.onSessionExpired?.(); return false; } - return true; } diff --git a/frontend/src/app/pages/game-page/game-page.component.spec.ts b/frontend/src/app/pages/game-page/game-page.component.spec.ts index 5b7dd2e..be8b3f1 100644 --- a/frontend/src/app/pages/game-page/game-page.component.spec.ts +++ b/frontend/src/app/pages/game-page/game-page.component.spec.ts @@ -10,7 +10,6 @@ import { GeolocationService } from '../../core/geolocation.service'; import { MapInfo, PlayerStatus, PlayerType } from '../../core/game.models'; import { WakeLockService } from '../../core/wake-lock.service'; import { GamePageComponent } from './game-page.component'; -import { PlayerNameService } from '../../core/player-name.service'; const map: MapInfo = { min: { latitude: 49.27, longitude: -122.92 }, @@ -26,12 +25,10 @@ const api = { const credentials = { get: vi.fn(() => ({ id: 'SELF' })), save: vi.fn(), + getPlayerName: vi.fn(() => ''), + savePlayerName: vi.fn(), clear: vi.fn(), }; -const playerName = { - get: vi.fn(() => ''), - save: vi.fn(), -}; const router = { navigateByUrl: vi.fn() }; let triggerSessionExpired: (() => void) | null = null; const gameSocket = { @@ -80,7 +77,6 @@ async function configureTestBed(): Promise { providers: [ { provide: ApiService, useValue: api }, { provide: CredentialsService, useValue: credentials }, - { provide: PlayerNameService, useValue: playerName }, { provide: Router, useValue: router }, ], }) @@ -166,7 +162,7 @@ describe('GamePageComponent re-registration', () => { } it('clears credentials and redirects to /register when no name is saved', async () => { - playerName.get.mockReturnValue(''); + credentials.getPlayerName.mockReturnValue(''); await render(); triggerSessionExpired?.(); @@ -178,7 +174,7 @@ describe('GamePageComponent re-registration', () => { }); it('re-registers with the saved name and reconnects', async () => { - playerName.get.mockReturnValue('Odin'); + credentials.getPlayerName.mockReturnValue('Odin'); api.registerPlayer.mockReturnValue(of({ id: 'NEWID' })); const page = await render(); @@ -201,7 +197,7 @@ describe('GamePageComponent re-registration', () => { }); it('clears credentials and redirects when re-registration fails', async () => { - playerName.get.mockReturnValue('Odin'); + credentials.getPlayerName.mockReturnValue('Odin'); api.registerPlayer.mockReturnValue(throwError(() => new Error('API is down'))); const page = await render(); @@ -215,7 +211,7 @@ describe('GamePageComponent re-registration', () => { }); it('treats an empty player ID from the API as a failure', async () => { - playerName.get.mockReturnValue('Odin'); + credentials.getPlayerName.mockReturnValue('Odin'); api.registerPlayer.mockReturnValue(of({ id: ' ' })); const page = await render(); diff --git a/frontend/src/app/pages/game-page/game-page.component.ts b/frontend/src/app/pages/game-page/game-page.component.ts index 3574ae2..106a960 100644 --- a/frontend/src/app/pages/game-page/game-page.component.ts +++ b/frontend/src/app/pages/game-page/game-page.component.ts @@ -19,7 +19,6 @@ import { isLeaderType, MapInfo, typeLabel } from '../../core/game.models'; import { WakeLockService } from '../../core/wake-lock.service'; import { GameCanvasComponent } from '../../game/game-canvas/game-canvas.component'; import { BrandHeaderComponent } from '../../shared/brand-header/brand-header.component'; -import { PlayerNameService } from '../../core/player-name.service'; @Component({ selector: 'pac-game-page', @@ -35,7 +34,6 @@ export class GamePageComponent { private readonly credentials = inject(CredentialsService); private readonly destroyRef = inject(DestroyRef); private readonly router = inject(Router); - private readonly playerName = inject(PlayerNameService); protected readonly socket = inject(GameSocketService); protected readonly geolocation = inject(GeolocationService); @@ -72,7 +70,7 @@ export class GamePageComponent { } private async autoReregister(): Promise { - const name = this.playerName.get(); + const name = this.credentials.getPlayerName(); if (!name) { this.credentials.clear(); this.socket.stop(); diff --git a/frontend/src/app/pages/register-page/register-page.component.spec.ts b/frontend/src/app/pages/register-page/register-page.component.spec.ts index 9ff5f38..353517c 100644 --- a/frontend/src/app/pages/register-page/register-page.component.spec.ts +++ b/frontend/src/app/pages/register-page/register-page.component.spec.ts @@ -5,7 +5,6 @@ import { of } from 'rxjs'; import { ApiService } from '../../core/api.service'; import { CredentialsService } from '../../core/credentials.service'; -import { PlayerNameService } from '../../core/player-name.service'; import { RegisterPageComponent } from './register-page.component'; describe('RegisterPageComponent', () => { @@ -13,8 +12,7 @@ describe('RegisterPageComponent', () => { registerAdmin: vi.fn(() => of(void 0)), registerPlayer: vi.fn(() => of({ id: 'ABCD' })), }; - const credentials = { save: vi.fn() }; - const playerName = { save: vi.fn() }; + const credentials = { save: vi.fn(), savePlayerName: vi.fn() }; const router = { navigateByUrl: vi.fn(() => Promise.resolve(true)) }; beforeEach(() => { @@ -24,7 +22,6 @@ describe('RegisterPageComponent', () => { providers: [ { provide: ApiService, useValue: api }, { provide: CredentialsService, useValue: credentials }, - { provide: PlayerNameService, useValue: playerName }, { provide: Router, useValue: router }, ], }); @@ -42,7 +39,7 @@ describe('RegisterPageComponent', () => { expect(api.registerPlayer).toHaveBeenCalledWith('Test2'); expect(api.registerAdmin).not.toHaveBeenCalled(); expect(credentials.save).toHaveBeenCalledWith({ id: 'ABCD' }); - expect(playerName.save).toHaveBeenCalledWith('Test2'); + expect(credentials.savePlayerName).toHaveBeenCalledWith('Test2'); expect(router.navigateByUrl).toHaveBeenCalledWith('/'); }); @@ -57,7 +54,7 @@ describe('RegisterPageComponent', () => { expect(api.registerPlayer).not.toHaveBeenCalled(); expect(credentials.save).not.toHaveBeenCalled(); - expect(playerName.save).not.toHaveBeenCalled(); + expect(credentials.savePlayerName).not.toHaveBeenCalled(); }); }); diff --git a/frontend/src/app/pages/register-page/register-page.component.ts b/frontend/src/app/pages/register-page/register-page.component.ts index 6b251ca..6330212 100644 --- a/frontend/src/app/pages/register-page/register-page.component.ts +++ b/frontend/src/app/pages/register-page/register-page.component.ts @@ -12,7 +12,6 @@ import { firstValueFrom } from 'rxjs'; import { ApiService } from '../../core/api.service'; import { CredentialsService } from '../../core/credentials.service'; import { BrandHeaderComponent } from '../../shared/brand-header/brand-header.component'; -import { PlayerNameService } from '../../core/player-name.service'; interface RegistrationModel { name: string; @@ -29,7 +28,6 @@ export class RegisterPageComponent { private readonly api = inject(ApiService); private readonly credentials = inject(CredentialsService); private readonly router = inject(Router); - private readonly playerName = inject(PlayerNameService); protected readonly registrationModel = signal({ name: '', @@ -64,7 +62,7 @@ export class RegisterPageComponent { throw new Error('The API returned an empty player ID.'); } this.credentials.save({ id }); - this.playerName.save(trimmedName); + this.credentials.savePlayerName(trimmedName); await this.router.navigateByUrl('/'); } catch (error) { this.status.set('Registration failed. Check your details and the API connection.'); From bc620001bf860053dd91f5ae2ab3f1980f451bc5 Mon Sep 17 00:00:00 2001 From: Karen Yao Date: Tue, 25 Aug 2026 15:13:14 -0700 Subject: [PATCH 09/16] feat: end client sessions on server shutdown instead of auto-reconnecting --- api/admin_socket.go | 2 - api/socket_test.go | 16 +++++++ .../core/sockets/game-socket.service.spec.ts | 38 +++++++++++++++++ .../app/core/sockets/game-socket.service.ts | 42 +++++++++++++++++-- .../src/app/core/sockets/websocket.service.ts | 4 ++ .../game-page/game-page.component.spec.ts | 32 ++++++++++++-- .../pages/game-page/game-page.component.ts | 6 +++ .../register-page.component.spec.ts | 23 +++++++++- .../register-page/register-page.component.ts | 19 ++++++++- 9 files changed, 169 insertions(+), 13 deletions(-) diff --git a/api/admin_socket.go b/api/admin_socket.go index da91fa8..b083ad3 100644 --- a/api/admin_socket.go +++ b/api/admin_socket.go @@ -11,8 +11,6 @@ const ( AdminEventSnapshot = "snapshot" AdminEventUpsert = "upsert" AdminEventFlag = "flag" - AdminEventShutdown = "shutdown" - AdminEventReset = "reset" ) type AdminSocketMessage struct { diff --git a/api/socket_test.go b/api/socket_test.go index 0ea15a0..697deb9 100644 --- a/api/socket_test.go +++ b/api/socket_test.go @@ -31,6 +31,22 @@ func TestPlayerStaysConnectedUntilLastSocketDisconnects(t *testing.T) { } } +func TestBroadcastShutdownQueuesShutdownCommand(t *testing.T) { + players := new(Players) + players.Init() + playerID := players.New(TypeGhost, "Player", StatusDisc) + hub := NewHub(players) + connection := newTestConnection(playerID) + hub.registerConnection(connection) + drainTestMessages(connection) + + hub.broadcastShutDown(CMD_SHUTDOWN) + message := receiveTestMessage(t, connection) + if message.Command != CMD_SHUTDOWN { + t.Errorf("shutdown command = %q, want %q", message.Command, CMD_SHUTDOWN) + } +} + func TestGameStateSnapshotAndBroadcastDoNotChangePlayerConnectionCounts(t *testing.T) { players := new(Players) players.Init() diff --git a/frontend/src/app/core/sockets/game-socket.service.spec.ts b/frontend/src/app/core/sockets/game-socket.service.spec.ts index ea6e3bb..c183d2f 100644 --- a/frontend/src/app/core/sockets/game-socket.service.spec.ts +++ b/frontend/src/app/core/sockets/game-socket.service.spec.ts @@ -203,6 +203,44 @@ describe('GameSocketService', () => { expect(service.status()).toContain('invalid game update'); }); + it('ends a player session when the server sends shutdown', () => { + vi.useFakeTimers(); + MockGameWebSocket.closeSynchronously = false; + const onSessionExpired = vi.fn(); + const onServerShutdown = vi.fn(); + service.start('ABCD', () => undefined, onSessionExpired, onServerShutdown); + const first = MockGameWebSocket.instances[0]; + first.open(); + const lateClose = first.onclose; + + first.message({ command: 'shutdown', data: '' }); + lateClose?.(new CloseEvent('close', { code: 1006, wasClean: false })); + service.resume(); + vi.runAllTimers(); + + expect(service.state()).toBe('shutdown'); + expect(service.status()).toBe('The server stopped. Register to join the next game.'); + expect(MockGameWebSocket.instances).toHaveLength(1); + expect(onServerShutdown).toHaveBeenCalledOnce(); + expect(onSessionExpired).not.toHaveBeenCalled(); + expect(service.sendCoordinate({ latitude: 49.2, longitude: -123 })).toBe(false); + }); + + it('ends a viewer session when the server sends shutdown', () => { + vi.useFakeTimers(); + service.startViewer(); + const first = MockGameWebSocket.instances[0]; + first.open(); + + first.message({ command: 'shutdown', data: '' }); + service.resume(); + vi.runAllTimers(); + + expect(service.state()).toBe('shutdown'); + expect(service.status()).toBe('The server stopped the admin map connection.'); + expect(MockGameWebSocket.instances).toHaveLength(1); + }); + it.each([ ['clean', true], ['abnormal', false], diff --git a/frontend/src/app/core/sockets/game-socket.service.ts b/frontend/src/app/core/sockets/game-socket.service.ts index 64e0c97..1bab16b 100644 --- a/frontend/src/app/core/sockets/game-socket.service.ts +++ b/frontend/src/app/core/sockets/game-socket.service.ts @@ -30,6 +30,7 @@ export class GameSocketService extends WebSocketService { private mode: SocketMode | null = null; private onConnected: (() => void) | null = null; private onSessionExpired: (() => void) | null = null; + private onServerShutdown: (() => void) | null = null; private reconnecting = false; private suspendedReason = 'Paused while the browser is offline.'; private consecutiveFailures = 0; @@ -40,12 +41,18 @@ export class GameSocketService extends WebSocketService { readonly sessionExpired = signal(false); readonly MAX_FAILED_ATTEMPTS = 3; - start(id: string, onConnected: () => void, onSessionExpired: () => void = () => undefined): void { + start( + id: string, + onConnected: () => void, + onSessionExpired: () => void = () => undefined, + onServerShutdown: () => void = () => undefined, + ): void { this.stop(); this.mode = 'player'; this.playerId = id; this.onConnected = onConnected; this.onSessionExpired = onSessionExpired; + this.onServerShutdown = onServerShutdown; this.consecutiveFailures = 0; this.sessionExpired.set(false); this.resume(); @@ -80,6 +87,7 @@ export class GameSocketService extends WebSocketService { this.playerId = null; this.onConnected = null; this.onSessionExpired = null; + this.onServerShutdown = null; this.reconnecting = false; this.statusMessage.set(null); this.consecutiveFailures = 0; @@ -91,6 +99,34 @@ export class GameSocketService extends WebSocketService { return this.mode === 'player' && isCoordinate(coordinate) && this.sendMessage(coordinate); } + private endForServerShutdown(): void { + const mode = this.mode; + const onServerShutdown = this.onServerShutdown; + + this.mode = null; + this.playerId = null; + this.onConnected = null; + this.onSessionExpired = null; + this.onServerShutdown = null; + this.reconnecting = false; + this.consecutiveFailures = 0; + this.sessionExpired.set(false); + + // Incrementing the connection identity before unsubscribing makes any + // queued retry or late close event belong to an obsolete connection. + this.disconnect(); + this.state.set('shutdown'); + this.statusMessage.set( + mode === 'player' + ? 'The server stopped. Register to join the next game.' + : 'The server stopped the admin map connection.', + ); + + if (mode === 'player') { + onServerShutdown?.(); + } + } + setInitialState(state: GameState): void { this.isFlagFound.set(state.isFlagFound); } @@ -200,9 +236,7 @@ export class GameSocketService extends WebSocketService { } if (message.command === 'shutdown') { - // Server is restarting cleanly — let the existing retry pipeline - // reconnect automatically. Just show a friendly status message. - this.statusMessage.set('Server is restarting. Reconnecting automatically…'); + this.endForServerShutdown(); return; } diff --git a/frontend/src/app/core/sockets/websocket.service.ts b/frontend/src/app/core/sockets/websocket.service.ts index 8c090c8..94062da 100644 --- a/frontend/src/app/core/sockets/websocket.service.ts +++ b/frontend/src/app/core/sockets/websocket.service.ts @@ -11,6 +11,7 @@ export type TransportState = | 'offline' // No network activity detected | 'suspended' // The client has deliberately paused transport | 'revoked' // For leaders that have been demoted to non-leaders + | 'shutdown' // The server deliberately ended the session | 'error'; // An error has occurred, the client may be attempting to reconnect @Service({ autoProvided: false }) @@ -281,6 +282,9 @@ export abstract class WebSocketService { case 'revoked': { return 'Websocket access revoked.'; } + case 'shutdown': { + return 'The server has stopped this connection.'; + } case 'error': { return 'Error with the websocket.'; } diff --git a/frontend/src/app/pages/game-page/game-page.component.spec.ts b/frontend/src/app/pages/game-page/game-page.component.spec.ts index be8b3f1..d383d0f 100644 --- a/frontend/src/app/pages/game-page/game-page.component.spec.ts +++ b/frontend/src/app/pages/game-page/game-page.component.spec.ts @@ -31,6 +31,7 @@ const credentials = { }; const router = { navigateByUrl: vi.fn() }; let triggerSessionExpired: (() => void) | null = null; +let triggerServerShutdown: (() => void) | null = null; const gameSocket = { players: signal({ SELF: { @@ -46,10 +47,18 @@ const gameSocket = { status: signal('Connected.'), isFlagFound: signal(false), sessionExpired: signal(false), - start: vi.fn((_id: string, _onConnected: () => void, onSessionExpired: () => void) => { - triggerSessionExpired = onSessionExpired; - gameSocket.sessionExpired.set(false); - }), + start: vi.fn( + ( + _id: string, + _onConnected: () => void, + onSessionExpired: () => void, + onServerShutdown: () => void, + ) => { + triggerSessionExpired = onSessionExpired; + triggerServerShutdown = onServerShutdown; + gameSocket.sessionExpired.set(false); + }, + ), stop: vi.fn(() => gameSocket.sessionExpired.set(false)), resume: vi.fn(), suspend: vi.fn(), @@ -144,6 +153,7 @@ describe('GamePageComponent re-registration', () => { vi.clearAllMocks(); gameSocket.sessionExpired.set(false); triggerSessionExpired = null; + triggerServerShutdown = null; await configureTestBed(); }); @@ -196,6 +206,20 @@ describe('GamePageComponent re-registration', () => { expect(page.textContent).toContain('Connected to PacMacro.'); }); + it('clears the player session and redirects when the server shuts down', async () => { + await render(); + + triggerServerShutdown?.(); + await flushReRegistration(); + + expect(geolocation.stop).toHaveBeenCalled(); + expect(credentials.clear).toHaveBeenCalled(); + expect(router.navigateByUrl).toHaveBeenCalledWith('/register', { + state: { serverStopped: true }, + }); + expect(api.registerPlayer).not.toHaveBeenCalled(); + }); + it('clears credentials and redirects when re-registration fails', async () => { credentials.getPlayerName.mockReturnValue('Odin'); api.registerPlayer.mockReturnValue(throwError(() => new Error('API is down'))); diff --git a/frontend/src/app/pages/game-page/game-page.component.ts b/frontend/src/app/pages/game-page/game-page.component.ts index 106a960..1f2c9e1 100644 --- a/frontend/src/app/pages/game-page/game-page.component.ts +++ b/frontend/src/app/pages/game-page/game-page.component.ts @@ -59,6 +59,11 @@ export class GamePageComponent { this.geolocation.stop(); this.socket.suspend('Offline. Waiting for a network connection…'); }; + private readonly onServerShutdown = () => { + this.geolocation.stop(); + this.credentials.clear(); + void this.router.navigateByUrl('/register', { state: { serverStopped: true } }); + }; constructor() { afterNextRender(() => void this.initialize()); @@ -138,6 +143,7 @@ export class GamePageComponent { this.geolocation.start((coordinate) => this.socket.sendCoordinate(coordinate)); }, () => void this.autoReregister(), + this.onServerShutdown, ); } diff --git a/frontend/src/app/pages/register-page/register-page.component.spec.ts b/frontend/src/app/pages/register-page/register-page.component.spec.ts index 353517c..8c4b0df 100644 --- a/frontend/src/app/pages/register-page/register-page.component.spec.ts +++ b/frontend/src/app/pages/register-page/register-page.component.spec.ts @@ -1,3 +1,4 @@ +import { Location } from '@angular/common'; import { WritableSignal } from '@angular/core'; import { TestBed } from '@angular/core/testing'; import { Router } from '@angular/router'; @@ -12,16 +13,19 @@ describe('RegisterPageComponent', () => { registerAdmin: vi.fn(() => of(void 0)), registerPlayer: vi.fn(() => of({ id: 'ABCD' })), }; - const credentials = { save: vi.fn(), savePlayerName: vi.fn() }; + const credentials = { save: vi.fn(), savePlayerName: vi.fn(), getPlayerName: vi.fn(() => '') }; + const location = { getState: vi.fn(() => ({})) }; const router = { navigateByUrl: vi.fn(() => Promise.resolve(true)) }; beforeEach(() => { vi.clearAllMocks(); + location.getState.mockReturnValue({}); TestBed.configureTestingModule({ imports: [RegisterPageComponent], providers: [ { provide: ApiService, useValue: api }, { provide: CredentialsService, useValue: credentials }, + { provide: Location, useValue: location }, { provide: Router, useValue: router }, ], }); @@ -56,12 +60,29 @@ describe('RegisterPageComponent', () => { expect(credentials.save).not.toHaveBeenCalled(); expect(credentials.savePlayerName).not.toHaveBeenCalled(); }); + + it('pre-fills the name from the saved player name', () => { + credentials.getPlayerName.mockReturnValue('SavedPlayer'); + const component = TestBed.createComponent(RegisterPageComponent) + .componentInstance as unknown as RegisterPageHarness; + + expect(component.registrationModel().name).toBe('SavedPlayer'); + }); + + it('shows the shutdown message when routed from a stopped server', () => { + location.getState.mockReturnValue({ serverStopped: true }); + const component = TestBed.createComponent(RegisterPageComponent) + .componentInstance as unknown as RegisterPageHarness; + + expect(component.status()).toBe('The server stopped. Register to join the next game.'); + }); }); interface RegisterPageHarness { registrationModel: WritableSignal<{ name: string; }>; + status: WritableSignal; submit(event: SubmitEvent): Promise; } diff --git a/frontend/src/app/pages/register-page/register-page.component.ts b/frontend/src/app/pages/register-page/register-page.component.ts index 6330212..91a5764 100644 --- a/frontend/src/app/pages/register-page/register-page.component.ts +++ b/frontend/src/app/pages/register-page/register-page.component.ts @@ -1,3 +1,4 @@ +import { Location } from '@angular/common'; import { ChangeDetectionStrategy, Component, inject, signal } from '@angular/core'; import { form, @@ -27,10 +28,11 @@ interface RegistrationModel { export class RegisterPageComponent { private readonly api = inject(ApiService); private readonly credentials = inject(CredentialsService); + private readonly location = inject(Location); private readonly router = inject(Router); protected readonly registrationModel = signal({ - name: '', + name: this.credentials.getPlayerName() ?? '', }); protected readonly registrationForm = form(this.registrationModel, (registration) => { @@ -38,7 +40,11 @@ export class RegisterPageComponent { maxLength(registration.name, 80, { message: 'Your name must be 80 characters or fewer.' }); }); - protected readonly status = signal(''); + protected readonly status = signal( + isServerStoppedNavigation(this.location.getState()) + ? 'The server stopped. Register to join the next game.' + : '', + ); protected async submit(event: SubmitEvent): Promise { event.preventDefault(); @@ -69,3 +75,12 @@ export class RegisterPageComponent { } } } + +function isServerStoppedNavigation(state: unknown): boolean { + return ( + typeof state === 'object' && + state !== null && + 'serverStopped' in state && + state.serverStopped === true + ); +} From 461f103fae29ee7f8511efb6108bc0300b9ff7f1 Mon Sep 17 00:00:00 2001 From: Karen Yao Date: Tue, 25 Aug 2026 15:54:29 -0700 Subject: [PATCH 10/16] fix: brittle unit tests with timers --- .../src/app/core/credentials.service.spec.ts | 6 ++-- .../core/sockets/game-socket.service.spec.ts | 32 +++++++++-------- .../game-page/game-page.component.spec.ts | 36 +++++++++++-------- 3 files changed, 41 insertions(+), 33 deletions(-) diff --git a/frontend/src/app/core/credentials.service.spec.ts b/frontend/src/app/core/credentials.service.spec.ts index f168d58..18fbdf8 100644 --- a/frontend/src/app/core/credentials.service.spec.ts +++ b/frontend/src/app/core/credentials.service.spec.ts @@ -6,12 +6,12 @@ import { CredentialsService, readCookie } from './credentials.service'; describe('readCookie', () => { it('reads and decodes an exact cookie name', () => { - expect(readCookie('theme=dark; id=AB%20CD; userid=wrong', 'id')).toBe('AB CD'); + expect(readCookie('pacmacro_admin=token; id=AB%20CD; userid=wrong', 'id')).toBe('AB CD'); }); it('returns an empty string for missing or malformed values', () => { expect(readCookie('id=%E0%A4%A', 'id')).toBe(''); - expect(readCookie('theme=dark', 'id')).toBe(''); + expect(readCookie('pacmacro_admin=token', 'id')).toBe(''); }); }); @@ -52,7 +52,7 @@ describe('CredentialsService', () => { it.each(['http:', 'https:'] as const)('expires the id cookie over %s', (protocol) => { mockWindow.location.protocol = protocol; - mockDocument.cookie = 'id=ABC; theme=dark'; + mockDocument.cookie = 'id=ABC; pacmacro_admin=token'; service.clear(); diff --git a/frontend/src/app/core/sockets/game-socket.service.spec.ts b/frontend/src/app/core/sockets/game-socket.service.spec.ts index c183d2f..2d0e036 100644 --- a/frontend/src/app/core/sockets/game-socket.service.spec.ts +++ b/frontend/src/app/core/sockets/game-socket.service.spec.ts @@ -93,6 +93,10 @@ describe('GameSocketService', () => { } }); + function advanceToNextReconnect(): void { + vi.advanceTimersToNextTimer(); + } + it('uses the player URL, invokes the callback, and sends coordinates as JSON', () => { const onConnected = vi.fn(); service.start('A B/C', onConnected); @@ -265,7 +269,7 @@ describe('GameSocketService', () => { first.serverClose(wasClean); expect(service.status()).toContain('Admin map connection lost'); - vi.advanceTimersByTime(1000); + advanceToNextReconnect(); expect(MockGameWebSocket.instances).toHaveLength(2); const second = MockGameWebSocket.instances[1]; @@ -344,9 +348,9 @@ describe('GameSocketService', () => { service.start('ABCD', () => undefined, onSessionExpired); MockGameWebSocket.instances[0].serverClose(false); - vi.advanceTimersByTime(1000); + advanceToNextReconnect(); MockGameWebSocket.instances[1].serverClose(false); - vi.advanceTimersByTime(2000); + advanceToNextReconnect(); expect(service.sessionExpired()).toBe(false); expect(service.state()).toBe('connecting'); @@ -354,7 +358,6 @@ describe('GameSocketService', () => { expect(onSessionExpired).not.toHaveBeenCalled(); MockGameWebSocket.instances[2].serverClose(false); - vi.runAllTimers(); expect(service.sessionExpired()).toBe(true); expect(service.state()).toBe('error'); @@ -370,15 +373,15 @@ describe('GameSocketService', () => { service.start('ABCD', () => undefined); MockGameWebSocket.instances[0].serverClose(false); - vi.advanceTimersByTime(1000); + advanceToNextReconnect(); MockGameWebSocket.instances[1].serverClose(false); - vi.advanceTimersByTime(2000); + advanceToNextReconnect(); MockGameWebSocket.instances[2].open(); MockGameWebSocket.instances[2].serverClose(false); - vi.advanceTimersByTime(4000); + advanceToNextReconnect(); MockGameWebSocket.instances[3].serverClose(false); - vi.runAllTimers(); + advanceToNextReconnect(); expect(service.sessionExpired()).toBe(false); expect(MockGameWebSocket.instances).toHaveLength(5); @@ -391,14 +394,14 @@ describe('GameSocketService', () => { service.startViewer(); MockGameWebSocket.instances[0].serverClose(false); - vi.advanceTimersByTime(1000); + advanceToNextReconnect(); MockGameWebSocket.instances[1].serverClose(false); - vi.advanceTimersByTime(2000); + advanceToNextReconnect(); MockGameWebSocket.instances[2].serverClose(false); - vi.runAllTimers(); + advanceToNextReconnect(); expect(service.sessionExpired()).toBe(false); - expect(MockGameWebSocket.instances.length).toBeGreaterThan(3); + expect(MockGameWebSocket.instances).toHaveLength(4); }); it('start and stop clear an expired session', () => { @@ -408,11 +411,10 @@ describe('GameSocketService', () => { service.start('ABCD', () => undefined); MockGameWebSocket.instances[0].serverClose(false); - vi.advanceTimersByTime(1000); + advanceToNextReconnect(); MockGameWebSocket.instances[1].serverClose(false); - vi.advanceTimersByTime(2000); + advanceToNextReconnect(); MockGameWebSocket.instances[2].serverClose(false); - vi.runAllTimers(); expect(service.sessionExpired()).toBe(true); diff --git a/frontend/src/app/pages/game-page/game-page.component.spec.ts b/frontend/src/app/pages/game-page/game-page.component.spec.ts index d383d0f..50ed35e 100644 --- a/frontend/src/app/pages/game-page/game-page.component.spec.ts +++ b/frontend/src/app/pages/game-page/game-page.component.spec.ts @@ -160,27 +160,22 @@ describe('GamePageComponent re-registration', () => { async function render(): Promise { fixture = TestBed.createComponent(GamePageComponent); fixture.detectChanges(); - await new Promise((resolve) => setTimeout(resolve, 0)); + await fixture.whenStable(); fixture.detectChanges(); return fixture.nativeElement as HTMLElement; } - async function flushReRegistration(): Promise { - fixture.detectChanges(); - await new Promise((resolve) => setTimeout(resolve, 0)); - fixture.detectChanges(); - } - it('clears credentials and redirects to /register when no name is saved', async () => { credentials.getPlayerName.mockReturnValue(''); await render(); triggerSessionExpired?.(); - await flushReRegistration(); + await vi.waitFor(() => { + expect(router.navigateByUrl).toHaveBeenCalledWith('/register'); + }); expect(credentials.clear).toHaveBeenCalled(); expect(gameSocket.stop).toHaveBeenCalled(); - expect(router.navigateByUrl).toHaveBeenCalledWith('/register'); }); it('re-registers with the saved name and reconnects', async () => { @@ -189,7 +184,15 @@ describe('GamePageComponent re-registration', () => { const page = await render(); triggerSessionExpired?.(); - await flushReRegistration(); + await vi.waitFor(() => { + expect(gameSocket.start).toHaveBeenLastCalledWith( + 'NEWID', + expect.any(Function), + expect.any(Function), + expect.any(Function), + ); + }); + fixture.detectChanges(); expect(api.registerPlayer).toHaveBeenCalledWith('Odin'); expect(credentials.save).toHaveBeenCalledWith({ id: 'NEWID' }); @@ -210,7 +213,6 @@ describe('GamePageComponent re-registration', () => { await render(); triggerServerShutdown?.(); - await flushReRegistration(); expect(geolocation.stop).toHaveBeenCalled(); expect(credentials.clear).toHaveBeenCalled(); @@ -226,11 +228,13 @@ describe('GamePageComponent re-registration', () => { const page = await render(); triggerSessionExpired?.(); - await flushReRegistration(); + await vi.waitFor(() => { + expect(router.navigateByUrl).toHaveBeenCalledWith('/register'); + }); + fixture.detectChanges(); expect(credentials.clear).toHaveBeenCalled(); expect(gameSocket.stop).toHaveBeenCalled(); - expect(router.navigateByUrl).toHaveBeenCalledWith('/register'); expect(page.textContent).toContain('Could not re-register. Redirecting…'); }); @@ -240,11 +244,13 @@ describe('GamePageComponent re-registration', () => { const page = await render(); triggerSessionExpired?.(); - await flushReRegistration(); + await vi.waitFor(() => { + expect(router.navigateByUrl).toHaveBeenCalledWith('/register'); + }); + fixture.detectChanges(); expect(credentials.clear).toHaveBeenCalled(); expect(gameSocket.stop).toHaveBeenCalled(); - expect(router.navigateByUrl).toHaveBeenCalledWith('/register'); expect(page.textContent).toContain('Could not re-register. Redirecting…'); }); }); From 2d9edf589eb98ee23a7bf6bc014ec15a3307009c Mon Sep 17 00:00:00 2001 From: Karen Yao Date: Tue, 25 Aug 2026 17:09:44 -0700 Subject: [PATCH 11/16] fix: remove duplicate code that duplicated player reset behaviour --- api/admin.go | 1 - api/admin_test.go | 62 +++++++++++++++++-- api/etc.go | 1 - api/hub.go | 6 +- api/socket.go | 4 +- frontend/src/app/core/game.models.ts | 2 +- .../app/core/sockets/game-socket.service.ts | 9 --- .../register-page/register-page.component.ts | 6 +- 8 files changed, 64 insertions(+), 27 deletions(-) diff --git a/api/admin.go b/api/admin.go index f066fe0..1834bf4 100644 --- a/api/admin.go +++ b/api/admin.go @@ -143,7 +143,6 @@ func (a *Admin) ServeReset(w http.ResponseWriter, r *http.Request) { if !a.authorizePost(w, r) { return } - a.sockets.BroadcastShutDown(CMD_RESET) changed := a.players.ResetNonLeaders() a.sockets.ClearOfflineLocations() for _, player := range changed { diff --git a/api/admin_test.go b/api/admin_test.go index 1744e11..52fea12 100644 --- a/api/admin_test.go +++ b/api/admin_test.go @@ -174,6 +174,62 @@ func TestAdminResetPreservesLeadersAndClearsFlag(t *testing.T) { } } +func TestAdminResetPreservesConnectedSessionsAndLeaderAuthorization(t *testing.T) { + players := new(Players) + players.Init() + game := new(Game) + sockets := new(Sockets) + sockets.Init(players, game) + admin := new(Admin) + admin.Init(players, sockets, "top-secret", game) + cookie := registerTestAdmin(t, admin, "top-secret") + + leaderID := players.New(TypeAntiPacLeader, "Leader", StatusDisc) + activeID := players.New(TypePacman, "Active", StatusDisc) + leaderConnection := newTestConnection(leaderID) + activeConnection := newTestConnection(activeID) + sockets.hub.registerConnection(leaderConnection) + sockets.hub.registerConnection(activeConnection) + drainTestMessages(leaderConnection) + drainTestMessages(activeConnection) + + request := httptest.NewRequest(http.MethodPost, "/api/admin/reset", nil) + request.AddCookie(cookie) + response := httptest.NewRecorder() + admin.ServeHTTP(response, request) + if response.Code != http.StatusNoContent { + t.Fatalf("reset status = %d, want 204", response.Code) + } + + if len(players.players) != 2 { + t.Errorf("player count after reset = %d, want 2", len(players.players)) + } + if leader := players.Get(leaderID); leader == nil || leader.Type != TypeAntiPacLeader { + t.Errorf("leader after reset = %#v, want AntiPac Leader", leader) + } + if active := players.Get(activeID); active == nil || active.Type != TypeGhost { + t.Errorf("active player after reset = %#v, want Ghost", active) + } + if !sockets.hub.hasConnectionForID(leaderID) || !sockets.hub.hasConnectionForID(activeID) { + t.Error("admin reset replaced a connected player session") + } + if leader, _, authorized := players.LeaderState(leaderID); !authorized || leader.ID != leaderID || leader.Type != TypeAntiPacLeader { + t.Errorf("leader authorization after reset = %#v, authorized %v", leader, authorized) + } + + leaderUpdate := informPlayer(t, receiveTestMessage(t, leaderConnection)) + if leaderUpdate.ID != activeID || leaderUpdate.Type != TypeGhost { + t.Errorf("leader's active-player reset update = %#v, want Ghost for %q", leaderUpdate, activeID) + } + updated := informPlayer(t, receiveTestMessage(t, activeConnection)) + if updated.ID != activeID || updated.Type != TypeGhost { + t.Errorf("active reset update = %#v, want Ghost for %q", updated, activeID) + } + if len(leaderConnection.send) != 0 || len(activeConnection.send) != 0 { + t.Errorf("unexpected extra reset messages: leader=%d active=%d", len(leaderConnection.send), len(activeConnection.send)) + } +} + func TestAdminFlagUpdatesSharedStateAndSocketClients(t *testing.T) { players := new(Players) players.Init() @@ -274,12 +330,6 @@ func TestAdminResetClearsOfflineLocationsButPreservesActiveCoordinates(t *testin if coordinate := sockets.hub.coordinates[activeID]; coordinate != activeCoordinate { t.Errorf("active coordinate after reset = %#v, want %#v", coordinate, activeCoordinate) } - // ServeReset now broadcasts CMD_RESET before wiping state so that - // connected players know to re-register. Consume that message first. - resetMsg := receiveTestMessage(t, viewer) - if resetMsg.Command != CMD_RESET { - t.Errorf("expected reset signal first, got command = %q", resetMsg.Command) - } message := receiveTestMessage(t, viewer) if message.Command != CMD_REMOVE || message.Data != string(offlineID) { t.Errorf("Admin reset location removal = %#v", message) diff --git a/api/etc.go b/api/etc.go index 867d0d3..bf9f6dc 100644 --- a/api/etc.go +++ b/api/etc.go @@ -22,7 +22,6 @@ const ( CMD_REMOVE = "remove" // remove a player marker without disclosing a location CMD_STATE = "state" // inform clients of shared game state CMD_SHUTDOWN = "shutdown" // inform clients of server shutdown - CMD_RESET = "reset" // inform clients of server reset // player type TypeHidden PlayerType = 0 diff --git a/api/hub.go b/api/hub.go index da34e5a..ce5e181 100644 --- a/api/hub.go +++ b/api/hub.go @@ -27,8 +27,8 @@ type Hub struct { } type shutdownEvent struct { - command string - done chan struct{} + command string + done chan struct{} } func NewHub(players *Players, games ...*Game) *Hub { @@ -439,4 +439,4 @@ func (h *Hub) broadcastShutDown(command string) { for connection := range h.connections { h.enqueue(connection, message) } -} \ No newline at end of file +} diff --git a/api/socket.go b/api/socket.go index 3685fe1..53fea32 100644 --- a/api/socket.go +++ b/api/socket.go @@ -36,9 +36,7 @@ type Sockets struct { hub *Hub } -// BroadcastShutDown signals every client.. -// CMD_RESET keeps socket open so players can re-register. -// CMD_SHUTDOWN closes everything. +// BroadcastShutDown signals every client about server shutdown. func (s *Sockets) BroadcastShutDown(command string) { done := make(chan struct{}) s.hub.shutdown <- shutdownEvent{command: command, done: done} diff --git a/frontend/src/app/core/game.models.ts b/frontend/src/app/core/game.models.ts index 681aa1b..e358e26 100644 --- a/frontend/src/app/core/game.models.ts +++ b/frontend/src/app/core/game.models.ts @@ -52,7 +52,7 @@ export interface LivePlayer { export interface SocketMessage { coordinate?: Coordinate; - command: 'inform' | 'move' | 'remove' | 'state' | 'shutdown' | 'reset' | string; + command: 'inform' | 'move' | 'remove' | 'state' | 'shutdown' | string; data: string; } diff --git a/frontend/src/app/core/sockets/game-socket.service.ts b/frontend/src/app/core/sockets/game-socket.service.ts index 1bab16b..07dec17 100644 --- a/frontend/src/app/core/sockets/game-socket.service.ts +++ b/frontend/src/app/core/sockets/game-socket.service.ts @@ -240,14 +240,6 @@ export class GameSocketService extends WebSocketService { return; } - if (message.command === 'reset') { - // Game was reset — this player's ID is no longer valid. - this.statusMessage.set('The game was reset. Redirecting to registration…'); - this.sessionExpired.set(true); - this.onSessionExpired?.(); - return; - } - if (message.command === 'remove') { this.players.update((players) => { if (!(message.data in players)) { @@ -311,7 +303,6 @@ function isSocketMessage(value: unknown): value is SocketMessage { switch (value['command']) { case 'remove': case 'shutdown': - case 'reset': return true; case 'inform': case 'move': diff --git a/frontend/src/app/pages/register-page/register-page.component.ts b/frontend/src/app/pages/register-page/register-page.component.ts index 91a5764..685b53e 100644 --- a/frontend/src/app/pages/register-page/register-page.component.ts +++ b/frontend/src/app/pages/register-page/register-page.component.ts @@ -41,9 +41,9 @@ export class RegisterPageComponent { }); protected readonly status = signal( - isServerStoppedNavigation(this.location.getState()) - ? 'The server stopped. Register to join the next game.' - : '', + (this.location.getState() as { serverStopped?: boolean })?.serverStopped + ? 'The server stopped. Register to join the next game.' + : '', ); protected async submit(event: SubmitEvent): Promise { From f4b375bd87e67385bcf965219f15f1ecadc91dff Mon Sep 17 00:00:00 2001 From: Karen Yao Date: Tue, 25 Aug 2026 19:37:18 -0700 Subject: [PATCH 12/16] fix: clean up socket timer tests and trailing whitespace --- api/hub.go | 48 ++++++++-- api/socket.go | 4 +- api/socket_test.go | 90 +++++++++++++++++++ .../core/sockets/game-socket.service.spec.ts | 26 ++++-- .../sockets/leader-socket.service.spec.ts | 12 ++- .../core/sockets/websocket.service.spec.ts | 11 ++- 6 files changed, 167 insertions(+), 24 deletions(-) diff --git a/api/hub.go b/api/hub.go index ce5e181..e4560cd 100644 --- a/api/hub.go +++ b/api/hub.go @@ -26,9 +26,9 @@ type Hub struct { shutdown chan shutdownEvent } -type shutdownEvent struct { - command string - done chan struct{} +type shutdownEvent struct { + command string + done chan struct{} } func NewHub(players *Players, games ...*Game) *Hub { @@ -72,7 +72,7 @@ func (h *Hub) Run() { h.broadcastShutDown(event.command) close(event.done) } - } + } } func isPrivateMapRole(playerType PlayerType) bool { @@ -188,7 +188,7 @@ func (h *Hub) unregisterConnection(connection *Conn) { if hasCoordinate && isMapVisibleRole(player.Type) { message, ok := informMessage(player, coordinate) if ok { - h.broadcast(message, nil, onlyViewers) + h.broadcastControl(message, nil, onlyViewers) } } else { h.broadcastRemove(connection.playerID, onlyViewers, nil) @@ -303,6 +303,21 @@ func (h *Hub) broadcast(message []byte, origin *Conn, include connectionFilter) } } +func (h *Hub) broadcastControl(message []byte, origin *Conn, include connectionFilter) { + var failedConnections []*Conn + for connection := range h.connections { + if connection == origin || !include(connection) { + continue + } + if !h.enqueueControl(connection, message) { + failedConnections = append(failedConnections, connection) + } + } + for _, connection := range failedConnections { + h.unregisterConnection(connection) + } +} + func (h *Hub) enqueue(connection *Conn, message []byte) bool { select { case connection.send <- message: @@ -312,6 +327,19 @@ func (h *Hub) enqueue(connection *Conn, message []byte) bool { } } +func (h *Hub) enqueueControl(connection *Conn, message []byte) bool { + if h.enqueue(connection, message) { + return true + } + for { + select { + case <-connection.send: + default: + return h.enqueue(connection, message) + } + } +} + func informMessage(player PlayerResponse, coordinate Coordinate) ([]byte, bool) { playerJSON, err := json.Marshal(player) if err != nil { @@ -361,7 +389,7 @@ func (h *Hub) broadcastInform(playerID PlayerID, origin *Conn) { if h.connectionCanSee(connection, playerID, player.Type) { outgoing = message } - if !h.enqueue(connection, outgoing) { + if !h.enqueueControl(connection, outgoing) { slowConnections = append(slowConnections, connection) } } @@ -377,7 +405,7 @@ func (h *Hub) broadcastInform(playerID PlayerID, origin *Conn) { if retained && isMapVisibleRole(player.Type) { message, ok := informMessage(player, coordinate) if ok { - h.broadcast(message, nil, onlyViewers) + h.broadcastControl(message, nil, onlyViewers) } return } @@ -421,7 +449,7 @@ func removeMessage(playerID PlayerID) []byte { } func (h *Hub) broadcastRemove(playerID PlayerID, include connectionFilter, origin *Conn) { - h.broadcast(removeMessage(playerID), origin, include) + h.broadcastControl(removeMessage(playerID), origin, include) } func (h *Hub) clearOfflineLocations() { @@ -437,6 +465,8 @@ func (h *Hub) broadcastShutDown(command string) { return } for connection := range h.connections { - h.enqueue(connection, message) + if !h.enqueueControl(connection, message) { + h.unregisterConnection(connection) + } } } diff --git a/api/socket.go b/api/socket.go index 53fea32..4bfb30a 100644 --- a/api/socket.go +++ b/api/socket.go @@ -36,11 +36,11 @@ type Sockets struct { hub *Hub } -// BroadcastShutDown signals every client about server shutdown. +// BroadcastShutDown sends a shutdown command to all connected clients. func (s *Sockets) BroadcastShutDown(command string) { done := make(chan struct{}) s.hub.shutdown <- shutdownEvent{command: command, done: done} - <-done + <-done } func (s *Sockets) Init(players *Players, games ...*Game) { diff --git a/api/socket_test.go b/api/socket_test.go index 697deb9..0b80081 100644 --- a/api/socket_test.go +++ b/api/socket_test.go @@ -47,6 +47,96 @@ func TestBroadcastShutdownQueuesShutdownCommand(t *testing.T) { } } +func TestBroadcastShutdownEvictsQueuedMessages(t *testing.T) { + players := new(Players) + players.Init() + playerID := players.New(TypeGhost, "Player", StatusDisc) + hub := NewHub(players) + connection := &Conn{ + playerID: playerID, + role: playerConnection, + send: make(chan []byte, 2), + } + hub.connections[connection] = struct{}{} + connection.send <- []byte("stale move") + connection.send <- []byte("stale state") + + hub.broadcastShutDown(CMD_SHUTDOWN) + + if _, exists := hub.connections[connection]; !exists { + t.Fatal("connection was unregistered after successful prioritized delivery") + } + if len(connection.send) != 1 { + t.Fatalf("queued messages = %d, want 1", len(connection.send)) + } + if message := receiveTestMessage(t, connection); message.Command != CMD_SHUTDOWN { + t.Errorf("shutdown command = %q, want %q", message.Command, CMD_SHUTDOWN) + } +} + +func TestBroadcastShutdownUnregistersUndeliverableConnection(t *testing.T) { + players := new(Players) + players.Init() + playerID := players.New(TypeGhost, "Player", StatusDisc) + hub := NewHub(players) + connection := &Conn{ + playerID: playerID, + role: playerConnection, + send: make(chan []byte), + } + hub.connections[connection] = struct{}{} + + hub.broadcastShutDown(CMD_SHUTDOWN) + + if _, exists := hub.connections[connection]; exists { + t.Error("undeliverable connection remains registered") + } +} + +func TestControlBroadcastEvictsQueuedMessagesForResetUpdates(t *testing.T) { + players := new(Players) + players.Init() + activeID := players.New(TypePacman, "Active", StatusDisc) + offlineID := players.New(TypeGhost, "Offline", StatusDisc) + hub := NewHub(players) + active := &Conn{ + playerID: activeID, + role: playerConnection, + send: make(chan []byte, 1), + } + viewer := &Conn{ + role: viewerConnection, + send: make(chan []byte, 1), + } + hub.connections[active] = struct{}{} + hub.connections[viewer] = struct{}{} + hub.coordinates[activeID] = Coordinate{Latitude: 49.27, Longitude: -122.91} + hub.offlineCoordinates[offlineID] = Coordinate{Latitude: 49.28, Longitude: -122.90} + active.send <- []byte("stale state") + viewer.send <- []byte("stale move") + + hub.clearOfflineLocations() + if _, exists := hub.connections[viewer]; !exists { + t.Fatal("viewer was unregistered after prioritized marker removal") + } + removed := receiveTestMessage(t, viewer) + if removed.Command != CMD_REMOVE || removed.Data != string(offlineID) { + t.Errorf("offline marker removal = %#v", removed) + } + + if _, _, found := players.Update(activeID, TypeGhost); !found { + t.Fatal("reset active player update failed") + } + hub.broadcastInform(activeID, nil) + if _, exists := hub.connections[active]; !exists { + t.Fatal("active connection was unregistered after prioritized reset update") + } + updated := informPlayer(t, receiveTestMessage(t, active)) + if updated.ID != activeID || updated.Type != TypeGhost { + t.Errorf("reset player update = %#v", updated) + } +} + func TestGameStateSnapshotAndBroadcastDoNotChangePlayerConnectionCounts(t *testing.T) { players := new(Players) players.Init() diff --git a/frontend/src/app/core/sockets/game-socket.service.spec.ts b/frontend/src/app/core/sockets/game-socket.service.spec.ts index 2d0e036..1efa70e 100644 --- a/frontend/src/app/core/sockets/game-socket.service.spec.ts +++ b/frontend/src/app/core/sockets/game-socket.service.spec.ts @@ -340,7 +340,7 @@ describe('GameSocketService', () => { expect(service.status()).toContain('Register as admin again in this browser'); }); - it('reconnects after two failures and expires the session after the third', () => { + it('expires the session after three consecutive failed player connections', () => { vi.useFakeTimers(); vi.spyOn(console, 'log').mockImplementation(() => undefined); vi.spyOn(console, 'warn').mockImplementation(() => undefined); @@ -351,12 +351,6 @@ describe('GameSocketService', () => { advanceToNextReconnect(); MockGameWebSocket.instances[1].serverClose(false); advanceToNextReconnect(); - - expect(service.sessionExpired()).toBe(false); - expect(service.state()).toBe('connecting'); - expect(MockGameWebSocket.instances).toHaveLength(3); - expect(onSessionExpired).not.toHaveBeenCalled(); - MockGameWebSocket.instances[2].serverClose(false); expect(service.sessionExpired()).toBe(true); @@ -366,6 +360,24 @@ describe('GameSocketService', () => { expect(onSessionExpired).toHaveBeenCalledOnce(); }); + it('reconnects without expiring the session after fewer than three failures', () => { + vi.useFakeTimers(); + vi.spyOn(console, 'log').mockImplementation(() => undefined); + vi.spyOn(console, 'warn').mockImplementation(() => undefined); + const onSessionExpired = vi.fn(); + service.start('ABCD', () => undefined, onSessionExpired); + + MockGameWebSocket.instances[0].serverClose(false); + advanceToNextReconnect(); + MockGameWebSocket.instances[1].serverClose(false); + advanceToNextReconnect(); + + expect(service.sessionExpired()).toBe(false); + expect(onSessionExpired).not.toHaveBeenCalled(); + expect(service.state()).toBe('connecting'); + expect(MockGameWebSocket.instances).toHaveLength(3); + }); + it('resets the failure counter when a player connection succeeds', () => { vi.useFakeTimers(); vi.spyOn(console, 'log').mockImplementation(() => undefined); diff --git a/frontend/src/app/core/sockets/leader-socket.service.spec.ts b/frontend/src/app/core/sockets/leader-socket.service.spec.ts index c5d1954..5d399bb 100644 --- a/frontend/src/app/core/sockets/leader-socket.service.spec.ts +++ b/frontend/src/app/core/sockets/leader-socket.service.spec.ts @@ -3,6 +3,7 @@ import { TestBed } from '@angular/core/testing'; import { PAC_WINDOW } from '../browser-window.token'; import { LeaderState, PlayerStatus, PlayerType } from '../game.models'; import { LeaderSocketService } from './leader-socket.service'; +import { WebSocketService } from './websocket.service'; class MockLeaderWebSocket { static readonly CONNECTING = 0; @@ -67,6 +68,11 @@ const player = { status: PlayerStatus.Connected, }; +function reconnectDelay(attempt: number): number { + const delays = WebSocketService.RECONNECT_DELAYS; + return delays[Math.min(attempt - 1, delays.length - 1)]; +} + describe('LeaderSocketService', () => { let service: LeaderSocketService; let originalWebSocket: typeof WebSocket; @@ -204,7 +210,7 @@ describe('LeaderSocketService', () => { first.serverClose(wasClean); expect(service.status()).toContain('Leader feed lost'); - vi.advanceTimersByTime(1000); + vi.advanceTimersByTime(reconnectDelay(1)); expect(MockLeaderWebSocket.instances).toHaveLength(2); MockLeaderWebSocket.instances[1].open(); @@ -257,7 +263,7 @@ describe('LeaderSocketService', () => { expect(service.players()).toEqual([]); expect(service.isFlagFound()).toBe(false); expect(service.status()).toContain('Waiting for the role to be restored'); - vi.advanceTimersByTime(1000); + vi.advanceTimersByTime(reconnectDelay(1)); const restoredSocket = MockLeaderWebSocket.instances[1]; restoredSocket.open(); @@ -286,7 +292,7 @@ describe('LeaderSocketService', () => { socket.serverClose(true, 1008, 'Leader authentication required'); expect(service.state()).toBe('revoked'); expect(service.status()).toContain('Leader access was revoked'); - vi.advanceTimersByTime(1000); + vi.advanceTimersByTime(reconnectDelay(1)); expect(MockLeaderWebSocket.instances).toHaveLength(2); MockLeaderWebSocket.instances[1].open(); diff --git a/frontend/src/app/core/sockets/websocket.service.spec.ts b/frontend/src/app/core/sockets/websocket.service.spec.ts index 37cc803..54fbeb8 100644 --- a/frontend/src/app/core/sockets/websocket.service.spec.ts +++ b/frontend/src/app/core/sockets/websocket.service.spec.ts @@ -99,6 +99,11 @@ class MockRxWebSocket { } } +function reconnectDelay(attempt: number): number { + const delays = WebSocketService.RECONNECT_DELAYS; + return delays[Math.min(attempt - 1, delays.length - 1)]; +} + describe('WebSocketService', () => { let service: TestWebSocketService; let originalWebSocket: typeof WebSocket; @@ -200,7 +205,7 @@ describe('WebSocketService', () => { first.serverClose(wasClean); expect(service.transportState()).toBe('connecting'); - vi.advanceTimersByTime(999); + vi.advanceTimersByTime(reconnectDelay(1) - 1); expect(MockRxWebSocket.instances).toHaveLength(1); vi.advanceTimersByTime(1); @@ -225,7 +230,7 @@ describe('WebSocketService', () => { Object.defineProperty(window.navigator, 'onLine', { configurable: true, value: true }); window.dispatchEvent(new Event('online')); - vi.advanceTimersByTime(999); + vi.advanceTimersByTime(reconnectDelay(1) - 1); expect(MockRxWebSocket.instances).toHaveLength(1); vi.advanceTimersByTime(1); expect(MockRxWebSocket.instances).toHaveLength(2); @@ -257,7 +262,7 @@ describe('WebSocketService', () => { expect(service.requestReconnect('revoked')).toBe(true); expect(service.transportState()).toBe('revoked'); - vi.advanceTimersByTime(999); + vi.advanceTimersByTime(reconnectDelay(1) - 1); expect(MockRxWebSocket.instances).toHaveLength(1); vi.advanceTimersByTime(1); From 6c0f83cab032208fbcf8df65a491688546e9e0b5 Mon Sep 17 00:00:00 2001 From: Karen Yao Date: Wed, 26 Aug 2026 01:13:25 -0700 Subject: [PATCH 13/16] fix: remove dead code --- api/hub.go | 3 +++ .../app/pages/register-page/register-page.component.ts | 9 --------- 2 files changed, 3 insertions(+), 9 deletions(-) diff --git a/api/hub.go b/api/hub.go index e4560cd..9933d2c 100644 --- a/api/hub.go +++ b/api/hub.go @@ -323,6 +323,7 @@ func (h *Hub) enqueue(connection *Conn, message []byte) bool { case connection.send <- message: return true default: + // Channel buffer is full. Client cannot keep up with real-time updates. return false } } @@ -334,7 +335,9 @@ func (h *Hub) enqueueControl(connection *Conn, message []byte) bool { for { select { case <-connection.send: + // Drain older queued messages until the buffer is empty to prioritize this critical control message. default: + // Buffer is now completely drained. Enqueue the critical control message. return h.enqueue(connection, message) } } diff --git a/frontend/src/app/pages/register-page/register-page.component.ts b/frontend/src/app/pages/register-page/register-page.component.ts index 685b53e..180b61f 100644 --- a/frontend/src/app/pages/register-page/register-page.component.ts +++ b/frontend/src/app/pages/register-page/register-page.component.ts @@ -75,12 +75,3 @@ export class RegisterPageComponent { } } } - -function isServerStoppedNavigation(state: unknown): boolean { - return ( - typeof state === 'object' && - state !== null && - 'serverStopped' in state && - state.serverStopped === true - ); -} From 8fc083cb94845445521b0008a821ccff3c75cc45 Mon Sep 17 00:00:00 2001 From: Karen Yao Date: Wed, 9 Sep 2026 15:57:20 -0700 Subject: [PATCH 14/16] fix: prevent control broadcasts from dropping queued updates --- api/hub.go | 35 +++++++------------ api/socket_test.go | 86 ++++++++++++++++++++++++++++++++-------------- 2 files changed, 72 insertions(+), 49 deletions(-) diff --git a/api/hub.go b/api/hub.go index 9933d2c..e2c5e9a 100644 --- a/api/hub.go +++ b/api/hub.go @@ -188,7 +188,7 @@ func (h *Hub) unregisterConnection(connection *Conn) { if hasCoordinate && isMapVisibleRole(player.Type) { message, ok := informMessage(player, coordinate) if ok { - h.broadcastControl(message, nil, onlyViewers) + h.broadcast(message, nil, onlyViewers) } } else { h.broadcastRemove(connection.playerID, onlyViewers, nil) @@ -303,21 +303,6 @@ func (h *Hub) broadcast(message []byte, origin *Conn, include connectionFilter) } } -func (h *Hub) broadcastControl(message []byte, origin *Conn, include connectionFilter) { - var failedConnections []*Conn - for connection := range h.connections { - if connection == origin || !include(connection) { - continue - } - if !h.enqueueControl(connection, message) { - failedConnections = append(failedConnections, connection) - } - } - for _, connection := range failedConnections { - h.unregisterConnection(connection) - } -} - func (h *Hub) enqueue(connection *Conn, message []byte) bool { select { case connection.send <- message: @@ -328,16 +313,20 @@ func (h *Hub) enqueue(connection *Conn, message []byte) bool { } } -func (h *Hub) enqueueControl(connection *Conn, message []byte) bool { +// enqueueShutdown discards queued gameplay updates only when necessary to +// deliver the terminal shutdown command. Ordinary messages must use enqueue so +// a slow client reconnects and receives a fresh snapshot instead of continuing +// with a silently truncated event stream. +func (h *Hub) enqueueShutdown(connection *Conn, message []byte) bool { if h.enqueue(connection, message) { return true } for { select { case <-connection.send: - // Drain older queued messages until the buffer is empty to prioritize this critical control message. + // Gameplay state is no longer relevant once this session is ending. default: - // Buffer is now completely drained. Enqueue the critical control message. + // The buffer is empty, so the shutdown command can be prioritized. return h.enqueue(connection, message) } } @@ -392,7 +381,7 @@ func (h *Hub) broadcastInform(playerID PlayerID, origin *Conn) { if h.connectionCanSee(connection, playerID, player.Type) { outgoing = message } - if !h.enqueueControl(connection, outgoing) { + if !h.enqueue(connection, outgoing) { slowConnections = append(slowConnections, connection) } } @@ -408,7 +397,7 @@ func (h *Hub) broadcastInform(playerID PlayerID, origin *Conn) { if retained && isMapVisibleRole(player.Type) { message, ok := informMessage(player, coordinate) if ok { - h.broadcastControl(message, nil, onlyViewers) + h.broadcast(message, nil, onlyViewers) } return } @@ -452,7 +441,7 @@ func removeMessage(playerID PlayerID) []byte { } func (h *Hub) broadcastRemove(playerID PlayerID, include connectionFilter, origin *Conn) { - h.broadcastControl(removeMessage(playerID), origin, include) + h.broadcast(removeMessage(playerID), origin, include) } func (h *Hub) clearOfflineLocations() { @@ -468,7 +457,7 @@ func (h *Hub) broadcastShutDown(command string) { return } for connection := range h.connections { - if !h.enqueueControl(connection, message) { + if !h.enqueueShutdown(connection, message) { h.unregisterConnection(connection) } } diff --git a/api/socket_test.go b/api/socket_test.go index 0b80081..6c97e43 100644 --- a/api/socket_test.go +++ b/api/socket_test.go @@ -93,47 +93,81 @@ func TestBroadcastShutdownUnregistersUndeliverableConnection(t *testing.T) { } } -func TestControlBroadcastEvictsQueuedMessagesForResetUpdates(t *testing.T) { +func TestRemoveBroadcastPreservesQueuedMessages(t *testing.T) { players := new(Players) players.Init() - activeID := players.New(TypePacman, "Active", StatusDisc) - offlineID := players.New(TypeGhost, "Offline", StatusDisc) hub := NewHub(players) - active := &Conn{ - playerID: activeID, - role: playerConnection, - send: make(chan []byte, 1), - } viewer := &Conn{ role: viewerConnection, - send: make(chan []byte, 1), + send: make(chan []byte, 2), } - hub.connections[active] = struct{}{} hub.connections[viewer] = struct{}{} - hub.coordinates[activeID] = Coordinate{Latitude: 49.27, Longitude: -122.91} - hub.offlineCoordinates[offlineID] = Coordinate{Latitude: 49.28, Longitude: -122.90} - active.send <- []byte("stale state") - viewer.send <- []byte("stale move") + queued := []byte("queued state") + viewer.send <- queued - hub.clearOfflineLocations() - if _, exists := hub.connections[viewer]; !exists { - t.Fatal("viewer was unregistered after prioritized marker removal") + playerID := PlayerID("offline") + hub.broadcastRemove(playerID, onlyViewers, nil) + + if got := receiveTestData(t, viewer); string(got) != string(queued) { + t.Fatalf("first queued message = %q, want %q", got, queued) } removed := receiveTestMessage(t, viewer) - if removed.Command != CMD_REMOVE || removed.Data != string(offlineID) { + if removed.Command != CMD_REMOVE || removed.Data != string(playerID) { t.Errorf("offline marker removal = %#v", removed) } +} + +func TestRemoveBroadcastDoesNotEvictFullQueue(t *testing.T) { + players := new(Players) + players.Init() + hub := NewHub(players) + viewer := &Conn{ + role: viewerConnection, + send: make(chan []byte, 1), + } + hub.connections[viewer] = struct{}{} + queued := []byte("queued state") + viewer.send <- queued + + hub.broadcastRemove(PlayerID("offline"), onlyViewers, nil) + + if _, exists := hub.connections[viewer]; exists { + t.Fatal("slow viewer remains registered") + } + if len(viewer.send) != 1 { + t.Fatalf("queued messages = %d, want 1", len(viewer.send)) + } + if got := receiveTestData(t, viewer); string(got) != string(queued) { + t.Fatalf("queued message = %q, want %q", got, queued) + } +} - if _, _, found := players.Update(activeID, TypeGhost); !found { - t.Fatal("reset active player update failed") +func TestInformBroadcastDoesNotEvictFullQueue(t *testing.T) { + players := new(Players) + players.Init() + playerID := players.New(TypePacman, "Active", StatusConn) + hub := NewHub(players) + owner := newTestConnection(playerID) + viewer := &Conn{ + role: viewerConnection, + send: make(chan []byte, 1), + } + hub.connections[owner] = struct{}{} + hub.connections[viewer] = struct{}{} + hub.coordinates[playerID] = Coordinate{Latitude: 49.27, Longitude: -122.91} + queued := []byte("queued move") + viewer.send <- queued + + hub.broadcastInform(playerID, owner) + + if _, exists := hub.connections[viewer]; exists { + t.Fatal("slow viewer remains registered") } - hub.broadcastInform(activeID, nil) - if _, exists := hub.connections[active]; !exists { - t.Fatal("active connection was unregistered after prioritized reset update") + if len(viewer.send) != 1 { + t.Fatalf("queued messages = %d, want 1", len(viewer.send)) } - updated := informPlayer(t, receiveTestMessage(t, active)) - if updated.ID != activeID || updated.Type != TypeGhost { - t.Errorf("reset player update = %#v", updated) + if got := receiveTestData(t, viewer); string(got) != string(queued) { + t.Fatalf("queued message = %q, want %q", got, queued) } } From f93c4dbedae012f2b36daddab17d2edcecb4692f Mon Sep 17 00:00:00 2001 From: Karen Yao Date: Wed, 9 Sep 2026 20:37:34 -0700 Subject: [PATCH 15/16] fix: handle server shutdown without automatic reconnection --- .../core/sockets/admin-socket.service.spec.ts | 15 ++++++++ .../core/sockets/game-socket.service.spec.ts | 36 +++++++++++++++++++ .../app/core/sockets/game-socket.service.ts | 30 +++++++++++++--- .../sockets/leader-socket.service.spec.ts | 17 +++++++++ .../app/core/sockets/leader-socket.service.ts | 4 +++ .../core/sockets/websocket.service.spec.ts | 32 +++++++++++++++++ .../src/app/core/sockets/websocket.service.ts | 24 +++++++++++++ 7 files changed, 153 insertions(+), 5 deletions(-) diff --git a/frontend/src/app/core/sockets/admin-socket.service.spec.ts b/frontend/src/app/core/sockets/admin-socket.service.spec.ts index 748ed12..eb9becd 100644 --- a/frontend/src/app/core/sockets/admin-socket.service.spec.ts +++ b/frontend/src/app/core/sockets/admin-socket.service.spec.ts @@ -118,6 +118,21 @@ describe('sockets/AdminSocketService', () => { expect(MockAdminWebSocket.instances).toHaveLength(1); }); + it('enters shutdown without reconnecting on a 1001 close', () => { + vi.useFakeTimers(); + vi.spyOn(console, 'log').mockImplementation(() => undefined); + socket.message({ event: 'snapshot', isFlagFound: false, players: [] }); + expect(service.isReady()).toBe(true); + + socket.serverClose(true, 1001, 'Server shutting down'); + vi.runAllTimers(); + + expect(service.state()).toBe('shutdown'); + expect(service.isReady()).toBe(false); + expect(service.status()).toBe('The server has stopped this connection.'); + expect(MockAdminWebSocket.instances).toHaveLength(1); + }); + it('applies and sorts a complete snapshot including flag state', () => { socket.message({ event: 'snapshot', diff --git a/frontend/src/app/core/sockets/game-socket.service.spec.ts b/frontend/src/app/core/sockets/game-socket.service.spec.ts index 1efa70e..7ff9692 100644 --- a/frontend/src/app/core/sockets/game-socket.service.spec.ts +++ b/frontend/src/app/core/sockets/game-socket.service.spec.ts @@ -245,6 +245,42 @@ describe('GameSocketService', () => { expect(MockGameWebSocket.instances).toHaveLength(1); }); + it('ends a player session on a 1001 close without counting a failure', () => { + vi.useFakeTimers(); + vi.spyOn(console, 'log').mockImplementation(() => undefined); + const onSessionExpired = vi.fn(); + const onServerShutdown = vi.fn(); + service.start('ABCD', () => undefined, onSessionExpired, onServerShutdown); + const first = MockGameWebSocket.instances[0]; + first.open(); + + first.serverClose(true, 1001, 'Server shutting down'); + vi.runAllTimers(); + + expect(service.state()).toBe('shutdown'); + expect(service.status()).toBe('The server stopped. Register to join the next game.'); + expect(MockGameWebSocket.instances).toHaveLength(1); + expect(onServerShutdown).toHaveBeenCalledOnce(); + expect(onSessionExpired).not.toHaveBeenCalled(); + expect(service.sessionExpired()).toBe(false); + expect(service.sendCoordinate({ latitude: 49.2, longitude: -123 })).toBe(false); + }); + + it('ends a viewer session on a 1001 close', () => { + vi.useFakeTimers(); + vi.spyOn(console, 'log').mockImplementation(() => undefined); + service.startViewer(); + const first = MockGameWebSocket.instances[0]; + first.open(); + + first.serverClose(true, 1001, 'Server shutting down'); + vi.runAllTimers(); + + expect(service.state()).toBe('shutdown'); + expect(service.status()).toBe('The server stopped the admin map connection.'); + expect(MockGameWebSocket.instances).toHaveLength(1); + }); + it.each([ ['clean', true], ['abnormal', false], diff --git a/frontend/src/app/core/sockets/game-socket.service.ts b/frontend/src/app/core/sockets/game-socket.service.ts index 07dec17..212e430 100644 --- a/frontend/src/app/core/sockets/game-socket.service.ts +++ b/frontend/src/app/core/sockets/game-socket.service.ts @@ -100,6 +100,27 @@ export class GameSocketService extends WebSocketService { } private endForServerShutdown(): void { + const { mode, onServerShutdown } = this.clearShutdownSession(); + + // Incrementing the connection identity before unsubscribing makes any + // queued retry or late close event belong to an obsolete connection. + this.disconnect(); + this.state.set('shutdown'); + this.statusMessage.set( + mode === 'player' + ? 'The server stopped. Register to join the next game.' + : 'The server stopped the admin map connection.', + ); + + if (mode === 'player') { + onServerShutdown?.(); + } + } + + private clearShutdownSession(): { + mode: SocketMode | null; + onServerShutdown: (() => void) | null; + } { const mode = this.mode; const onServerShutdown = this.onServerShutdown; @@ -111,17 +132,16 @@ export class GameSocketService extends WebSocketService { this.reconnecting = false; this.consecutiveFailures = 0; this.sessionExpired.set(false); + return { mode, onServerShutdown }; + } - // Incrementing the connection identity before unsubscribing makes any - // queued retry or late close event belong to an obsolete connection. - this.disconnect(); - this.state.set('shutdown'); + protected override onShutdown(): void { + const { mode, onServerShutdown } = this.clearShutdownSession(); this.statusMessage.set( mode === 'player' ? 'The server stopped. Register to join the next game.' : 'The server stopped the admin map connection.', ); - if (mode === 'player') { onServerShutdown?.(); } diff --git a/frontend/src/app/core/sockets/leader-socket.service.spec.ts b/frontend/src/app/core/sockets/leader-socket.service.spec.ts index 5d399bb..6566ea3 100644 --- a/frontend/src/app/core/sockets/leader-socket.service.spec.ts +++ b/frontend/src/app/core/sockets/leader-socket.service.spec.ts @@ -298,4 +298,21 @@ describe('LeaderSocketService', () => { MockLeaderWebSocket.instances[1].open(); expect(service.state()).toBe('revoked'); }); + + it('enters shutdown without reconnecting on a 1001 close', () => { + vi.useFakeTimers(); + vi.spyOn(console, 'log').mockImplementation(() => undefined); + vi.spyOn(console, 'warn').mockImplementation(() => undefined); + service.start(); + const socket = MockLeaderWebSocket.instances[0]; + socket.open(); + socket.message({ event: 'snapshot', leader, players: [player], isFlagFound: false }); + + socket.serverClose(true, 1001, 'Server shutting down'); + vi.runAllTimers(); + + expect(service.state()).toBe('shutdown'); + expect(service.status()).toBe('The server has stopped this connection.'); + expect(MockLeaderWebSocket.instances).toHaveLength(1); + }); }); diff --git a/frontend/src/app/core/sockets/leader-socket.service.ts b/frontend/src/app/core/sockets/leader-socket.service.ts index f139395..08069cc 100644 --- a/frontend/src/app/core/sockets/leader-socket.service.ts +++ b/frontend/src/app/core/sockets/leader-socket.service.ts @@ -84,6 +84,10 @@ export class LeaderSocketService extends WebSocketService { } } + protected override onShutdown(): void { + this.reconnecting = false; + } + protected override onSocketError(): void { this.statusMessage.set( 'Could not authenticate the leader feed. Open the game with a current Leader identity, then retry.', diff --git a/frontend/src/app/core/sockets/websocket.service.spec.ts b/frontend/src/app/core/sockets/websocket.service.spec.ts index 54fbeb8..64190b6 100644 --- a/frontend/src/app/core/sockets/websocket.service.spec.ts +++ b/frontend/src/app/core/sockets/websocket.service.spec.ts @@ -301,6 +301,38 @@ describe('WebSocketService', () => { expect(service.transportState()).toBe('idle'); }); + it('enters shutdown without reconnecting on a 1001 close', () => { + vi.useFakeTimers(); + vi.spyOn(console, 'log').mockImplementation(() => undefined); + vi.spyOn(console, 'warn').mockImplementation(() => undefined); + service.connect(); + const socket = MockRxWebSocket.instances[0]; + socket.open(); + + socket.serverClose(true, WebSocketService.SHUTDOWN_CODE, 'Server shutting down'); + expect(service.transportState()).toBe('shutdown'); + expect(service.status()).toBe('The server has stopped this connection.'); + vi.runAllTimers(); + + expect(MockRxWebSocket.instances).toHaveLength(1); + }); + + it('ignores a late close after a 1001 shutdown', () => { + vi.spyOn(console, 'log').mockImplementation(() => undefined); + MockRxWebSocket.closeSynchronously = false; + service.connect(); + const socket = MockRxWebSocket.instances[0]; + socket.open(); + const lateClose = socket.onclose; + + socket.serverClose(true, WebSocketService.SHUTDOWN_CODE, 'Server shutting down'); + expect(service.transportState()).toBe('shutdown'); + lateClose?.(new CloseEvent('close', { code: 1006, wasClean: false })); + + expect(service.transportState()).toBe('shutdown'); + expect(MockRxWebSocket.instances).toHaveLength(1); + }); + it('closes the active socket when its injection context is destroyed', () => { service.connect(); const socket = MockRxWebSocket.instances[0]; diff --git a/frontend/src/app/core/sockets/websocket.service.ts b/frontend/src/app/core/sockets/websocket.service.ts index 94062da..5ebf3a6 100644 --- a/frontend/src/app/core/sockets/websocket.service.ts +++ b/frontend/src/app/core/sockets/websocket.service.ts @@ -20,6 +20,7 @@ export abstract class WebSocketService { static readonly WEBSOCKET_OPEN = 1; static readonly WEBSOCKET_CLOSING = 2; static readonly POLICY_VIOLATION_CODE = 1008; + static readonly SHUTDOWN_CODE = 1001; readonly status = computed(() => this.getStatus(this.state())); @@ -48,6 +49,8 @@ export abstract class WebSocketService { protected onSocketClose(_closeEvent: CloseEvent): void {} + protected onShutdown(): void {} + protected onSocketError(_error: unknown): void {} protected onInvalidMessage(_message: unknown, _error: unknown): void {} @@ -138,6 +141,10 @@ export abstract class WebSocketService { if (!this.isCurrentConnection(connectionId, socketSubject$)) { return; } + if (closeEvent.code === WebSocketService.SHUTDOWN_CODE) { + this.handleServerShutdown(connectionId, socketSubject$, closeEvent); + return; + } this.socketOpen = false; console.log('WebSocket closed: ', closeEvent); this.onSocketClose(closeEvent); @@ -347,6 +354,23 @@ export abstract class WebSocketService { return this.requestedReconnectState ?? this.getReconnectState(); } + private handleServerShutdown( + connectionId: number, + socketSubject$: WebSocketSubject, + closeEvent: CloseEvent, + ): void { + if (!this.isCurrentConnection(connectionId, socketSubject$)) { + return; + } + this.socketOpen = false; + console.log('WebSocket closed: ', closeEvent); + this.onSocketClose(closeEvent); + this.reconnectAllowed = false; + this.requestedReconnectState = undefined; + this.onShutdown(); + this.state.set('shutdown'); + } + private isCurrentConnection( connectionId: number, socketSubject$: WebSocketSubject, From c655a121c07f15a5db5b625b8fe5f3acbac27aed Mon Sep 17 00:00:00 2001 From: Karen Yao Date: Thu, 10 Sep 2026 08:09:36 -0700 Subject: [PATCH 16/16] fix: coordinate HTTP and WebSocket shutdown --- api/admin.go | 4 +- api/admin_socket.go | 21 +++++ api/admin_test.go | 20 ++--- api/etc.go | 3 + api/hub.go | 41 +-------- api/leader.go | 17 +++- api/leader_test.go | 4 +- api/lifecycle.go | 149 +++++++++++++++++++++++++++++++ api/lifecycle_test.go | 203 ++++++++++++++++++++++++++++++++++++++++++ api/shutdown_test.go | 129 +++++++++++++++++++++++++++ api/socket.go | 57 +++++++++--- api/socket_test.go | 62 ------------- main.go | 87 +++++++++++++----- 13 files changed, 643 insertions(+), 154 deletions(-) create mode 100644 api/lifecycle.go create mode 100644 api/lifecycle_test.go create mode 100644 api/shutdown_test.go diff --git a/api/admin.go b/api/admin.go index 1834bf4..596860e 100644 --- a/api/admin.go +++ b/api/admin.go @@ -30,6 +30,7 @@ type Admin struct { players *Players sockets *Sockets game *Game + lifecycle *Lifecycle connections map[adminSocketConnection]struct{} password string @@ -39,9 +40,10 @@ type Admin struct { socketMutex sync.Mutex } -func (a *Admin) Init(players *Players, sockets *Sockets, password string, games ...*Game) { +func (a *Admin) Init(players *Players, sockets *Sockets, password string, lifecycle *Lifecycle, games ...*Game) { a.players = players a.sockets = sockets + a.lifecycle = lifecycle a.password = password a.cookieValue = base64.RawURLEncoding.EncodeToString([]byte(password)) a.registered = false diff --git a/api/admin_socket.go b/api/admin_socket.go index b083ad3..4edcb90 100644 --- a/api/admin_socket.go +++ b/api/admin_socket.go @@ -3,6 +3,7 @@ package api import ( "encoding/json" "net/http" + "time" ws "github.com/gorilla/websocket" ) @@ -44,6 +45,13 @@ func (a *Admin) ServeSocket(w http.ResponseWriter, r *http.Request) { if err != nil { return } + if a.lifecycle != nil { + if !a.lifecycle.Track(connection) { + _ = connection.Close() + return + } + defer a.lifecycle.Untrack(connection) + } if !a.addConnection(connection) { return } @@ -127,6 +135,7 @@ func (a *Admin) broadcastSocketMessage(message AdminSocketMessage) { a.socketMutex.Lock() defer a.socketMutex.Unlock() for connection := range a.connections { + setAdminWriteDeadline(connection) if err := connection.WriteMessage(ws.TextMessage, JSON); err != nil { delete(a.connections, connection) _ = connection.Close() @@ -139,5 +148,17 @@ func writeAdminSocketMessage(connection adminSocketConnection, message AdminSock if err != nil { return false } + setAdminWriteDeadline(connection) return connection.WriteMessage(ws.TextMessage, JSON) == nil } + +// setAdminWriteDeadline applies the normal write timeout without holding the +// registry mutex during I/O. Connections without a deadline API keep the +// previous behavior for test doubles. +func setAdminWriteDeadline(connection adminSocketConnection) { + if deadlineWriter, ok := connection.(interface { + SetWriteDeadline(time.Time) error + }); ok { + _ = deadlineWriter.SetWriteDeadline(time.Now().Add(socketWriteTimeout)) + } +} diff --git a/api/admin_test.go b/api/admin_test.go index 52fea12..8a6fd50 100644 --- a/api/admin_test.go +++ b/api/admin_test.go @@ -28,8 +28,8 @@ func newAdminTestState(t *testing.T, password string) (*Players, *Admin) { sockets := new(Sockets) admin := new(Admin) players.Init() - sockets.Init(players) - admin.Init(players, sockets, password) + sockets.Init(players, nil) + admin.Init(players, sockets, password, nil) return players, admin } @@ -145,9 +145,9 @@ func TestAdminResetPreservesLeadersAndClearsFlag(t *testing.T) { players.Init() game := new(Game) sockets := new(Sockets) - sockets.Init(players, game) + sockets.Init(players, nil, game) admin := new(Admin) - admin.Init(players, sockets, "top-secret", game) + admin.Init(players, sockets, "top-secret", nil, game) cookie := registerTestAdmin(t, admin, "top-secret") for _, playerType := range []PlayerType{TypeLeader, TypeAntiPacLeader, TypeFlagLeader} { players.New(playerType, TypeString(playerType), StatusDisc) @@ -179,9 +179,9 @@ func TestAdminResetPreservesConnectedSessionsAndLeaderAuthorization(t *testing.T players.Init() game := new(Game) sockets := new(Sockets) - sockets.Init(players, game) + sockets.Init(players, nil, game) admin := new(Admin) - admin.Init(players, sockets, "top-secret", game) + admin.Init(players, sockets, "top-secret", nil, game) cookie := registerTestAdmin(t, admin, "top-secret") leaderID := players.New(TypeAntiPacLeader, "Leader", StatusDisc) @@ -235,9 +235,9 @@ func TestAdminFlagUpdatesSharedStateAndSocketClients(t *testing.T) { players.Init() game := new(Game) sockets := new(Sockets) - sockets.Init(players, game) + sockets.Init(players, nil, game) admin := new(Admin) - admin.Init(players, sockets, "top-secret", game) + admin.Init(players, sockets, "top-secret", nil, game) cookie := registerTestAdmin(t, admin, "top-secret") connection := new(recordingAdminConnection) if !admin.addConnection(connection) { @@ -298,9 +298,9 @@ func TestAdminResetClearsOfflineLocationsButPreservesActiveCoordinates(t *testin players.Init() game := new(Game) sockets := new(Sockets) - sockets.Init(players, game) + sockets.Init(players, nil, game) admin := new(Admin) - admin.Init(players, sockets, "top-secret", game) + admin.Init(players, sockets, "top-secret", nil, game) cookie := registerTestAdmin(t, admin, "top-secret") activeID := players.New(TypeLeader, "Active", StatusDisc) diff --git a/api/etc.go b/api/etc.go index bf9f6dc..eb560b6 100644 --- a/api/etc.go +++ b/api/etc.go @@ -21,6 +21,9 @@ const ( CMD_INFORM = "inform" // inform another player change/connection CMD_REMOVE = "remove" // remove a player marker without disclosing a location CMD_STATE = "state" // inform clients of shared game state + // CMD_SHUTDOWN is the legacy JSON shutdown command. New servers notify + // shutdown with a 1001 Going Away close frame; clients still accept this + // command for compatibility. CMD_SHUTDOWN = "shutdown" // inform clients of server shutdown // player type diff --git a/api/hub.go b/api/hub.go index e2c5e9a..85bbe18 100644 --- a/api/hub.go +++ b/api/hub.go @@ -23,12 +23,6 @@ type Hub struct { inform chan PlayerID state chan GameState clearOffline chan chan struct{} - shutdown chan shutdownEvent -} - -type shutdownEvent struct { - command string - done chan struct{} } func NewHub(players *Players, games ...*Game) *Hub { @@ -44,7 +38,6 @@ func NewHub(players *Players, games ...*Game) *Hub { inform: make(chan PlayerID), state: make(chan GameState), clearOffline: make(chan chan struct{}), - shutdown: make(chan shutdownEvent), } if len(games) > 0 { hub.game = games[0] @@ -68,9 +61,6 @@ func (h *Hub) Run() { case done := <-h.clearOffline: h.clearOfflineLocations() close(done) - case event := <-h.shutdown: - h.broadcastShutDown(event.command) - close(event.done) } } } @@ -313,25 +303,6 @@ func (h *Hub) enqueue(connection *Conn, message []byte) bool { } } -// enqueueShutdown discards queued gameplay updates only when necessary to -// deliver the terminal shutdown command. Ordinary messages must use enqueue so -// a slow client reconnects and receives a fresh snapshot instead of continuing -// with a silently truncated event stream. -func (h *Hub) enqueueShutdown(connection *Conn, message []byte) bool { - if h.enqueue(connection, message) { - return true - } - for { - select { - case <-connection.send: - // Gameplay state is no longer relevant once this session is ending. - default: - // The buffer is empty, so the shutdown command can be prioritized. - return h.enqueue(connection, message) - } - } -} - func informMessage(player PlayerResponse, coordinate Coordinate) ([]byte, bool) { playerJSON, err := json.Marshal(player) if err != nil { @@ -451,14 +422,4 @@ func (h *Hub) clearOfflineLocations() { } } -func (h *Hub) broadcastShutDown(command string) { - message, err := json.Marshal(Message{Command: command}) - if err != nil { - return - } - for connection := range h.connections { - if !h.enqueueShutdown(connection, message) { - h.unregisterConnection(connection) - } - } -} + diff --git a/api/leader.go b/api/leader.go index 0800505..8fd73a1 100644 --- a/api/leader.go +++ b/api/leader.go @@ -51,18 +51,20 @@ type leaderSocketConnection interface { } type Leader struct { - players *Players - game *Game - sockets *Sockets + players *Players + game *Game + sockets *Sockets + lifecycle *Lifecycle connections map[leaderSocketConnection]PlayerID socketMutex sync.Mutex } -func (l *Leader) Init(players *Players, game *Game, sockets *Sockets) { +func (l *Leader) Init(players *Players, game *Game, sockets *Sockets, lifecycle *Lifecycle) { l.players = players l.game = game l.sockets = sockets + l.lifecycle = lifecycle l.connections = make(map[leaderSocketConnection]PlayerID) players.AddObserver(l.BroadcastPlayer) players.AddRemovalObserver(l.BroadcastRemoval) @@ -230,6 +232,13 @@ func (l *Leader) ServeSocket(w http.ResponseWriter, r *http.Request) { if err != nil { return } + if l.lifecycle != nil { + if !l.lifecycle.Track(connection) { + _ = connection.Close() + return + } + defer l.lifecycle.Untrack(connection) + } if !l.addConnection(connection, state.Leader.ID) { return } diff --git a/api/leader_test.go b/api/leader_test.go index bf87d39..bd2ad6b 100644 --- a/api/leader_test.go +++ b/api/leader_test.go @@ -46,9 +46,9 @@ func newLeaderTestState() (*Players, *Game, *Sockets, *Leader) { players.Init() game := new(Game) sockets := new(Sockets) - sockets.Init(players, game) + sockets.Init(players, nil, game) leader := new(Leader) - leader.Init(players, game, sockets) + leader.Init(players, game, sockets, nil) return players, game, sockets, leader } diff --git a/api/lifecycle.go b/api/lifecycle.go new file mode 100644 index 0000000..2b977ae --- /dev/null +++ b/api/lifecycle.go @@ -0,0 +1,149 @@ +package api + +import ( + "sync" + "time" + + ws "github.com/gorilla/websocket" +) + +// Shutdown budgets. CloseWriteTimeout bounds the WebSocket close-frame write +// for every connection sharing one absolute deadline. ShutdownTimeout bounds +// HTTP draining plus WebSocket worker cleanup in main. +const ( + WebsocketCloseWriteTimeout = 1 * time.Second + ShutdownTimeout = 5 * time.Second +) + +// websocketConn is the subset of Gorilla's API used during shutdown. +// WriteControl and Close are safe for concurrent use with other connection +// methods, which lets shutdown bypass a stalled write pump. +type websocketConn interface { + WriteControl(messageType int, data []byte, deadline time.Time) error + Close() error +} + +// Lifecycle tracks live WebSocket connections and their handler workers so +// server shutdown can notify every endpoint with a deadline-bounded close +// frame instead of sleeping an arbitrary duration. +type Lifecycle struct { + mutex sync.Mutex + shuttingDown bool + connections map[websocketConn]struct{} + wait sync.WaitGroup +} + +// NewLifecycle returns an idle lifecycle ready to track connections. +func NewLifecycle() *Lifecycle { + return &Lifecycle{connections: make(map[websocketConn]struct{})} +} + +// Track registers a connection worker. It returns false when shutdown has +// started, in which case the caller must close the connection and must not +// call Untrack. Each successful Track requires exactly one Untrack call. +func (l *Lifecycle) Track(connection websocketConn) bool { + if l == nil || connection == nil { + return false + } + l.mutex.Lock() + defer l.mutex.Unlock() + if l.shuttingDown { + return false + } + if l.connections == nil { + l.connections = make(map[websocketConn]struct{}) + } + l.connections[connection] = struct{}{} + l.wait.Add(1) + return true +} + +// Untrack removes one worker registration. It is safe to call for a +// connection that is no longer in the snapshot; the WaitGroup accounting +// still pairs with its Track call. +func (l *Lifecycle) Untrack(connection websocketConn) { + if l == nil || connection == nil { + return + } + l.mutex.Lock() + delete(l.connections, connection) + l.mutex.Unlock() + l.wait.Done() +} + +// ShuttingDown reports whether Shutdown has started. New upgrades must be +// rejected once this returns true. +func (l *Lifecycle) ShuttingDown() bool { + if l == nil { + return false + } + l.mutex.Lock() + defer l.mutex.Unlock() + return l.shuttingDown +} + +// ConnectionCount returns the number of distinct tracked connections. It +// exists for tests; production code uses Shutdown. +func (l *Lifecycle) ConnectionCount() int { + if l == nil { + return 0 + } + l.mutex.Lock() + defer l.mutex.Unlock() + return len(l.connections) +} + +// Shutdown sends a Going Away close frame to every tracked connection using +// one shared absolute deadline, closes the underlying sockets, then waits +// until deadline for tracked workers to finish cleanup. It returns true when +// every worker finished before deadline. It is idempotent. +func (l *Lifecycle) Shutdown(deadline time.Time) bool { + if l == nil { + return true + } + l.mutex.Lock() + l.shuttingDown = true + snapshot := make([]websocketConn, 0, len(l.connections)) + for connection := range l.connections { + snapshot = append(snapshot, connection) + } + l.mutex.Unlock() + + var closeWait sync.WaitGroup + for _, connection := range snapshot { + closeWait.Add(1) + go func(connection websocketConn) { + defer closeWait.Done() + _ = connection.WriteControl( + ws.CloseMessage, + ws.FormatCloseMessage(ws.CloseGoingAway, "Server shutting down"), + deadline, + ) + _ = connection.Close() + }(connection) + } + closeWait.Wait() + + done := make(chan struct{}) + go func() { + l.wait.Wait() + close(done) + }() + timeout := time.Until(deadline) + if timeout <= 0 { + select { + case <-done: + return true + default: + return false + } + } + timer := time.NewTimer(timeout) + defer timer.Stop() + select { + case <-done: + return true + case <-timer.C: + return false + } +} diff --git a/api/lifecycle_test.go b/api/lifecycle_test.go new file mode 100644 index 0000000..f163830 --- /dev/null +++ b/api/lifecycle_test.go @@ -0,0 +1,203 @@ +package api + +import ( + "sync" + "testing" + "time" + + ws "github.com/gorilla/websocket" +) + +type recordingShutdownConnection struct { + mutex sync.Mutex + writeCalls int + closeCalls int + code int + deadline time.Time + writeErr error + blockWrite chan struct{} + writeStarted chan struct{} +} + +func (c *recordingShutdownConnection) WriteControl(messageType int, data []byte, deadline time.Time) error { + c.mutex.Lock() + c.writeCalls++ + c.deadline = deadline + c.mutex.Unlock() + if c.writeStarted != nil { + close(c.writeStarted) + } + if c.blockWrite != nil { + <-c.blockWrite + } + if messageType == ws.CloseMessage && len(data) >= 2 { + c.mutex.Lock() + c.code = int(data[0])<<8 | int(data[1]) + c.mutex.Unlock() + } + return c.writeErr +} + +func (c *recordingShutdownConnection) Close() error { + c.mutex.Lock() + c.closeCalls++ + c.mutex.Unlock() + return nil +} + +func (c *recordingShutdownConnection) counts() (writes, closes, code int) { + c.mutex.Lock() + defer c.mutex.Unlock() + return c.writeCalls, c.closeCalls, c.code +} + +func TestLifecycleShutdownNotifiesHealthyConnections(t *testing.T) { + lifecycle := NewLifecycle() + first := &recordingShutdownConnection{} + second := &recordingShutdownConnection{} + if !lifecycle.Track(first) || !lifecycle.Track(second) { + t.Fatal("track healthy connections") + } + before := time.Now() + deadline := before.Add(WebsocketCloseWriteTimeout) + done := make(chan bool, 1) + go func() { + done <- lifecycle.Shutdown(deadline) + }() + // Simulate handler workers finishing after the close frame is written. + // Untrack must happen after Shutdown snapshots, mirroring production + // where handlers exit in response to the socket close. + for _, connection := range []*recordingShutdownConnection{first, second} { + deadline := time.Now().Add(2 * time.Second) + for { + writes, _, _ := connection.counts() + if writes > 0 { + break + } + if time.Now().After(deadline) { + t.Fatal("timed out waiting for close frame") + } + time.Sleep(time.Millisecond) + } + lifecycle.Untrack(connection) + } + if !<-done { + t.Fatal("shutdown did not complete before deadline") + } + for index, connection := range []*recordingShutdownConnection{first, second} { + writes, closes, code := connection.counts() + if writes != 1 || closes != 1 { + t.Errorf("connection %d writes=%d closes=%d, want 1 and 1", index, writes, closes) + } + if code != ws.CloseGoingAway { + t.Errorf("connection %d close code=%d, want %d", index, code, ws.CloseGoingAway) + } + if connection.deadline.Before(before) || connection.deadline.After(deadline.Add(time.Second)) { + t.Errorf("connection %d deadline=%v, want shared deadline %v", index, connection.deadline, deadline) + } + } +} + +func TestLifecycleShutdownSharesOneDeadlineAcrossConnections(t *testing.T) { + lifecycle := NewLifecycle() + first := &recordingShutdownConnection{} + second := &recordingShutdownConnection{} + if !lifecycle.Track(first) || !lifecycle.Track(second) { + t.Fatal("track connections") + } + deadline := time.Now().Add(WebsocketCloseWriteTimeout) + go func() { + lifecycle.Untrack(first) + lifecycle.Untrack(second) + }() + if !lifecycle.Shutdown(deadline) { + t.Fatal("shutdown did not complete") + } + if !first.deadline.Equal(deadline) || !second.deadline.Equal(deadline) { + t.Errorf("deadlines = %v and %v, want shared %v", first.deadline, second.deadline, deadline) + } +} + +func TestLifecycleRejectsNewConnectionsDuringShutdown(t *testing.T) { + lifecycle := NewLifecycle() + stuck := &recordingShutdownConnection{} + if !lifecycle.Track(stuck) { + t.Fatal("track connection") + } + deadline := time.Now().Add(100 * time.Millisecond) + done := make(chan bool, 1) + go func() { + done <- lifecycle.Shutdown(deadline) + }() + // Wait until Shutdown has marked the lifecycle so the racing admission + // observes the shutting-down state. + admissionDeadline := time.Now().Add(2 * time.Second) + for !lifecycle.ShuttingDown() { + if time.Now().After(admissionDeadline) { + t.Fatal("timed out waiting for shutdown to start") + } + time.Sleep(time.Millisecond) + } + if lifecycle.Track(&recordingShutdownConnection{}) { + t.Error("Track succeeded after shutdown started") + } + select { + case result := <-done: + if result { + t.Error("shutdown reported success with an unfinished worker") + } + case <-time.After(2 * time.Second): + t.Fatal("timed out waiting for shutdown timeout") + } + lifecycle.Untrack(stuck) + racing := &recordingShutdownConnection{} + if lifecycle.Track(racing) { + t.Error("Track succeeded after shutdown completed") + lifecycle.Untrack(racing) + } +} + +func TestLifecycleShutdownWithZeroConnectionsCompletes(t *testing.T) { + lifecycle := NewLifecycle() + if !lifecycle.Shutdown(time.Now().Add(WebsocketCloseWriteTimeout)) { + t.Error("zero-connection shutdown did not complete") + } + if !lifecycle.ShuttingDown() { + t.Error("ShuttingDown = false after shutdown") + } +} + +func TestLifecycleShutdownIsIdempotent(t *testing.T) { + lifecycle := NewLifecycle() + connection := &recordingShutdownConnection{} + if !lifecycle.Track(connection) { + t.Fatal("track connection") + } + deadline := time.Now().Add(WebsocketCloseWriteTimeout) + done := make(chan bool, 1) + go func() { + done <- lifecycle.Shutdown(deadline) + }() + writeDeadline := time.Now().Add(2 * time.Second) + for { + writes, _, _ := connection.counts() + if writes > 0 { + break + } + if time.Now().After(writeDeadline) { + t.Fatal("timed out waiting for close frame") + } + time.Sleep(time.Millisecond) + } + lifecycle.Untrack(connection) + if !<-done { + t.Fatal("first shutdown did not complete") + } + if !lifecycle.Shutdown(time.Now().Add(WebsocketCloseWriteTimeout)) { + t.Error("second shutdown did not complete") + } + writes, closes, _ := connection.counts() + if writes != 1 || closes != 1 { + t.Errorf("writes=%d closes=%d, want exactly one close attempt per tracked connection", writes, closes) + } +} diff --git a/api/shutdown_test.go b/api/shutdown_test.go new file mode 100644 index 0000000..2d13db2 --- /dev/null +++ b/api/shutdown_test.go @@ -0,0 +1,129 @@ +package api + +import ( + "net/http" + "net/http/httptest" + "strings" + "testing" + "time" + + ws "github.com/gorilla/websocket" +) + +func newShutdownTestAPI() (*Players, *Sockets, *Admin, *Leader, *Lifecycle) { + players := new(Players) + players.Init() + lifecycle := NewLifecycle() + sockets := new(Sockets) + sockets.Init(players, lifecycle) + game := new(Game) + admin := new(Admin) + admin.Init(players, sockets, "top-secret", lifecycle, game) + leader := new(Leader) + leader.Init(players, game, sockets, lifecycle) + return players, sockets, admin, leader, lifecycle +} + +func dialTestSocket(t *testing.T, url string, cookie *http.Cookie) *ws.Conn { + t.Helper() + header := http.Header{} + if cookie != nil { + header.Set("Cookie", cookie.Name+"="+cookie.Value) + } + connection, _, err := ws.DefaultDialer.Dial(url, header) + if err != nil { + t.Fatalf("dial %s: %v", url, err) + } + return connection +} + +func expectGoingAwayClose(t *testing.T, connection *ws.Conn) { + t.Helper() + _ = connection.SetReadDeadline(time.Now().Add(3 * time.Second)) + for { + _, _, err := connection.ReadMessage() + if err == nil { + // Snapshot or gameplay frame queued before shutdown; keep reading. + continue + } + closeErr, ok := err.(*ws.CloseError) + if !ok { + t.Fatalf("read error = %#v, want CloseError", err) + } + if closeErr.Code != ws.CloseGoingAway { + t.Fatalf("close code = %d, want %d", closeErr.Code, ws.CloseGoingAway) + } + return + } +} + +func TestShutdownNotifiesAllWebSocketEndpoints(t *testing.T) { + players, sockets, admin, leader, lifecycle := newShutdownTestAPI() + playerID := players.New(TypeGhost, "Player", StatusDisc) + leaderID := players.New(TypeAntiPacLeader, "Leader", StatusDisc) + + mux := http.NewServeMux() + mux.Handle("/api/ws/", sockets) + mux.Handle("/api/admin/ws", http.HandlerFunc(admin.ServeSocket)) + mux.Handle("/api/admin/map/ws", http.HandlerFunc(admin.ServeMapSocket)) + mux.Handle("/api/leader/ws", http.HandlerFunc(leader.ServeSocket)) + server := httptest.NewServer(mux) + defer server.Close() + wsBase := "ws" + strings.TrimPrefix(server.URL, "http") + + adminCookie := registerTestAdmin(t, admin, "top-secret") + player := dialTestSocket(t, wsBase+"/api/ws/"+string(playerID), nil) + defer player.Close() + viewer := dialTestSocket(t, wsBase+"/api/admin/map/ws", adminCookie) + defer viewer.Close() + status := dialTestSocket(t, wsBase+"/api/admin/ws", adminCookie) + defer status.Close() + leaderConn := dialTestSocket(t, wsBase+"/api/leader/ws", &http.Cookie{Name: leaderCookieName, Value: string(leaderID)}) + defer leaderConn.Close() + + // Wait until every endpoint registered its connection worker. + admissionDeadline := time.Now().Add(2 * time.Second) + for lifecycle.ConnectionCount() != 4 { + if time.Now().After(admissionDeadline) { + t.Fatalf("tracked connections = %d, want 4", lifecycle.ConnectionCount()) + } + time.Sleep(time.Millisecond) + } + + done := make(chan bool, 1) + go func() { + done <- lifecycle.Shutdown(time.Now().Add(WebsocketCloseWriteTimeout)) + }() + select { + case ok := <-done: + if !ok { + t.Fatal("shutdown did not complete before deadline") + } + case <-time.After(3 * time.Second): + t.Fatal("timed out waiting for shutdown") + } + + for index, connection := range []*ws.Conn{player, viewer, status, leaderConn} { + expectGoingAwayClose(t, connection) + _ = index + } + + if lifecycle.ConnectionCount() != 0 { + t.Errorf("tracked connections after shutdown = %d, want 0", lifecycle.ConnectionCount()) + } + if !lifecycle.ShuttingDown() { + t.Error("ShuttingDown = false after shutdown") + } + racing := &recordingShutdownConnection{} + if lifecycle.Track(racing) { + t.Error("Track succeeded after shutdown") + lifecycle.Untrack(racing) + } +} + +func TestShutdownCompletesWithZeroConnections(t *testing.T) { + _, _, _, _, lifecycle := newShutdownTestAPI() + if !lifecycle.Shutdown(time.Now().Add(WebsocketCloseWriteTimeout)) { + t.Error("zero-connection shutdown did not complete") + } +} diff --git a/api/socket.go b/api/socket.go index 4bfb30a..0570f96 100644 --- a/api/socket.go +++ b/api/socket.go @@ -32,19 +32,14 @@ const ( type Sockets struct { // private - players *Players - hub *Hub + players *Players + hub *Hub + lifecycle *Lifecycle } -// BroadcastShutDown sends a shutdown command to all connected clients. -func (s *Sockets) BroadcastShutDown(command string) { - done := make(chan struct{}) - s.hub.shutdown <- shutdownEvent{command: command, done: done} - <-done -} - -func (s *Sockets) Init(players *Players, games ...*Game) { +func (s *Sockets) Init(players *Players, lifecycle *Lifecycle, games ...*Game) { s.players = players + s.lifecycle = lifecycle s.hub = NewHub(players, games...) go s.hub.Run() @@ -134,6 +129,24 @@ func (c *Conn) readPump(hub *Hub) error { } } +// trackGameSocket registers the read handler and write pump with the shared +// lifecycle. The caller must already hold one Track for the read handler when +// calling this; it acquires the second Track for the write pump. +func (s *Sockets) trackGameSocket(socket *ws.Conn, connection *Conn) bool { + if s.lifecycle == nil { + go connection.writePump(s.hub) + return true + } + if !s.lifecycle.Track(socket) { + return false + } + go func() { + defer s.lifecycle.Untrack(socket) + connection.writePump(s.hub) + }() + return true +} + // WS /api/ws/ // ServeHTTP upgrades the connection to a websocket connection func (s *Sockets) ServeHTTP(w http.ResponseWriter, r *http.Request) { @@ -153,6 +166,13 @@ func (s *Sockets) ServeHTTP(w http.ResponseWriter, r *http.Request) { if err != nil { return } + if s.lifecycle != nil { + if !s.lifecycle.Track(socket) { + _ = socket.Close() + return + } + defer s.lifecycle.Untrack(socket) + } connection := &Conn{ socket: socket, @@ -161,7 +181,10 @@ func (s *Sockets) ServeHTTP(w http.ResponseWriter, r *http.Request) { send: make(chan []byte, socketSendQueueSize), } - go connection.writePump(s.hub) + if !s.trackGameSocket(socket, connection) { + _ = socket.Close() + return + } s.hub.register <- connection fmt.Printf("Sockets\tServeHTTP (/api/ws/):\tID %q: Connection opened.\n", playerID) @@ -180,6 +203,13 @@ func (s *Sockets) ServeViewer(w http.ResponseWriter, r *http.Request) { if err != nil { return } + if s.lifecycle != nil { + if !s.lifecycle.Track(socket) { + _ = socket.Close() + return + } + defer s.lifecycle.Untrack(socket) + } connection := &Conn{ socket: socket, @@ -187,7 +217,10 @@ func (s *Sockets) ServeViewer(w http.ResponseWriter, r *http.Request) { send: make(chan []byte, socketSendQueueSize), } - go connection.writePump(s.hub) + if !s.trackGameSocket(socket, connection) { + _ = socket.Close() + return + } s.hub.register <- connection fmt.Print("Sockets\tServeViewer (/api/admin/map/ws):\tConnection opened.\n") diff --git a/api/socket_test.go b/api/socket_test.go index 6c97e43..d25d455 100644 --- a/api/socket_test.go +++ b/api/socket_test.go @@ -31,68 +31,6 @@ func TestPlayerStaysConnectedUntilLastSocketDisconnects(t *testing.T) { } } -func TestBroadcastShutdownQueuesShutdownCommand(t *testing.T) { - players := new(Players) - players.Init() - playerID := players.New(TypeGhost, "Player", StatusDisc) - hub := NewHub(players) - connection := newTestConnection(playerID) - hub.registerConnection(connection) - drainTestMessages(connection) - - hub.broadcastShutDown(CMD_SHUTDOWN) - message := receiveTestMessage(t, connection) - if message.Command != CMD_SHUTDOWN { - t.Errorf("shutdown command = %q, want %q", message.Command, CMD_SHUTDOWN) - } -} - -func TestBroadcastShutdownEvictsQueuedMessages(t *testing.T) { - players := new(Players) - players.Init() - playerID := players.New(TypeGhost, "Player", StatusDisc) - hub := NewHub(players) - connection := &Conn{ - playerID: playerID, - role: playerConnection, - send: make(chan []byte, 2), - } - hub.connections[connection] = struct{}{} - connection.send <- []byte("stale move") - connection.send <- []byte("stale state") - - hub.broadcastShutDown(CMD_SHUTDOWN) - - if _, exists := hub.connections[connection]; !exists { - t.Fatal("connection was unregistered after successful prioritized delivery") - } - if len(connection.send) != 1 { - t.Fatalf("queued messages = %d, want 1", len(connection.send)) - } - if message := receiveTestMessage(t, connection); message.Command != CMD_SHUTDOWN { - t.Errorf("shutdown command = %q, want %q", message.Command, CMD_SHUTDOWN) - } -} - -func TestBroadcastShutdownUnregistersUndeliverableConnection(t *testing.T) { - players := new(Players) - players.Init() - playerID := players.New(TypeGhost, "Player", StatusDisc) - hub := NewHub(players) - connection := &Conn{ - playerID: playerID, - role: playerConnection, - send: make(chan []byte), - } - hub.connections[connection] = struct{}{} - - hub.broadcastShutDown(CMD_SHUTDOWN) - - if _, exists := hub.connections[connection]; exists { - t.Error("undeliverable connection remains registered") - } -} - func TestRemoveBroadcastPreservesQueuedMessages(t *testing.T) { players := new(Players) players.Init() diff --git a/main.go b/main.go index 3905dbb..e930cca 100644 --- a/main.go +++ b/main.go @@ -1,6 +1,7 @@ package main import ( + "context" "fmt" "log" "net/http" @@ -56,49 +57,89 @@ func main() { } var ( - players api.Players - game api.Game - admin api.Admin - leader api.Leader - sock api.Sockets + players api.Players + game api.Game + admin api.Admin + leader api.Leader + sock api.Sockets + lifecycle = api.NewLifecycle() ) players.Init() // initialize players handler if err := game.Init(&players); err != nil { log.Fatalf("initialize game: %v", err) } - sock.Init(&players, &game) // initialize sockets handler - admin.Init(&players, &sock, adminPassword, &game) // initialize admin handler - leader.Init(&players, &game, &sock) // initialize leader handler + sock.Init(&players, lifecycle, &game) // initialize sockets handler + admin.Init(&players, &sock, adminPassword, lifecycle, &game) // initialize admin handler + leader.Init(&players, &game, &sock, lifecycle) // initialize leader handler - http.Handle("/api/player/", corsMiddleware(&players)) // /api/player/register; /api/player/list.json - http.Handle("/api/admin/", corsMiddleware(&admin)) // registration and authenticated admin operations - http.Handle("/api/leader/", corsMiddleware(&leader)) // authenticated leader operations - http.Handle("/api/game/", corsMiddleware(&game)) // /api/game/map.json - http.Handle("/api/ws/", corsMiddleware(&sock)) // /api/ws/ + mux := http.NewServeMux() + mux.Handle("/api/player/", corsMiddleware(&players)) // /api/player/register; /api/player/list.json + mux.Handle("/api/admin/", corsMiddleware(&admin)) // registration and authenticated admin operations + mux.Handle("/api/leader/", corsMiddleware(&leader)) // authenticated leader operations + mux.Handle("/api/game/", corsMiddleware(&game)) // /api/game/map.json + mux.Handle("/api/ws/", corsMiddleware(&sock)) // /api/ws/ port := ":49152" + server := &http.Server{Addr: port, Handler: mux} // print to terminal that server started fmt.Printf("Started PacMacro; listening on localhost%s...\n", port) // PacMacro API is served on port 49152. // this should be proxied inside the web server used. + serverErr := make(chan error, 1) go func() { - if err := http.ListenAndServe(port, nil); err != nil && err != http.ErrServerClosed { - log.Fatal(err) - } + serverErr <- server.ListenAndServe() }() // Block until SIGINT (Ctrl+C) or SIGTERM (systemd stop/restart). - quit := make(chan os.Signal, 1) - signal.Notify(quit, syscall.SIGINT, syscall.SIGTERM) - <-quit + ctx, stop := signal.NotifyContext(context.Background(), syscall.SIGINT, syscall.SIGTERM) + defer stop() + select { + case err := <-serverErr: + if err != nil && err != http.ErrServerClosed { + shutdownWebSockets(lifecycle) + log.Fatalf("listen and serve: %v", err) + } + return + case <-ctx.Done(): + } 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) + shutdown(server, lifecycle) fmt.Println("Server exiting.") } + +func shutdownWebSockets(lifecycle *api.Lifecycle) { + if !lifecycle.Shutdown(time.Now().Add(api.WebsocketCloseWriteTimeout)) { + fmt.Println("Shutdown timed out waiting for WebSocket workers.") + } +} + +func shutdown(server *http.Server, lifecycle *api.Lifecycle) { + // Close listeners immediately so no new HTTP or WebSocket connections are + // accepted while existing connections drain. + shutdownCtx, cancel := context.WithTimeout(context.Background(), api.ShutdownTimeout) + defer cancel() + + httpDone := make(chan struct{}) + go func() { + _ = server.Shutdown(shutdownCtx) + close(httpDone) + }() + + // Notify WebSocket clients concurrently with HTTP draining. All close + // frames share one absolute deadline so stalled clients share the budget. + wsDone := make(chan bool, 1) + wsDeadline := time.Now().Add(api.WebsocketCloseWriteTimeout) + go func() { + wsDone <- lifecycle.Shutdown(wsDeadline) + }() + + <-httpDone + websocketsClean := <-wsDone + if !websocketsClean { + fmt.Println("Shutdown timed out waiting for WebSocket workers.") + } +}