From 1d7aba2d60164e48e7acbe09fe55a5fca3063d5d Mon Sep 17 00:00:00 2001 From: Thom Lamb Date: Wed, 19 Aug 2026 13:12:25 -0500 Subject: [PATCH 1/2] fix(backend): route every async handler through the error middleware (#59) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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) --- backend/src/app.ts | 6 +- backend/src/routes/admin.ts | 20 +-- backend/src/routes/adminSettings.ts | 9 +- backend/src/routes/cart.ts | 13 +- backend/src/routes/cartCheckout.ts | 17 ++- backend/src/routes/customers.ts | 44 +++--- backend/src/routes/public.ts | 5 +- backend/src/routes/shippingAddresses.ts | 21 +-- backend/tests/unit/routesAreWrapped.test.ts | 154 ++++++++++++++++++++ 9 files changed, 226 insertions(+), 63 deletions(-) create mode 100644 backend/tests/unit/routesAreWrapped.test.ts diff --git a/backend/src/app.ts b/backend/src/app.ts index 86563f1..9172962 100755 --- a/backend/src/app.ts +++ b/backend/src/app.ts @@ -14,6 +14,7 @@ import publicRouter from './routes/public'; import cartRouter from './routes/cart'; import shippingAddressesRouter from './routes/shippingAddresses'; import { attachCustomer } from './middleware/customerAuth'; +import { asyncRoute } from './asyncRoute'; const app = express(); // Express advertises itself in X-Powered-By by default, which hands an @@ -24,7 +25,10 @@ app.set('trust proxy', 1); app.use('/webhooks/paypal', express.json(), cartCheckoutWebhookRouter); app.use(express.json()); app.use(cookieParser()); -app.use(attachCustomer); +// Wrapped like any route: attachCustomer awaits a session lookup, and mounted +// globally an unforwarded rejection here would hang every request in the app — +// including the routes that wrap their own handlers correctly. +app.use(asyncRoute(attachCustomer)); app.use('/uploads', express.static(process.env.UPLOADS_DIR || '/app/uploads')); app.get('/api/config', (_req, res) => { diff --git a/backend/src/routes/admin.ts b/backend/src/routes/admin.ts index d83fe32..fda676a 100755 --- a/backend/src/routes/admin.ts +++ b/backend/src/routes/admin.ts @@ -140,7 +140,7 @@ router.get('/items', asyncRoute(async (req: Request, res: Response) => { res.json(rows); })); -router.post('/items', uploadImages, async (req: Request, res: Response) => { +router.post('/items', uploadImages, asyncRoute(async (req: Request, res: Response) => { const { name, description, price } = req.body; const categoryId = readCategoryId(req.body.category_id); @@ -180,9 +180,9 @@ router.post('/items', uploadImages, async (req: Request, res: Response) => { } finally { client.release(); } -}); +})); -router.put('/items/:id', uploadImages, async (req: Request, res: Response) => { +router.put('/items/:id', uploadImages, asyncRoute(async (req: Request, res: Response) => { const { name, description, price } = req.body; const categoryId = readCategoryId(req.body.category_id); @@ -233,7 +233,7 @@ router.put('/items/:id', uploadImages, async (req: Request, res: Response) => { } finally { client.release(); } -}); +})); router.delete('/items/:id', asyncRoute(async (req: Request, res: Response) => { const itemId = Number(req.params.id); @@ -251,12 +251,12 @@ router.delete('/items/:id', asyncRoute(async (req: Request, res: Response) => { res.status(204).end(); })); -router.delete('/items/:id/images/:imageId', async (req: Request, res: Response) => { +router.delete('/items/:id/images/:imageId', asyncRoute(async (req: Request, res: Response) => { await pool.query(`DELETE FROM item_images WHERE id = $1 AND item_id = $2`, [req.params.imageId, req.params.id]); res.status(204).end(); -}); +})); -router.post('/items/:id/mark-sold', async (req: Request, res: Response) => { +router.post('/items/:id/mark-sold', asyncRoute(async (req: Request, res: Response) => { const { rows } = await pool.query( `UPDATE items SET status='sold', sold_at=now() WHERE id=$1 RETURNING *`, [req.params.id] @@ -265,15 +265,15 @@ router.post('/items/:id/mark-sold', async (req: Request, res: Response) => { // customer, so everyone watching it hears about it. await notifyFavoritersOfSale([Number(req.params.id)], null); res.json(rows[0]); -}); +})); -router.post('/items/:id/mark-available', async (req: Request, res: Response) => { +router.post('/items/:id/mark-available', asyncRoute(async (req: Request, res: Response) => { const { rows } = await pool.query( `UPDATE items SET status='available', sold_at=NULL, reserved_until=NULL, paypal_order_id=NULL WHERE id=$1 RETURNING *`, [req.params.id] ); res.json(rows[0]); -}); +})); export default router; diff --git a/backend/src/routes/adminSettings.ts b/backend/src/routes/adminSettings.ts index def7528..02a8e9c 100644 --- a/backend/src/routes/adminSettings.ts +++ b/backend/src/routes/adminSettings.ts @@ -1,18 +1,19 @@ import { Router, Request, Response } from 'express'; import { pool } from '../db'; +import { asyncRoute } from '../asyncRoute'; const router = Router(); -router.get('/', async (_req: Request, res: Response) => { +router.get('/', asyncRoute(async (_req: Request, res: Response) => { const { rows } = await pool.query(`SELECT key, value FROM admin_settings`); const map: Record = {}; for (const r of rows) map[r.key] = r.value; res.json({ cartExpiryHours: parseFloat(map.cart_expiry_hours || '24') }); -}); +})); -router.put('/', async (req: Request, res: Response) => { +router.put('/', asyncRoute(async (req: Request, res: Response) => { const { cartExpiryHours } = req.body; const hours = parseFloat(cartExpiryHours); if (Number.isNaN(hours) || hours <= 0) { @@ -24,6 +25,6 @@ router.put('/', async (req: Request, res: Response) => { [String(hours)] ); res.json({ cartExpiryHours: hours }); -}); +})); export default router; diff --git a/backend/src/routes/cart.ts b/backend/src/routes/cart.ts index 6e36362..b40578a 100644 --- a/backend/src/routes/cart.ts +++ b/backend/src/routes/cart.ts @@ -1,5 +1,6 @@ import { Router, Request, Response } from 'express'; import { pool } from '../db'; +import { asyncRoute } from '../asyncRoute'; import { requireCustomer } from '../middleware/customerAuth'; const router = Router(); @@ -26,14 +27,14 @@ const CART_ITEM_SELECT = ` ORDER BY ci.added_at DESC `; -router.get('/', requireCustomer, async (req: Request, res: Response) => { +router.get('/', requireCustomer, asyncRoute(async (req: Request, res: Response) => { const { rows: cartRows } = await pool.query(`SELECT id FROM carts WHERE customer_id = $1`, [req.customerId]); if (!cartRows.length) return res.json({ items: [] }); const { rows: items } = await pool.query(CART_ITEM_SELECT, [cartRows[0].id]); res.json({ items }); -}); +})); -router.post('/items/:itemId', requireCustomer, async (req: Request, res: Response) => { +router.post('/items/:itemId', requireCustomer, asyncRoute(async (req: Request, res: Response) => { const itemId = req.params.itemId; const client = await pool.connect(); try { @@ -75,9 +76,9 @@ router.post('/items/:itemId', requireCustomer, async (req: Request, res: Respons } finally { client.release(); } -}); +})); -router.delete('/items/:itemId', requireCustomer, async (req: Request, res: Response) => { +router.delete('/items/:itemId', requireCustomer, asyncRoute(async (req: Request, res: Response) => { const itemId = req.params.itemId; const client = await pool.connect(); try { @@ -103,6 +104,6 @@ router.delete('/items/:itemId', requireCustomer, async (req: Request, res: Respo } finally { client.release(); } -}); +})); export default router; diff --git a/backend/src/routes/cartCheckout.ts b/backend/src/routes/cartCheckout.ts index fdd1c51..5e2e3be 100644 --- a/backend/src/routes/cartCheckout.ts +++ b/backend/src/routes/cartCheckout.ts @@ -1,5 +1,6 @@ import { Router, Request, Response } from 'express'; import { pool } from '../db'; +import { asyncRoute } from '../asyncRoute'; import { requireCustomer } from '../middleware/customerAuth'; import { notifyFavoritersOfSale } from '../favoriteAlerts'; @@ -93,7 +94,7 @@ async function openCheckout( return { ok: true, checkoutId, cart }; } -router.post('/paypal/create', requireCustomer, async (req: Request, res: Response) => { +router.post('/paypal/create', requireCustomer, asyncRoute(async (req: Request, res: Response) => { const { shippingAddressId } = req.body; if (!shippingAddressId) return res.status(400).json({ error: 'shippingAddressId is required' }); @@ -139,7 +140,7 @@ router.post('/paypal/create', requireCustomer, async (req: Request, res: Respons } finally { client.release(); } -}); +})); // Returns the sold item ids and the buyer, so the caller can notify favoriters // *after* COMMIT. Sending inside the transaction would email people about a @@ -169,7 +170,7 @@ async function completeCheckout(client: any, checkoutId: number, processor: stri }; } -router.post('/paypal/capture', requireCustomer, async (req: Request, res: Response) => { +router.post('/paypal/capture', requireCustomer, asyncRoute(async (req: Request, res: Response) => { const { orderID } = req.body; const client = await pool.connect(); try { @@ -201,9 +202,9 @@ router.post('/paypal/capture', requireCustomer, async (req: Request, res: Respon } finally { client.release(); } -}); +})); -router.post('/demo/purchase', requireCustomer, async (req: Request, res: Response) => { +router.post('/demo/purchase', requireCustomer, asyncRoute(async (req: Request, res: Response) => { if (process.env.DEMO_MODE === 'false') return res.status(403).json({ error: 'demo mode disabled' }); const { shippingAddressId } = req.body; if (!shippingAddressId) return res.status(400).json({ error: 'shippingAddressId is required' }); @@ -225,9 +226,9 @@ router.post('/demo/purchase', requireCustomer, async (req: Request, res: Respons } finally { client.release(); } -}); +})); -webhookRouter.post('/', async (req: Request, res: Response) => { +webhookRouter.post('/', asyncRoute(async (req: Request, res: Response) => { try { const token = await getAccessToken(); const verifyResp = await fetch(`${PAYPAL_BASE}/v1/notifications/verify-webhook-signature`, { @@ -272,6 +273,6 @@ webhookRouter.post('/', async (req: Request, res: Response) => { console.error('webhook error', err); res.status(500).end(); } -}); +})); export { router, webhookRouter }; diff --git a/backend/src/routes/customers.ts b/backend/src/routes/customers.ts index 4b78366..20a06fc 100755 --- a/backend/src/routes/customers.ts +++ b/backend/src/routes/customers.ts @@ -44,7 +44,7 @@ function publicCustomer(c: any) { }; } -router.post('/register', async (req: Request, res: Response) => { +router.post('/register', asyncRoute(async (req: Request, res: Response) => { const { email, password, name, marketingConsent } = req.body; if (!email || !isValidEmail(String(email)) || !password || String(password).length < 8) { return res.status(400).json({ error: 'valid email and password (min 8 chars) required' }); @@ -83,9 +83,9 @@ router.post('/register', async (req: Request, res: Response) => { const sessionToken = await createSession(customer.id); setSessionCookie(res, sessionToken); res.json(publicCustomer(customer)); -}); +})); -router.post('/verify-email', async (req: Request, res: Response) => { +router.post('/verify-email', asyncRoute(async (req: Request, res: Response) => { const { token } = req.body; const { rows } = await pool.query( `SELECT * FROM customer_tokens WHERE token = $1 AND kind = 'verify_email' AND expires_at > now()`, @@ -95,7 +95,7 @@ router.post('/verify-email', async (req: Request, res: Response) => { await pool.query(`UPDATE customers SET email_verified = true WHERE id = $1`, [rows[0].customer_id]); await pool.query(`DELETE FROM customer_tokens WHERE token = $1`, [token]); res.json({ status: 'verified' }); -}); +})); const RESET_TOKEN_TTL_MS = 60 * 60 * 1000; @@ -196,7 +196,7 @@ router.post('/reset-password', asyncRoute(async (req: Request, res: Response) => res.json(publicCustomer(fresh[0])); })); -router.post('/login', async (req: Request, res: Response) => { +router.post('/login', asyncRoute(async (req: Request, res: Response) => { const { email, password } = req.body; const { rows } = await pool.query(`SELECT * FROM customers WHERE email = $1`, [String(email || '').toLowerCase().trim()]); const customer = rows[0]; @@ -211,14 +211,14 @@ router.post('/login', async (req: Request, res: Response) => { const sessionToken = await createSession(customer.id); setSessionCookie(res, sessionToken); res.json(publicCustomer(customer)); -}); +})); -router.post('/logout', async (req: Request, res: Response) => { +router.post('/logout', asyncRoute(async (req: Request, res: Response) => { const token = req.cookies?.rd_session; if (token) await pool.query(`DELETE FROM customer_sessions WHERE token = $1`, [token]); res.clearCookie('rd_session'); res.status(204).end(); -}); +})); router.get('/me/favorites', requireCustomer, asyncRoute(async (req: Request, res: Response) => { const { rows } = await pool.query( @@ -265,22 +265,22 @@ router.put('/me/favorite-alerts', requireCustomer, asyncRoute(async (req: Reques res.json(publicCustomer(rows[0])); })); -router.get('/me', requireCustomer, async (req: Request, res: Response) => { +router.get('/me', requireCustomer, asyncRoute(async (req: Request, res: Response) => { const { rows } = await pool.query(`SELECT * FROM customers WHERE id = $1`, [req.customerId]); if (!rows.length) return res.status(404).json({ error: 'not found' }); res.json(publicCustomer(rows[0])); -}); +})); -router.put('/me', requireCustomer, async (req: Request, res: Response) => { +router.put('/me', requireCustomer, asyncRoute(async (req: Request, res: Response) => { const { name } = req.body; const { rows } = await pool.query( `UPDATE customers SET name = $1 WHERE id = $2 RETURNING *`, [name || null, req.customerId] ); res.json(publicCustomer(rows[0])); -}); +})); -router.post('/change-password', requireCustomer, async (req: Request, res: Response) => { +router.post('/change-password', requireCustomer, asyncRoute(async (req: Request, res: Response) => { const { currentPassword, newPassword } = req.body; if (!newPassword || String(newPassword).length < 8) { return res.status(400).json({ error: 'new password must be at least 8 characters' }); @@ -293,18 +293,18 @@ router.post('/change-password', requireCustomer, async (req: Request, res: Respo const newHash = await bcrypt.hash(newPassword, 12); await pool.query(`UPDATE customers SET password_hash = $1 WHERE id = $2`, [newHash, req.customerId]); res.status(204).end(); -}); +})); -router.post('/me/consent', requireCustomer, async (req: Request, res: Response) => { +router.post('/me/consent', requireCustomer, asyncRoute(async (req: Request, res: Response) => { const consent = !!req.body.marketingConsent; await pool.query( `UPDATE customers SET marketing_consent = $1, marketing_consent_at = now(), marketing_consent_text = $2 WHERE id = $3`, [consent, consent ? MARKETING_CONSENT_TEXT : 'Withdrew consent via account settings', req.customerId] ); res.status(204).end(); -}); +})); -router.get('/me/orders', requireCustomer, async (req: Request, res: Response) => { +router.get('/me/orders', requireCustomer, asyncRoute(async (req: Request, res: Response) => { const { rows } = await pool.query( `SELECT o.id, o.processor, o.amount_cents, o.status, o.created_at, i.name AS item_name FROM orders o JOIN items i ON i.id = o.item_id @@ -312,9 +312,9 @@ router.get('/me/orders', requireCustomer, async (req: Request, res: Response) => [req.customerId] ); res.json(rows); -}); +})); -router.get('/me/export', requireCustomer, async (req: Request, res: Response) => { +router.get('/me/export', requireCustomer, asyncRoute(async (req: Request, res: Response) => { const { rows: customerRows } = await pool.query(`SELECT * FROM customers WHERE id = $1`, [req.customerId]); const { rows: orderRows } = await pool.query(`SELECT * FROM orders WHERE customer_id = $1`, [req.customerId]); res.setHeader('Content-Disposition', 'attachment; filename="my-data.json"'); @@ -323,13 +323,13 @@ router.get('/me/export', requireCustomer, async (req: Request, res: Response) => orders: orderRows, exported_at: new Date().toISOString() }); -}); +})); -router.delete('/me', requireCustomer, async (req: Request, res: Response) => { +router.delete('/me', requireCustomer, asyncRoute(async (req: Request, res: Response) => { await pool.query(`UPDATE orders SET customer_id = NULL WHERE customer_id = $1`, [req.customerId]); await pool.query(`DELETE FROM customers WHERE id = $1`, [req.customerId]); res.clearCookie('rd_session'); res.status(204).end(); -}); +})); export default router; diff --git a/backend/src/routes/public.ts b/backend/src/routes/public.ts index 51d9d4b..6cc1c79 100755 --- a/backend/src/routes/public.ts +++ b/backend/src/routes/public.ts @@ -1,9 +1,10 @@ import { Router, Request, Response } from 'express'; import { pool } from '../db'; +import { asyncRoute } from '../asyncRoute'; const router = Router(); -router.get('/unsubscribe', async (req: Request, res: Response) => { +router.get('/unsubscribe', asyncRoute(async (req: Request, res: Response) => { const token = req.query.token as string; const { rows } = await pool.query(`SELECT id FROM customers WHERE unsubscribe_token = $1`, [token]); if (!rows.length) { @@ -16,6 +17,6 @@ router.get('/unsubscribe', async (req: Request, res: Response) => { [rows[0].id] ); res.send('

You\'ve been unsubscribed.

You will no longer receive marketing emails from Redefined Designs.

'); -}); +})); export default router; diff --git a/backend/src/routes/shippingAddresses.ts b/backend/src/routes/shippingAddresses.ts index 3cc09c5..571bfcc 100644 --- a/backend/src/routes/shippingAddresses.ts +++ b/backend/src/routes/shippingAddresses.ts @@ -1,19 +1,20 @@ import { Router, Request, Response } from 'express'; import { pool } from '../db'; +import { asyncRoute } from '../asyncRoute'; import { requireCustomer } from '../middleware/customerAuth'; import { validateAddress, uspsConfigured, UspsValidationResult } from '../usps'; const router = Router(); -router.get('/', requireCustomer, async (req: Request, res: Response) => { +router.get('/', requireCustomer, asyncRoute(async (req: Request, res: Response) => { const { rows } = await pool.query( `SELECT * FROM shipping_addresses WHERE customer_id = $1 ORDER BY is_default DESC, created_at DESC`, [req.customerId] ); res.json(rows); -}); +})); -router.post('/', requireCustomer, async (req: Request, res: Response) => { +router.post('/', requireCustomer, asyncRoute(async (req: Request, res: Response) => { const { fullName, addressLine1, addressLine2, city, state, postalCode, country, isDefault } = req.body; if (!fullName || !addressLine1 || !city || !state || !postalCode) { return res.status(400).json({ error: 'fullName, addressLine1, city, state, and postalCode are required' }); @@ -48,9 +49,9 @@ if ((country || 'US') === 'US') { } finally { client.release(); } -}); +})); -router.put('/:id', requireCustomer, async (req: Request, res: Response) => { +router.put('/:id', requireCustomer, asyncRoute(async (req: Request, res: Response) => { const { fullName, addressLine1, addressLine2, city, state, postalCode, country, isDefault } = req.body; const client = await pool.connect(); try { @@ -75,14 +76,14 @@ router.put('/:id', requireCustomer, async (req: Request, res: Response) => { } finally { client.release(); } -}); +})); -router.delete('/:id', requireCustomer, async (req: Request, res: Response) => { +router.delete('/:id', requireCustomer, asyncRoute(async (req: Request, res: Response) => { await pool.query(`DELETE FROM shipping_addresses WHERE id = $1 AND customer_id = $2`, [req.params.id, req.customerId]); res.status(204).end(); -}); +})); -router.post('/:id/set-default', requireCustomer, async (req: Request, res: Response) => { +router.post('/:id/set-default', requireCustomer, asyncRoute(async (req: Request, res: Response) => { const client = await pool.connect(); try { await client.query('BEGIN'); @@ -99,6 +100,6 @@ router.post('/:id/set-default', requireCustomer, async (req: Request, res: Respo } finally { client.release(); } -}); +})); export default router; diff --git a/backend/tests/unit/routesAreWrapped.test.ts b/backend/tests/unit/routesAreWrapped.test.ts new file mode 100644 index 0000000..734b216 --- /dev/null +++ b/backend/tests/unit/routesAreWrapped.test.ts @@ -0,0 +1,154 @@ +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([]); + }); +}); From e58c8b400904d829c9edd648f7845a740aec5481 Mon Sep 17 00:00:00 2001 From: Thom Lamb Date: Wed, 19 Aug 2026 13:13:01 -0500 Subject: [PATCH 2/2] docs: the asyncRoute guarantee now holds everywhere, and is enforced (#59) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The note said the older route files were still unwrapped, which stopped being true with #59. Records where the guarantee now reaches — including the second `webhookRouter` and the globally-mounted `attachCustomer`, both of which an audit grepping for `router.` misses — and points at the test that enforces it, so the next person adding a route learns it from a failing build rather than from this file. --- .claude/project-context.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.claude/project-context.md b/.claude/project-context.md index 6e43caa..c9dd8f8 100644 --- a/.claude/project-context.md +++ b/.claude/project-context.md @@ -75,7 +75,7 @@ A storefront for one-of-a-kind items (quantity 1 per item — once sold, it's go 8. **Categories are manual metadata, deliberately not a rule engine.** Issue #23's wording ("rules that dictate how the app automatically organizes items") was explicitly resolved with the author to mean an admin-built tree with a per-item assignment — there is no predicate evaluation anywhere, and adding one would be a new feature, not a completion of this one. Categories are single-valued per item on purpose, to stay distinct from multi-valued tags. **Tag filtering is AND** ("must have all"), not OR. 9. **Item filtering happens in SQL, and malformed filter params return `400`** rather than being ignored — a broken filter link should show itself instead of quietly returning the whole catalogue. Parsing/SQL-building live in `backend/src/itemFilters.ts`, apart from the route so they're unit-testable with no database. Category filtering matches a node *and all descendants* via a recursive CTE; a materialized path column was rejected because reparenting would then have to rewrite every descendant's path, which is a standing drift risk. 10. **Migrations run automatically at container start**, via `CMD ["sh", "-c", "node migrate.js up && node dist/server.js"]` in the `Dockerfile`. Deployed code can therefore never be ahead of the schema. This replaced a separate manual `docker exec ... node migrate.js up` step that was easy to forget — and forgetting it took the storefront down (see the incident note below). `migrate.js` waits for Postgres to accept connections before running (the NAS routinely brings the DB container up slower than the app), and exits non-zero on failure, so `&&` stops a bad migration from serving against a half-migrated schema. -11. **Every async route is wrapped in `asyncRoute()` and the app mounts error middleware.** Express 4 does *not* forward a rejected promise from an async handler, and with no error middleware such a request **never responds at all** — it hangs until the client gives up. A hung request is indistinguishable from an empty result in the UI. New async routes must use the wrapper (`backend/src/asyncRoute.ts`); it becomes unnecessary only if the project moves to Express 5. Note the older route files predate this and are still unwrapped. +11. **Every async route is wrapped in `asyncRoute()` and the app mounts error middleware.** Express 4 does *not* forward a rejected promise from an async handler, and with no error middleware such a request **never responds at all** — it hangs until the client gives up. A hung request is indistinguishable from an empty result in the UI. New async routes must use the wrapper (`backend/src/asyncRoute.ts`); it becomes unnecessary only if the project moves to Express 5. As of #59 this holds everywhere — every route file, the second `webhookRouter` in `cartCheckout.ts`, and the globally-mounted `attachCustomer` middleware — and `backend/tests/unit/routesAreWrapped.test.ts` fails the build if a new handler is added bare, so it is enforced rather than remembered. Delete that test along with `asyncRoute` on any Express 5 upgrade. 12. **Item SELECTs aggregate with scalar subqueries, never `LEFT JOIN` + `GROUP BY`** — see `backend/src/itemSelect.ts`, the single source for both the public and admin shapes. Joining two one-to-many relations in one query multiplies their rows together: with the old shape, an item with 2 images and 3 tags repeated every image three times. This bit once already when tags were added. If a third one-to-many relation is ever attached to items, extend `itemSelect.ts` the same way rather than adding a join. ## Frontend gotchas