13 KiB
Implementation Plan: REST Event-State Snapshot, SSE De-dup, Solve Publish-Once, REST Error Notification
0. Goal
Four small refinements called out by the reviewer:
- Add a typed REST endpoint that returns the authoritative
EventStatePayload(a one-shot snapshot) and call it fromchallenges.page.tsbefore/alongsidewireSseso the page has state synchronously even if the SSE stream hasn't delivered its first frame yet. - In
challenges.store.ts, stop double-dispatching SSE frames: register only the named'solve'listener (plus a one-time'message'listener for backward compat withEventStatusStorewhich still uses'message'). Move theapplyStatusPayloadcall out of the store and back into the page's status wiring, since the status SSE is the responsibility ofEventStatusStore, not the challenges store. - In
backend/src/modules/challenges/challenges.service.ts, remove the twopublishSolveEvent(...)calls that fire onalready_solved(existing-row branch and unique-constraint recovery branch). Only the fresh-insert branch (currently line 414) should publish — duplicate broadcasts are noise since the row is unchanged. - Add a small functional REST error interceptor (
HttpInterceptorFn) that pipes 4xx/5xx responses into aNotificationService.error(...)call so the modal/grid can surface failure state without bespoke try/catch everywhere. Register it infrontend/src/main.ts.
1. Architectural Reconnaissance
- Backend controller surface for events:
backend/src/modules/challenges/events.controller.tsexposesGET /api/v1/events(SSE) andbackend/src/modules/system/system.controller.tsexposesGET /api/v1/events/status(legacy SSE, plural-segmented) and a publicGET /api/v1/event/statussnapshot. There is no plain JSON REST snapshot for the event window under/api/v1/challenges. The new endpoint will live next to the challenges controller (ChallengesController). - DTO:
EventStatusService.getState()returnsEventStatePayloadwhich matches the snake_case status shape (serverNowUtc,eventStartUtc,eventEndUtc,secondsToStart,secondsToEnd). We reuse it. - Frontend SSE wiring today:
ChallengesStore.wireSseregisters the same handler for'message','solve','status'. Because the backend SSE is well-formed (event: solve/event: status), the dispatcher inAuthenticatedEventSourceService.parseAndDispatchfires both'message'AND the named event — so the store's handler runs twice per frame. Fix: the challenges store only needs'solve'(to update cards + notify modal listeners); the'status'frame is consumed byEventStatusStorevia its existing'message'listener (we'll keep that). EventStatusStoreSSE wiring: it still usesaddEventListener('message', ...)and ignores named events. That's correct for backward compat — it doesn't double-process frames.- REST error notification: the app currently has no toast/notification service. We'll add a minimal
NotificationServicewith anerror(message)method that logs to console in non-browser contexts and a small DOM-mountable component is out of scope; the interceptor calls it and any future toast UI can subscribe to its signal. - Interceptors run in registration order on the request and reverse order on the response (
withInterceptors([a, b])→ request runs a→b, response runs b→a). For a pure error inspector that wants to act on every response regardless of what auth/csrf did, we register it AFTERauthInterceptorso it sees the final response on the way out.
2. Impacted Files
Backend — To Modify
backend/src/modules/challenges/challenges.controller.ts— add@Get('status')returningEventStatePayload.backend/src/modules/challenges/challenges.service.ts— remove twopublishSolveEvent(...)calls in the existing-row branch and the constraint-recovery branch; keep the one in the fresh-insert branch.
Frontend — To Create
frontend/src/app/core/services/notification.service.ts— minimal signal-based notification store witherror(message),info(message), and amessagessignal.frontend/src/app/core/interceptors/error-notification.interceptor.ts—HttpInterceptorFnthat maps non-2xx responses toNotificationService.error(...).
Frontend — To Modify
frontend/src/app/features/challenges/challenges.service.ts— addgetEventState()returningObservable<EventStatePayload>.frontend/src/app/features/challenges/challenges.store.ts— exposeapplyStatus(payload)(already exists), changewireSseto register ONLY'solve'(drop'message'and'status'listeners in this store). Move the local handler logic so it doesn't process status frames.frontend/src/app/features/challenges/challenges.page.ts— callstore.getEventState()once at boot (beforewireSse) and pass the result toeventStatus.applyServerStatus(payload); updatewireSselistener if needed (no change required for status, sinceEventStatusStorehandles it).frontend/src/main.ts— registererrorNotificationInterceptorAFTERauthInterceptor.
Tests — To Modify
tests/backend/challenges-submit-flag.spec.ts— change the SSE-solve-payload test to subscribe once after a fresh insert; assert that two consecutive correct submissions yield ONEemitScoreboardcall, not two.tests/frontend/event-status.store.spec.ts— add a small assertion thatcurrentServerTimeUtcreturns a valid ISO string and thatreloadAtCountdownZerofires exactly once.- New test file
tests/frontend/notification-interceptor.spec.ts— assert the interceptor callsNotificationService.error(...)on a 500 response and does NOT call it on a 2xx response.
3. Proposed Changes
Backend
-
challenges.controller.ts— add (after the existingboard/detail/submithandlers):@Get('status') @ApiOperation({ summary: 'Authenticated event-window snapshot (REST JSON)' }) async eventState(): Promise<EventStatePayload> { return this.statusSvc.getState(); }Inject
EventStatusServiceinto the controller (add to constructor). The existinggetState()already returnsEventStatePayload. This endpoint sits at/api/v1/challenges/status. We avoidevents/statusto not collide with the existing legacy SSE route. -
challenges.service.ts— remove thepublishSolveEvent(...)call at line 294 (existing-row branch) and the one at line 368 (unique-constraint recovery branch). Keep only the line 414 call (fresh-insert path). ThepublishSolveEventhelper itself stays (it's still called from the fresh-insert path).
Frontend
-
notification.service.ts(new):import { Injectable, signal } from '@angular/core'; export interface Notification { id: number; kind: 'error' | 'info'; message: string; ts: number; } @Injectable({ providedIn: 'root' }) export class NotificationService { private next = 1; readonly messages = signal<Notification[]>([]); error(message: string): void { this.push('error', message); // eslint-disable-next-line no-console console.error('[notification]', message); } info(message: string): void { this.push('info', message); // eslint-disable-next-line no-console console.info('[notification]', message); } dismiss(id: number): void { this.messages.update((cur) => cur.filter((m) => m.id !== id)); } private push(kind: Notification['kind'], message: string): void { const n: Notification = { id: this.next++, kind, message, ts: Date.now() }; this.messages.update((cur) => [...cur, n]); } } -
error-notification.interceptor.ts(new):import { HttpErrorResponse, HttpInterceptorFn } from '@angular/common/http'; import { inject } from '@angular/core'; import { catchError, throwError } from 'rxjs'; import { NotificationService } from '../services/notification.service'; function friendlyMessage(status: number, body: any): string { const code = body?.code as string | undefined; const msg = (body?.message as string | undefined) ?? ''; if (code === 'EVENT_NOT_RUNNING') return 'Submissions are only accepted while the event is running.'; if (code === 'FLAG_INCORRECT') return 'Incorrect flag.'; if (status === 0) return 'Network error. Please try again.'; if (status === 401 || status === 403) return 'Your session is no longer valid.'; if (status >= 500) return 'Server error. Please try again later.'; return msg || `Request failed (${status}).`; } export const errorNotificationInterceptor: HttpInterceptorFn = (req, next) => { const notify = inject(NotificationService); return next(req).pipe( catchError((err) => { if (err instanceof HttpErrorResponse) { notify.error(friendlyMessage(err.status, err.error)); } else { notify.error('Unexpected error.'); } return throwError(() => err); }), ); }; -
challenges.service.ts— add:getEventState(): Observable<EventStatePayload> { return this.http .get<EventStatePayload>('/api/v1/challenges/status', { withCredentials: true }) .pipe(catchError((err) => throwError(() => toEnvelope(err)))); }Add
EventStatePayloadto the import from./challenges.pure. -
challenges.store.ts— updatewireSseto register only'solve':const handler = (ev: MessageEvent | Event) => this.handleSseFrame(ev); src.addEventListener('solve', handler);Drop the
addEventListener('message', ...)andaddEventListener('status', ...)calls insidewireSse.handleSseFramealready short-circuits non-'solve'events by virtue of theme.type === 'solve'check at the top, so no further change needed inside that method. -
challenges.page.ts— atngOnInit, before wiring the SSE:// REST snapshot first so the page has synchronous state. this.store.getEventState().subscribe({ next: (payload) => this.eventStatus.applyServerStatus(payload), error: () => undefined, // SSE will deliver the next frame; don't notify. });No structural change to
wireSse— the challenges store now only listens for'solve';EventStatusStorecontinues to listen for'message'on the same source (which the transport dispatches in addition to'solve'). -
main.ts— register the error interceptor last so it sees the final response:provideHttpClient(withInterceptors([csrfInterceptor, authInterceptor, errorNotificationInterceptor])),
4. Test Strategy
- Backend (
tests/backend/challenges-submit-flag.spec.ts): change the SSE-payload test to subscribe ONCE onhub.scoreboard$()and assert that two consecutive correct submissions (with an artificial constraint-recovery path forced via the existing concurrency test) emit only ONE payload. Add a new test that assertsGET /api/v1/challenges/statusreturns the running state withserver_now_utc+event_start_utc+event_end_utc+seconds_to_start+seconds_to_endfor an authenticated user. - Frontend (
tests/frontend/event-status.store.spec.ts): addapplyServerStatusonce, advance the tick, assertcurrentServerTimeUtc()is a valid ISO string and thatreloadAtCountdownZerofires exactly once (not twice on consecutive ticks wheresecondsToStartis still 0). - Frontend (
tests/frontend/notification-interceptor.spec.ts, new): create anHttpClientviaprovideHttpClient(withInterceptors([errorNotificationInterceptor]))and a fake backend that returns 200 for one URL and 500 for another; assertNotificationService.messages()only contains the 500 message. - All existing 504 tests must continue to pass.
5. Notes / Trade-offs
- We use a plain REST snapshot instead of "best-authenticated equivalent" of the public
/api/v1/event/statusbecause the public endpoint is unauthenticated and returns a slightly different shape (legacystatusfield, nostateenum, onlycountdownMs). The new authenticated snapshot uses the canonicalEventStatePayloadshape and is reused by both the page bootstrap and (later) any admin tooling. - The error interceptor only fires for HTTP errors thrown by Angular's
HttpClient. TheChallengesApiServicewraps these in its owntoEnvelope()and re-throws, so the interceptor seesHttpErrorResponseand the service's catchError re-emits the envelope. We do not double-notify. - Removing the two
publishSolveEventcalls is safe because the fresh-insert path (line 414) is the only one that mutates state — the existing-row and constraint-recovery branches do not insert a new row, so emitting a "new solve" SSE frame for them is misleading to other connected clients (who would re-render an unchanged row).