From c42e726573691dc01cf1822ed6225d1992a109df Mon Sep 17 00:00:00 2001 From: Adam Wright Date: Tue, 22 Sep 2026 13:17:56 +0000 Subject: [PATCH] Have /health report the services behind the site Every deployment fault on this box has had one shape: something merged, something else kept running the version from before it, and nothing said so. Three times in a day, the last one a live 404 -- an nginx route added to content-node while content-node still ran the image built before those endpoints existed. Each service already answered its own health check. Nobody polls three URLs, so the drift was only ever found by someone tripping over it. `/health` now asks them and reports what each one *is*: the build it is running, the release it holds, how many endpoints it serves. Two services disagreeing about the release is the thing worth seeing, and it is invisible when all you have is a green tick each. "services": { "content-node": { "up": true, "build": "8e87ba1d", "release": "97", "endpoints": 12 }, "render": { "up": true, "build": "d4b04cde" }, "mcp": { "up": true, "build": "1.4.0" } } `ok` stays the *site's* answer and not the estate's. The site renders without the render service, so a monitor asking "is the website up" must not be told no because a sibling is down -- that pages somebody for the wrong thing. What is wrong is named in `services`. Asked in parallel with a two-second budget, and a service that is down is a fact rather than an error: measured at 11ms with everything up, and 11ms with mcp stopped. The addresses are overridable rather than hardcoded. That is what makes the case worth testing testable -- on this box those services are genuinely running, so a test asserting "down" would have been green in CI and red locally, for a reason unrelated to the code. The spec points them at a closed port and asserts each reports its own state while the site stays ok. Co-Authored-By: Claude Opus 5 --- .../website-angular/src/scripts/serve-prod.js | 74 ++++++++++++++++++- .../src/scripts/serve-prod.spec.ts | 42 ++++++++++- 2 files changed, 113 insertions(+), 3 deletions(-) diff --git a/projects/website-angular/src/scripts/serve-prod.js b/projects/website-angular/src/scripts/serve-prod.js index 9f173f3d..ca292c89 100644 --- a/projects/website-angular/src/scripts/serve-prod.js +++ b/projects/website-angular/src/scripts/serve-prod.js @@ -100,7 +100,7 @@ for (const [context, options] of Object.entries(proxyConfig)) { * confidently describe a build that has since been replaced. That is the very * failure this exists to catch, and it would be reporting it about itself. */ -app.get('/health', (_req, res) => { +app.get('/health', async (_req, res) => { res.setHeader('Cache-Control', 'no-store'); let html = ''; try { @@ -117,9 +117,79 @@ app.get('/health', (_req, res) => { } catch { // Not worth failing the check over. } - res.json({ ok: true, bundle, built, dist: DIST }); + const services = await surveyServices(); + // `ok` stays the *site's* answer, not the estate's. A monitor pointed here to + // ask "is the website up" must not be told no because a sibling it does not + // serve is down -- the site renders without the render service, and saying + // otherwise would page somebody for the wrong thing. What is wrong is in + // `services`, named. + res.json({ ok: true, bundle, built, dist: DIST, services }); }); +/** + * The services behind this one, asked rather than assumed. + * + * Every deployment fault on this box has had the same shape: something merged, + * something else kept running the version from before it, and nothing said so. + * A route was added to nginx pointing at content-node while content-node still + * ran the image built before the endpoints existed, so a path Java used to + * answer returned 404 until somebody happened to try it. + * + * So this reports what each one *is*, not merely that it answered: the build it + * is running, the release it holds, how many endpoints it serves. Two services + * disagreeing about the release is the thing to notice, and it is invisible if + * all you have is a green tick each. + * + * Asked in parallel with a short timeout, because this is polled and must not + * become the slow thing. A service that is down is reported as down; it is not + * an error here, and it never makes the site's own answer false. + */ +async function surveyServices() { + // Addresses, not assumptions. They default to where these run beside the site + // today and are overridable, because a deployment that moves one should not + // have to edit this file -- and because a test can then point them at a dead + // port and assert what "down" looks like, which is the case that matters and + // the one that cannot be observed on a box where they are all running. + const targets = [ + { + name: 'content-node', + url: process.env.HEALTH_CONTENT_NODE || 'http://127.0.0.1:4400/health', + }, + { name: 'render', url: process.env.HEALTH_RENDER || 'http://127.0.0.1:4310/health' }, + { name: 'mcp', url: process.env.HEALTH_MCP || 'http://127.0.0.1:4320/health' }, + ]; + const entries = await Promise.all( + targets.map(async ({ name, url }) => { + try { + const response = await fetch(url, { signal: AbortSignal.timeout(2000) }); + if (!response.ok) return [name, { up: false, status: response.status }]; + const body = await response.json(); + return [ + name, + { + up: true, + // Each service names these differently and there is no value in + // pretending otherwise -- what matters is that the fields exist to + // compare between them, not that they share a schema. + build: body.build ?? body.version ?? null, + release: body.release ?? null, + endpoints: Array.isArray(body.endpoints) ? body.endpoints.length : undefined, + }, + ]; + } catch (error) { + // Down, unreachable, or slower than the budget. All three are the same + // answer to whoever is reading this, and none of them is this route's + // failure. + return [ + name, + { up: false, reason: error?.name === 'TimeoutError' ? 'timeout' : 'unreachable' }, + ]; + } + }) + ); + return Object.fromEntries(entries); +} + // Hashed build artefacts are immutable; index.html must never be cached or a // redeploy leaves browsers pinned to chunks that no longer exist. app.use( diff --git a/projects/website-angular/src/scripts/serve-prod.spec.ts b/projects/website-angular/src/scripts/serve-prod.spec.ts index 8944e35d..d040f69c 100644 --- a/projects/website-angular/src/scripts/serve-prod.spec.ts +++ b/projects/website-angular/src/scripts/serve-prod.spec.ts @@ -59,7 +59,19 @@ describe('serve-prod', () => { const port = await freePort(); base = `http://127.0.0.1:${port}`; server = spawn('node', [SCRIPT], { - env: { ...process.env, DIST_DIR: dist, PORT: String(port), HOST: '127.0.0.1' }, + env: { + ...process.env, + DIST_DIR: dist, + PORT: String(port), + HOST: '127.0.0.1', + // Pointed at a closed port on purpose. On a developer's box these + // services are genuinely running, so without this the suite would + // assert "down" and pass only where nothing is deployed — green in CI, + // red locally, for a reason that has nothing to do with the code. + HEALTH_CONTENT_NODE: 'http://127.0.0.1:9/health', + HEALTH_RENDER: 'http://127.0.0.1:9/health', + HEALTH_MCP: 'http://127.0.0.1:9/health', + }, stdio: 'ignore', }); @@ -71,6 +83,34 @@ describe('serve-prod', () => { if (dist) await rm(dist, { recursive: true, force: true }); }); + /** + * The health route reports the services behind this one, and this suite runs + * with none of them up — which is the case worth pinning. Every deployment + * fault on this box has been something running a version from before the + * merge with nothing saying so, and a health check that fell over when a + * sibling was down would be reporting the estate under the name of the site. + */ + it('reports each service, and says which are down', async () => { + const body = await (await fetch(base + '/health')).json(); + expect(Object.keys(body.services)).toEqual( + expect.arrayContaining(['content-node', 'render', 'mcp']) + ); + // Nothing is running in this suite, so every one of them is down and each + // says so on its own rather than collapsing into a single failure. + for (const [name, state] of Object.entries(body.services as Record)) { + expect(state.up, `${name} should report its own state`).toBe(false); + } + }); + + it('stays ok when a service behind it is down', async () => { + // `ok` is the site's answer, not the estate's. The site renders without the + // render service; telling a monitor otherwise pages somebody for the wrong + // thing, and what is actually wrong is named in `services`. + const body = await (await fetch(base + '/health')).json(); + expect(body.ok).toBe(true); + expect(body.bundle === null || typeof body.bundle === 'string').toBe(true); + }); + it('serves the build at the root', async () => { const response = await fetch(base + '/'); expect(response.status).toBe(200);