Merge pull request 'Feature/59 wrap async routes' (#66) from feature/59-wrap-async-routes into main
Reviewed-on: #66
This commit was merged in pull request #66.
This commit is contained in:
@@ -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
|
||||
|
||||
+5
-1
@@ -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) => {
|
||||
|
||||
+10
-10
@@ -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;
|
||||
|
||||
@@ -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<string, string> = {};
|
||||
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;
|
||||
|
||||
@@ -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;
|
||||
|
||||
@@ -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 };
|
||||
|
||||
@@ -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;
|
||||
|
||||
@@ -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('<html><body><h2>You\'ve been unsubscribed.</h2><p>You will no longer receive marketing emails from Redefined Designs.</p></body></html>');
|
||||
});
|
||||
}));
|
||||
|
||||
export default router;
|
||||
|
||||
@@ -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;
|
||||
|
||||
@@ -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([]);
|
||||
});
|
||||
});
|
||||
Reference in New Issue
Block a user