LSU's own coaching surface, and what auditing the site turned up

Review threads live on the whole diff, not on one commit, so none are shown here - a thread's line means something in the branch's final form, and painting it into an intermediate step would put it on code it is not about.

resources/functions/coaching-source.tsadded+83-0

Changes to resources/functions/coaching-source.ts

		@@ -0,0 +1,83 @@
	1	+import type { CoachingBrand } from './coaching-brand'
	2	+import coaching from '../../config/coaching'
	3	+import { parseBrand } from './coaching-brand'
	4	+
	5	+/**
	6	+ * Fetching the published brand, once, for a whole render.
	7	+ *
	8	+ * Nine pages on this site want the same payload, and a page render must never
	9	+ * wait on another service to finish: a slow HQ would otherwise hold the whole
	10	+ * coaching section open, and a down HQ would hold it open until the proxy gave
	11	+ * up. So the call is bounded, it never throws, and the answer is remembered
	12	+ * for a short while.
	13	+ *
	14	+ * A miss is remembered too, for a shorter time. An outage that is not cached
	15	+ * turns one visitor per render into one upstream request per render, which is
	16	+ * precisely the load an outage cannot absorb.
	17	+ */
	18	+
	19	+/** Long enough that a visitor moving between modules makes one request. */
	20	+const HIT_TTL_MS = 60_000
	21	+/** Short enough that a brand going live shows up within a minute. */
	22	+const MISS_TTL_MS = 10_000
	23	+/** A render blocked on another service is a broken page, not a slow one. */
	24	+const TIMEOUT_MS = 2500
	25	+
	26	+interface Entry {
	27	+ brand: CoachingBrand | null
	28	+ until: number
	29	+}
	30	+
	31	+const cache = new Map<string, Entry>()
	32	+/** One in-flight request per brand, so a cold cache does not stampede. */
	33	+const inFlight = new Map<string, Promise<CoachingBrand | null>>()
	34	+
	35	+/** The endpoint, built from config rather than from anything a visitor sends. */
	36	+export function brandEndpoint(origin = coaching.origin, slug = coaching.brand): string {
	37	+ return new URL(`/api/brands/${encodeURIComponent(slug)}`, origin).toString()
	38	+}
	39	+
	40	+async function request(slug: string, origin: string): Promise<CoachingBrand | null> {
	41	+ try {
	42	+ const response = await fetch(brandEndpoint(origin, slug), {
	43	+ headers: { accept: 'application/json' },
	44	+ signal: AbortSignal.timeout(TIMEOUT_MS),
	45	+ })
	46	+ // 404 is the correct answer for a brand that is not published yet.
	47	+ if (!response.ok) return null
	48	+ return parseBrand(await response.json(), slug)
	49	+ }
	50	+ catch {
	51	+ // A timeout, a DNS failure and malformed JSON are all "nothing to show".
	52	+ return null
	53	+ }
	54	+}
	55	+
	56	+/**
	57	+ * The published brand, or null. Never throws, never waits more than a moment,
	58	+ * and never makes the same call twice in a row.
	59	+ */
	60	+export async function loadBrand(slug = coaching.brand, origin = coaching.origin): Promise<CoachingBrand | null> {
	61	+ const key = `${origin}|${slug}`
	62	+ const cached = cache.get(key)
	63	+ if (cached && cached.until > Date.now()) return cached.brand
	64	+
	65	+ const existing = inFlight.get(key)
	66	+ if (existing) return existing
	67	+
	68	+ const pending = request(slug, origin)
	69	+ .then((brand) => {
	70	+ cache.set(key, { brand, until: Date.now() + (brand ? HIT_TTL_MS : MISS_TTL_MS) })
	71	+ return brand
	72	+ })
	73	+ .finally(() => inFlight.delete(key))
	74	+
	75	+ inFlight.set(key, pending)
	76	+ return pending
	77	+}
	78	+
	79	+/** For tests, and for a dev server that should not hold a stale brand. */
	80	+export function forgetBrand(): void {
	81	+ cache.clear()
	82	+ inFlight.clear()
	83	+}

tests/coaching-source.test.tsadded+108-0

