Files
redefined-designs/backend/tests/unit/routesAreWrapped.test.ts
T
bermudalambandClaude Opus 5 1d7aba2d60 fix(backend): route every async handler through the error middleware (#59)
Express 4 does not forward a rejected promise from an async handler, so an unwrapped async route never responds at all — the request hangs until the client gives up, nothing reaches the error middleware, and monitoring sees an open connection rather than a 500. That silence is the shape of the 2026-08-17 incident, where an unhandled rejection left every item query hanging and the storefront rendered it as an empty shop. `asyncRoute` was written in response, but it was only applied to some routes: 30 handlers added afterwards were still bare, including register, login, the whole cart, and PayPal checkout.

Wraps all 30, plus two the issue's inventory missed. `attachCustomer` is a bare async middleware mounted globally in app.ts, so a rejection in its session lookup would hang every request in the application — including the 25 handlers that were already wrapped correctly, which meant the guarantee did not actually hold anywhere. The PayPal webhook registers on a second router named `webhookRouter`, so an audit grepping for `router.` walked straight past it.

Adds a unit test that scans the route sources and fails on any registration whose handler is not wrapped. A convention already half-forgotten once will be forgotten again, and enforcement is what the issue asked for; ESLint would be the better home for it but there is no ESLint in this repo yet (#60). The test walks parens rather than lines, so it also catches a handler whose `async` sits on its own line, and it matches any `*Router` name rather than just `router` — the two ways the existing bare handlers escaped notice. It is deleted along with `asyncRoute` if the project moves to Express 5, which forwards rejections natively.

No behaviour changes on the success path; the failure path turns a hung request into a logged 500.

Closes #59

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-08-19 13:12:25 -05:00

155 lines
5.3 KiB
TypeScript

import { readdirSync, readFileSync } from 'fs';
import { join } from 'path';
// Express 4 does not forward a rejected promise from an async handler, so an
// unwrapped async route never responds at all — the request hangs until the
// client gives up, nothing reaches the error middleware, and nothing is logged
// as a failure. That is the shape of the 2026-08-17 incident that put an empty
// storefront in front of customers.
//
// `asyncRoute` fixes it, but a convention only holds while everyone remembers
// it, and this one was already half-forgotten once: 30 handlers were added
// unwrapped after the wrapper existed. This test is the part that remembers.
//
// Express 5 forwards rejections natively. When the project upgrades, delete
// `asyncRoute` and delete this test with it.
const SRC = join(__dirname, '..', '..', 'src');
// `router.get(`, `app.use(`, and friends — where a handler gets registered.
// Any `*Router` name counts, not just `router`: cartCheckout.ts registers the
// PayPal webhook on a second router, and matching only the common name is how
// that one stayed unwrapped while everything around it was audited.
const REGISTRATION = /\b(?:app|\w*[Rr]outer)\.(?:get|post|put|patch|delete|all|use)\s*\(/g;
/**
* Returns the full text of the registration call starting at `open` (the index
* of its `(`), by counting parens to the matching close. Quotes and comments
* are skipped so a path like `'/items/:id'` or a `)` inside a string cannot
* throw the count off.
*/
function registrationAt(source: string, open: number): string {
let depth = 0;
for (let i = open; i < source.length; i++) {
const c = source[i];
if (c === "'" || c === '"' || c === '`') {
i = skipString(source, i);
continue;
}
if (c === '/' && source[i + 1] === '/') {
i = source.indexOf('\n', i);
if (i === -1) break;
continue;
}
if (c === '/' && source[i + 1] === '*') {
i = source.indexOf('*/', i) + 1;
continue;
}
if (c === '(') depth++;
if (c === ')') {
depth--;
if (depth === 0) return source.slice(open, i + 1);
}
}
return source.slice(open);
}
/** Returns the index of the closing quote of the string opening at `start`. */
function skipString(source: string, start: number): number {
const quote = source[start];
for (let i = start + 1; i < source.length; i++) {
if (source[i] === '\\') {
i++;
continue;
}
if (source[i] === quote) return i;
}
return source.length;
}
/**
* Every `async` inside a registration must sit directly behind `asyncRoute(`.
* Checking the token rather than the line catches a handler whose `async`
* lands on its own line, which a line-oriented grep would wave through.
*/
function unwrappedHandlers(source: string): string[] {
const offenders: string[] = [];
for (const match of source.matchAll(REGISTRATION)) {
const open = match.index! + match[0].length - 1;
const call = registrationAt(source, open);
for (const found of call.matchAll(/\basync\b/g)) {
const before = call.slice(0, found.index!).trimEnd();
if (!before.endsWith('asyncRoute(')) {
offenders.push(`line ${lineOf(source, match.index!)}: ${match[0].trim()}`);
}
}
}
return offenders;
}
function lineOf(source: string, index: number): number {
return source.slice(0, index).split('\n').length;
}
const routeFiles = readdirSync(join(SRC, 'routes'))
.filter((f) => f.endsWith('.ts'))
.map((f) => join('routes', f));
describe.each([...routeFiles, 'app.ts'])('%s', (relative) => {
it('wraps every async handler in asyncRoute', () => {
const source = readFileSync(join(SRC, relative), 'utf8');
expect(unwrappedHandlers(source)).toEqual([]);
});
});
describe('async middleware', () => {
// `attachCustomer` is mounted globally, so a rejection there hangs every
// request in the application — including the routes that are wrapped
// correctly. It is the one handler whose failure is not contained.
it('wraps attachCustomer where app.ts mounts it', () => {
const source = readFileSync(join(SRC, 'app.ts'), 'utf8');
expect(source).toContain('app.use(asyncRoute(attachCustomer))');
});
});
// Keeps the helper honest: if the paren-walking ever silently stops finding
// registrations, the tests above would pass by finding nothing at all.
describe('the guard itself', () => {
it('finds a bare async handler', () => {
const bad = `router.get('/', requireCustomer, async (req, res) => { res.json({}); });`;
expect(unwrappedHandlers(bad)).toHaveLength(1);
});
it('accepts a wrapped one', () => {
const good = `router.get('/', requireCustomer, asyncRoute(async (req, res) => { res.json({}); }));`;
expect(unwrappedHandlers(good)).toEqual([]);
});
it('sees an async handler that starts on a later line', () => {
const bad = `router.post(\n '/items/:id',\n uploadImages,\n async (req, res) => { res.json({}); }\n);`;
expect(unwrappedHandlers(bad)).toHaveLength(1);
});
it('is not fooled by a paren inside a route path', () => {
const bad = `router.get('/odd(path', async (req, res) => { res.json({}); });`;
expect(unwrappedHandlers(bad)).toHaveLength(1);
});
it('leaves a synchronous handler alone', () => {
const sync = `router.get('/', (req, res) => { res.json({}); });`;
expect(unwrappedHandlers(sync)).toEqual([]);
});
});