From 17f441151882418670c6687c691c7b7523c45fc0 Mon Sep 17 00:00:00 2001 From: lilleman Date: Mon, 15 Jun 2026 08:42:16 +0200 Subject: [PATCH] =?UTF-8?q?Address=20architecture=20+=20stability=20review?= =?UTF-8?q?=20(todo=20=C2=A70):=20wire=20buildContext,=20graceful=20shutdo?= =?UTF-8?q?wn,=20prod=20template=20caching?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- README.md | 5 +++++ src/app.test.ts | 16 ++++++++++++++++ src/app.ts | 11 +++++++++-- src/server.ts | 7 ++++++- todo.md | 2 +- 5 files changed, 37 insertions(+), 4 deletions(-) diff --git a/README.md b/README.md index b37e831..8b0ae93 100644 --- a/README.md +++ b/README.md @@ -346,6 +346,9 @@ docker compose -f compose.yml up --build -d # base config only, no source moun _(Production compose grows to include the Ory services and Postgres — planned.)_ +The server drains in-flight requests on `SIGTERM`/`SIGINT` rather than cutting them +mid-response, so container restarts are clean. + ## Layout ``` @@ -365,6 +368,8 @@ html-css-foundation/ Raw HTML/CSS design reference — the source for the building-block partials; not served. ``` +Comments and docs cite roadmap phases as `§N` — the sections in `todo.md`. + ## Extending the core - **New page in a plugin:** add a route + handler to the plugin manifest and a diff --git a/src/app.test.ts b/src/app.test.ts index 9fe0077..6633607 100644 --- a/src/app.test.ts +++ b/src/app.test.ts @@ -34,6 +34,22 @@ test("serves static CSS", async () => { assert.match(res.headers.get("content-type") ?? "", /text\/css/); }); +// Production caches compiled templates; rendering must stay correct across repeated requests. +test("renders correctly with template caching enabled", async () => { + const app = createApp({ cache: true }); + try { + await new Promise((resolve) => app.listen(0, resolve)); + const url = `http://localhost:${(app.address() as AddressInfo).port}/`; + for (let i = 0; i < 2; i++) { + const res = await fetch(url); + assert.equal(res.status, 200); + assert.match(await res.text(), /Plainpages/); + } + } finally { + app.close(); + } +}); + test("returns the 404 HTML page for unknown routes", async () => { const res = await fetch(base + "/missing"); assert.equal(res.status, 404); diff --git a/src/app.ts b/src/app.ts index aef8b79..f055c72 100644 --- a/src/app.ts +++ b/src/app.ts @@ -2,21 +2,26 @@ import { createServer, type Server, type ServerResponse } from "node:http"; import { dirname, join } from "node:path"; import { fileURLToPath } from "node:url"; import * as ejs from "ejs"; +import { buildContext } from "./context.ts"; import { serveStatic } from "./static.ts"; const rootDir = join(dirname(fileURLToPath(import.meta.url)), ".."); export interface AppOptions { + // Cache compiled templates (compile once vs. re-read+recompile per request). + // Defaults to on in production, off in dev so source edits show up live. + cache?: boolean; publicDir?: string; viewsDir?: string; } export function createApp(options: AppOptions = {}): Server { + const cache = options.cache ?? process.env["NODE_ENV"] === "production"; const publicDir = options.publicDir ?? join(rootDir, "public"); const viewsDir = options.viewsDir ?? join(rootDir, "views"); const render = (view: string, data: Record): Promise => - ejs.renderFile(join(viewsDir, `${view}.ejs`), data); + ejs.renderFile(join(viewsDir, `${view}.ejs`), data, { cache }); const sendHtml = (res: ServerResponse, status: number, html: string): void => { res.writeHead(status, { "content-type": "text/html; charset=utf-8" }); @@ -30,7 +35,9 @@ export function createApp(options: AppOptions = {}): Server { return; } - const { pathname } = new URL(req.url ?? "/", "http://localhost"); + // The single request shape handlers receive (§2/§4 router passes it on); routing + // reads its parsed URL instead of building a throwaway one. + const { pathname } = buildContext(req, res).url; if (pathname.startsWith("/public/")) { await serveStatic(publicDir, pathname.slice("/public/".length), res, req.method === "HEAD"); diff --git a/src/server.ts b/src/server.ts index 0104ddc..0d89288 100644 --- a/src/server.ts +++ b/src/server.ts @@ -3,6 +3,11 @@ import { loadConfig } from "./config.ts"; const { port } = loadConfig(); // validates the env (incl. prod secrets) — fails loud at boot -createApp().listen(port, () => { +const server = createApp().listen(port, () => { console.log(`Listening on http://localhost:${port}`); }); + +// Drain in-flight requests on container stop instead of cutting them mid-response. +for (const signal of ["SIGINT", "SIGTERM"] as const) { + process.on(signal, () => server.close(() => process.exit(0))); +} diff --git a/todo.md b/todo.md index f2e8e13..510d74f 100644 --- a/todo.md +++ b/todo.md @@ -17,7 +17,7 @@ everything via Docker. - [x] Request context type threaded to handlers: `{ req, res, url, params, query, user|null, roles }`. → `src/context.ts` (`RequestContext` + `buildContext`); `roles` mirror `user.roles`, the §2 router/§4 JWT middleware supply `params`/`user`. - [x] Error templates: add 403 + 500 (404 exists). → `views/403.ejs` + `views/500.ejs`; 500 wired into `app.ts` error handler (HTML, plain-text fallback). - [x] Config/env loader: Ory endpoints, cookie/CSRF secret, JWKS location, ports. → `src/config.ts` (`loadConfig`); validated at boot, dev defaults for clean-clone, prod requires real secrets; wired into `server.ts`. -- [ ] Run the architecture _and_ the stability reviewer agents on the _whole_ project, not just the latest changes, and address their issues. +- [x] Run the architecture _and_ the stability reviewer agents on the _whole_ project, not just the latest changes, and address their issues. → Both: no bugs/security issues. Addressed: wired `buildContext` into `app.ts`; graceful SIGTERM/SIGINT shutdown; EJS template caching in prod. Deferred `core/`/`shell/` split (premature for an 8-file scaffold; revisit at §2/§4). - [ ] Go over all comments in the code and the README and try to make it shorter and more information dense. Remove not strictly needed stuff. - [ ] Go over all tests and combine/unify ones that cover the same stuff or are very related and could be combined in a good way. Remove tests that aren't helping, we only want tests that are actually helpful to us.