Changes to tests/coaching-source.test.ts

		@@ -0,0 +1,108 @@
	1	+import type { Server } from 'bun'
	2	+import { afterEach, beforeEach, describe, expect, it } from 'bun:test'
	3	+import { brandEndpoint, forgetBrand, loadBrand } from '../resources/functions/coaching-source'
	4	+
	5	+/**
	6	+ * These stand a throwaway server in for HQ. The point is not the happy path -
	7	+ * coaching-surface.test.ts pins that against a recorded payload - it is that
	8	+ * every unhappy path ends in a null brand quickly and quietly, because a
	9	+ * coaching page that waits on another service is a page that does not render.
	10	+ */
	11	+
	12	+let server: Server
	13	+let hits = 0
	14	+
	15	+function serve(handler: (request: Request) => Response | Promise<Response>): string {
	16	+ hits = 0
	17	+ server = Bun.serve({
	18	+ port: 0,
	19	+ fetch(request) {
	20	+ hits += 1
	21	+ return handler(request)
	22	+ },
	23	+ })
	24	+ return `http://127.0.0.1:${server.port}`
	25	+}
	26	+
	27	+const published = {
	28	+ slug: 'last-soul-ultra',
	29	+ settings: { name: 'Last Soul Ultra', headline: 'Train for the next start.', modules: ['plans'] },
	30	+ offers: [],
	31	+}
	32	+
	33	+beforeEach(() => forgetBrand())
	34	+afterEach(async () => { await server?.stop(true) })
	35	+
	36	+describe('loading the published brand', () => {
	37	+ it('asks the configured origin for the configured slug', () => {
	38	+ expect(brandEndpoint('https://hq.training', 'last-soul-ultra'))
	39	+ .toBe('https://hq.training/api/brands/last-soul-ultra')
	40	+ })
	41	+
	42	+ it('escapes a slug rather than letting it reshape the path', () => {
	43	+ expect(brandEndpoint('https://hq.training', '../org/experience'))
	44	+ .toBe('https://hq.training/api/brands/..%2Forg%2Fexperience')
	45	+ })
	46	+
	47	+ it('reads a published brand', async () => {
	48	+ const origin = serve(() => Response.json(published))
	49	+ expect((await loadBrand('last-soul-ultra', origin))?.name).toBe('Last Soul Ultra')
	50	+ })
	51	+
	52	+ it('answers null for a brand that is not published yet', async () => {
	53	+ const origin = serve(() => new Response('not found', { status: 404 }))
	54	+ expect(await loadBrand('last-soul-ultra', origin)).toBeNull()
	55	+ })
	56	+
	57	+ it('answers null when HQ errors, rather than failing the page', async () => {
	58	+ const origin = serve(() => new Response('boom', { status: 500 }))
	59	+ expect(await loadBrand('last-soul-ultra', origin)).toBeNull()
	60	+ })
	61	+
	62	+ it('answers null for a response that is not JSON', async () => {
	63	+ const origin = serve(() => new Response('<html>maintenance</html>', { headers: { 'content-type': 'text/html' } }))
	64	+ expect(await loadBrand('last-soul-ultra', origin)).toBeNull()
	65	+ })
	66	+
	67	+ it('answers null when nothing is listening at all', async () => {
	68	+ // Port 9 (discard) refuses fast and needs no server of its own.
	69	+ server = Bun.serve({ port: 0, fetch: () => new Response('') })
	70	+ expect(await loadBrand('last-soul-ultra', 'http://127.0.0.1:9')).toBeNull()
	71	+ })
	72	+
	73	+ it('gives up on an HQ that never answers', async () => {
	74	+ const origin = serve(async () => { await Bun.sleep(30_000); return new Response('late') })
	75	+ const started = performance.now()
	76	+
	77	+ expect(await loadBrand('last-soul-ultra', origin)).toBeNull()
	78	+ expect(performance.now() - started).toBeLessThan(5000)
	79	+ }, 10_000)
	80	+
	81	+ it('makes one request for a whole render, not one per page', async () => {
	82	+ const origin = serve(() => Response.json(published))
	83	+
	84	+ await loadBrand('last-soul-ultra', origin)
	85	+ await loadBrand('last-soul-ultra', origin)
	86	+ await loadBrand('last-soul-ultra', origin)
	87	+
	88	+ expect(hits).toBe(1)
	89	+ })
	90	+
	91	+ it('does not stampede a cold cache', async () => {
	92	+ const origin = serve(async () => { await Bun.sleep(50); return Response.json(published) })
	93	+
	94	+ const all = await Promise.all([1, 2, 3, 4, 5].map(() => loadBrand('last-soul-ultra', origin)))
	95	+
	96	+ expect(hits).toBe(1)
	97	+ expect(all.every(brand => brand?.name === 'Last Soul Ultra')).toBe(true)
	98	+ })
	99	+
	100	+ it('caches an outage too, so a down HQ is not asked once per visitor', async () => {
	101	+ const origin = serve(() => new Response('boom', { status: 503 }))
	102	+
	103	+ await loadBrand('last-soul-ultra', origin)
	104	+ await loadBrand('last-soul-ultra', origin)
	105	+
	106	+ expect(hits).toBe(1)
	107	+ })
	108	+})