fix: run migrations on boot and stop failures rendering as empty (#23)
The storefront showed no inventory after deploying the categories/tags release. No data was lost: the code queried categories/item_tags/ items.category_id against a database where the migration had not been run, and that failure was invisible at every layer. Three changes, each addressing one layer: Migrations now run at container start, so deployed code cannot be ahead of the schema and the easily-forgotten manual `docker exec migrate.js up` step disappears. migrate.js waits for Postgres to accept connections first, since the NAS brings the DB container up slower than the app, and still exits non-zero so a bad migration stops the container rather than serving a half-migrated schema. Express 4 does not forward a rejected async handler, and no error middleware was mounted, so a failing query never responded at all. Async routes are now wrapped and an error middleware guarantees a 500. A hung request is indistinguishable from an empty result in the UI, which is how a schema mismatch came to read as "the store has no items". The storefront now separates "request failed" from "no items" and offers a retry. fetchItems/fetchFilterOptions throw on a non-OK response rather than returning the parsed error body, which would have been set as the item list and crashed the grid on .map. Also restores the project-context update from 7fb5764, which was left out of PR #24 and ended up dangling. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
+46
-14
@@ -1,22 +1,54 @@
|
||||
const { runner } = require('node-pg-migrate');
|
||||
const { Client } = require('pg');
|
||||
const path = require('path');
|
||||
|
||||
const direction = process.argv[2] || 'up';
|
||||
|
||||
runner({
|
||||
databaseUrl: {
|
||||
host: process.env.PGHOST || 'localhost',
|
||||
port: parseInt(process.env.PGPORT || '5432', 10),
|
||||
user: process.env.PGUSER,
|
||||
password: process.env.PGPASSWORD,
|
||||
database: process.env.PGDATABASE
|
||||
},
|
||||
dir: path.resolve(__dirname, 'migrations'),
|
||||
direction,
|
||||
migrationsTable: 'pgmigrations',
|
||||
count: direction === 'down' ? 1 : Infinity,
|
||||
log: (msg) => console.log(msg)
|
||||
})
|
||||
const dbConfig = {
|
||||
host: process.env.PGHOST || 'localhost',
|
||||
port: parseInt(process.env.PGPORT || '5432', 10),
|
||||
user: process.env.PGUSER,
|
||||
password: process.env.PGPASSWORD,
|
||||
database: process.env.PGDATABASE
|
||||
};
|
||||
|
||||
// This now runs at container start, ahead of the app, so it can come up before
|
||||
// Postgres is accepting connections — on the NAS the database container is
|
||||
// routinely slower to be ready than the app container. Without a wait the
|
||||
// migration would fail, take the app down with it, and look like a broken
|
||||
// deploy rather than a startup race.
|
||||
const WAIT_ATTEMPTS = 30;
|
||||
const WAIT_INTERVAL_MS = 2000;
|
||||
|
||||
async function waitForDb() {
|
||||
for (let attempt = 1; attempt <= WAIT_ATTEMPTS; attempt++) {
|
||||
const client = new Client(dbConfig);
|
||||
try {
|
||||
await client.connect();
|
||||
await client.end();
|
||||
return;
|
||||
} catch (err) {
|
||||
await client.end().catch(() => {});
|
||||
if (attempt === WAIT_ATTEMPTS) {
|
||||
throw new Error(`Database unreachable after ${WAIT_ATTEMPTS} attempts: ${err.message}`);
|
||||
}
|
||||
console.log(`Waiting for database (attempt ${attempt}/${WAIT_ATTEMPTS})...`);
|
||||
await new Promise((resolve) => setTimeout(resolve, WAIT_INTERVAL_MS));
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
waitForDb()
|
||||
.then(() =>
|
||||
runner({
|
||||
databaseUrl: dbConfig,
|
||||
dir: path.resolve(__dirname, 'migrations'),
|
||||
direction,
|
||||
migrationsTable: 'pgmigrations',
|
||||
count: direction === 'down' ? 1 : Infinity,
|
||||
log: (msg) => console.log(msg)
|
||||
})
|
||||
)
|
||||
.then((applied) => {
|
||||
console.log(`Migration complete — ${applied.length} migration(s) ${direction === 'down' ? 'reverted' : 'applied'}.`);
|
||||
process.exit(0);
|
||||
|
||||
+17
-1
@@ -1,4 +1,4 @@
|
||||
import express from 'express';
|
||||
import express, { Request, Response, NextFunction } from 'express';
|
||||
import cookieParser from 'cookie-parser';
|
||||
import path from 'path';
|
||||
import itemsRouter from './routes/items';
|
||||
@@ -58,4 +58,20 @@ if (process.env.NODE_ENV !== 'test') {
|
||||
});
|
||||
}
|
||||
|
||||
// Mounted last, so it sees errors from every route above. Without it, a route
|
||||
// that hands an error to next() falls through to Express's default handler,
|
||||
// and — worse — an async route that rejects never responds at all, leaving the
|
||||
// client hanging. A hung request is indistinguishable from an empty catalogue
|
||||
// in the UI, which is exactly how a schema mismatch once read as "the store
|
||||
// has no items". Always answer.
|
||||
//
|
||||
// The four-argument signature is what marks this as error middleware; `next`
|
||||
// is unused but must stay for Express to recognise it.
|
||||
// eslint-disable-next-line @typescript-eslint/no-unused-vars
|
||||
app.use((err: Error, _req: Request, res: Response, _next: NextFunction) => {
|
||||
console.error(err);
|
||||
if (res.headersSent) return;
|
||||
res.status(500).json({ error: 'internal error' });
|
||||
});
|
||||
|
||||
export default app;
|
||||
|
||||
@@ -0,0 +1,20 @@
|
||||
import { Request, Response, NextFunction, RequestHandler } from 'express';
|
||||
|
||||
// Express 4 does not forward a rejected promise from an async handler to the
|
||||
// error middleware. An async route that throws therefore never responds at all
|
||||
// — the request hangs until the client gives up, which reads to a user as "the
|
||||
// page is empty" rather than "the server failed". Wrapping the handler routes
|
||||
// the rejection into next(), so the error middleware can answer with a 500.
|
||||
//
|
||||
// Express 5 does this natively; drop this helper if the project ever upgrades.
|
||||
export function asyncRoute(
|
||||
handler: (req: Request, res: Response, next: NextFunction) => unknown
|
||||
): RequestHandler {
|
||||
return (req, res, next) => {
|
||||
// Promise.resolve also captures a synchronous throw, so both failure modes
|
||||
// reach the same place.
|
||||
return Promise.resolve()
|
||||
.then(() => handler(req, res, next))
|
||||
.catch(next);
|
||||
};
|
||||
}
|
||||
@@ -1,5 +1,6 @@
|
||||
import { Router, Request, Response } from 'express';
|
||||
import { pool } from '../db';
|
||||
import { asyncRoute } from '../asyncRoute';
|
||||
|
||||
const router = Router();
|
||||
|
||||
@@ -36,7 +37,7 @@ async function parentExists(id: number): Promise<boolean> {
|
||||
return rows.length > 0;
|
||||
}
|
||||
|
||||
router.get('/', async (_req: Request, res: Response) => {
|
||||
router.get('/', asyncRoute(async (_req: Request, res: Response) => {
|
||||
const { rows } = await pool.query(
|
||||
`SELECT c.id, c.name, c.parent_id, c.sort_order,
|
||||
(SELECT COUNT(*)::int FROM items i WHERE i.category_id = c.id) AS item_count
|
||||
@@ -44,9 +45,9 @@ router.get('/', async (_req: Request, res: Response) => {
|
||||
ORDER BY c.sort_order, lower(c.name)`
|
||||
);
|
||||
res.json(rows);
|
||||
});
|
||||
}));
|
||||
|
||||
router.post('/', async (req: Request, res: Response) => {
|
||||
router.post('/', asyncRoute(async (req: Request, res: Response) => {
|
||||
const name = readName(req.body.name);
|
||||
if (!name) {
|
||||
return res.status(400).json({ error: 'name is required' });
|
||||
@@ -76,9 +77,9 @@ router.post('/', async (req: Request, res: Response) => {
|
||||
}
|
||||
throw err;
|
||||
}
|
||||
});
|
||||
}));
|
||||
|
||||
router.put('/:id', async (req: Request, res: Response) => {
|
||||
router.put('/:id', asyncRoute(async (req: Request, res: Response) => {
|
||||
const id = Number(req.params.id);
|
||||
const existing = await pool.query(`SELECT id, name, parent_id, sort_order FROM categories WHERE id = $1`, [id]);
|
||||
if (!existing.rows.length) {
|
||||
@@ -134,9 +135,9 @@ router.put('/:id', async (req: Request, res: Response) => {
|
||||
}
|
||||
throw err;
|
||||
}
|
||||
});
|
||||
}));
|
||||
|
||||
router.delete('/:id', async (req: Request, res: Response) => {
|
||||
router.delete('/:id', asyncRoute(async (req: Request, res: Response) => {
|
||||
const id = Number(req.params.id);
|
||||
const { rows: subtree } = await pool.query(`${SUBTREE_CTE} SELECT id FROM subtree`, [id]);
|
||||
if (!subtree.length) {
|
||||
@@ -154,6 +155,6 @@ router.delete('/:id', async (req: Request, res: Response) => {
|
||||
await pool.query(`DELETE FROM categories WHERE id = $1`, [id]);
|
||||
|
||||
res.json({ deleted_categories: ids.length, uncategorized_items: affected[0].n });
|
||||
});
|
||||
}));
|
||||
|
||||
export default router;
|
||||
|
||||
@@ -1,5 +1,6 @@
|
||||
import { Router, Request, Response } from 'express';
|
||||
import { pool } from '../db';
|
||||
import { asyncRoute } from '../asyncRoute';
|
||||
import { TAG_COLORS, tagColorFor } from '../utils';
|
||||
|
||||
const router = Router();
|
||||
@@ -20,7 +21,7 @@ function readColor(value: unknown): string | null | undefined {
|
||||
return value;
|
||||
}
|
||||
|
||||
router.get('/', async (_req: Request, res: Response) => {
|
||||
router.get('/', asyncRoute(async (_req: Request, res: Response) => {
|
||||
const { rows } = await pool.query(
|
||||
`SELECT t.id, t.name, t.color,
|
||||
(SELECT COUNT(*)::int FROM item_tags it WHERE it.tag_id = t.id) AS item_count
|
||||
@@ -28,9 +29,9 @@ router.get('/', async (_req: Request, res: Response) => {
|
||||
ORDER BY lower(t.name)`
|
||||
);
|
||||
res.json(rows);
|
||||
});
|
||||
}));
|
||||
|
||||
router.post('/', async (req: Request, res: Response) => {
|
||||
router.post('/', asyncRoute(async (req: Request, res: Response) => {
|
||||
const name = readName(req.body.name);
|
||||
if (!name) {
|
||||
return res.status(400).json({ error: 'name is required' });
|
||||
@@ -54,9 +55,9 @@ router.post('/', async (req: Request, res: Response) => {
|
||||
}
|
||||
throw err;
|
||||
}
|
||||
});
|
||||
}));
|
||||
|
||||
router.put('/:id', async (req: Request, res: Response) => {
|
||||
router.put('/:id', asyncRoute(async (req: Request, res: Response) => {
|
||||
const id = Number(req.params.id);
|
||||
const existing = await pool.query(`SELECT id, name, color FROM tags WHERE id = $1`, [id]);
|
||||
if (!existing.rows.length) {
|
||||
@@ -93,12 +94,12 @@ router.put('/:id', async (req: Request, res: Response) => {
|
||||
}
|
||||
throw err;
|
||||
}
|
||||
});
|
||||
}));
|
||||
|
||||
router.delete('/:id', async (req: Request, res: Response) => {
|
||||
router.delete('/:id', asyncRoute(async (req: Request, res: Response) => {
|
||||
// item_tags cascades; the items themselves are untouched.
|
||||
await pool.query(`DELETE FROM tags WHERE id = $1`, [req.params.id]);
|
||||
res.status(204).end();
|
||||
});
|
||||
}));
|
||||
|
||||
export default router;
|
||||
|
||||
@@ -1,12 +1,13 @@
|
||||
import { Router, Request, Response } from 'express';
|
||||
import { pool } from '../db';
|
||||
import { asyncRoute } from '../asyncRoute';
|
||||
|
||||
const router = Router();
|
||||
|
||||
// Everything the storefront's filter drawer needs, in one request: the whole
|
||||
// category tree (flat — the frontend nests it), every tag with its colour, and
|
||||
// the catalogue's price bounds for the slider.
|
||||
router.get('/', async (_req: Request, res: Response) => {
|
||||
router.get('/', asyncRoute(async (_req: Request, res: Response) => {
|
||||
const [categories, tags, price] = await Promise.all([
|
||||
pool.query(
|
||||
`SELECT id, name, parent_id, sort_order FROM categories ORDER BY sort_order, lower(name)`
|
||||
@@ -31,6 +32,6 @@ router.get('/', async (_req: Request, res: Response) => {
|
||||
tags: tags.rows,
|
||||
priceRange: price.rows[0]
|
||||
});
|
||||
});
|
||||
}));
|
||||
|
||||
export default router;
|
||||
|
||||
@@ -1,11 +1,12 @@
|
||||
import { Router, Request, Response } from 'express';
|
||||
import { pool } from '../db';
|
||||
import { asyncRoute } from '../asyncRoute';
|
||||
import { PUBLIC_ITEM_SELECT } from '../itemSelect';
|
||||
import { parseItemFilters, buildItemFilterSql, FilterError } from '../itemFilters';
|
||||
|
||||
const router = Router();
|
||||
|
||||
router.get('/', async (req: Request, res: Response) => {
|
||||
router.get('/', asyncRoute(async (req: Request, res: Response) => {
|
||||
let filters;
|
||||
try {
|
||||
filters = parseItemFilters(req.query as Record<string, unknown>);
|
||||
@@ -22,12 +23,12 @@ router.get('/', async (req: Request, res: Response) => {
|
||||
const where = clauses.length ? `WHERE ${clauses.join(' AND ')}` : '';
|
||||
const { rows } = await pool.query(`${PUBLIC_ITEM_SELECT} ${where} ORDER BY i.created_at DESC`, params);
|
||||
res.json(rows);
|
||||
});
|
||||
}));
|
||||
|
||||
router.get('/:id', async (req: Request, res: Response) => {
|
||||
router.get('/:id', asyncRoute(async (req: Request, res: Response) => {
|
||||
const { rows } = await pool.query(`${PUBLIC_ITEM_SELECT} WHERE i.id = $1`, [req.params.id]);
|
||||
if (!rows.length) return res.status(404).json({ error: 'not found' });
|
||||
res.json(rows[0]);
|
||||
});
|
||||
}));
|
||||
|
||||
export default router;
|
||||
|
||||
@@ -0,0 +1,34 @@
|
||||
import request from 'supertest';
|
||||
import app from '../../src/app';
|
||||
import { pool } from '../../src/db';
|
||||
import { resetDb, closeDb } from './setup/testDb';
|
||||
|
||||
beforeEach(async () => {
|
||||
await resetDb();
|
||||
});
|
||||
|
||||
afterAll(async () => {
|
||||
await pool.end();
|
||||
await closeDb();
|
||||
});
|
||||
|
||||
describe('unexpected route failures', () => {
|
||||
it('answers with 500 instead of leaving the request hanging', async () => {
|
||||
// A non-numeric id reaches Postgres as `WHERE i.id = 'not-a-number'`, which
|
||||
// raises invalid-input-syntax. Express 4 does not forward a rejected async
|
||||
// handler on its own, so without the asyncRoute wrapper plus the error
|
||||
// middleware this request never gets a response at all — and a hung request
|
||||
// renders as an empty storefront rather than a visible failure.
|
||||
const res = await request(app).get('/api/items/not-a-number');
|
||||
|
||||
expect(res.status).toBe(500);
|
||||
expect(res.body.error).toBe('internal error');
|
||||
});
|
||||
|
||||
it('does not leak the underlying database error to the client', async () => {
|
||||
const res = await request(app).get('/api/items/not-a-number');
|
||||
|
||||
expect(JSON.stringify(res.body)).not.toContain('syntax');
|
||||
expect(JSON.stringify(res.body)).not.toContain('items');
|
||||
});
|
||||
});
|
||||
@@ -0,0 +1,44 @@
|
||||
import { Request, Response, NextFunction } from 'express';
|
||||
import { asyncRoute } from '../../src/asyncRoute';
|
||||
|
||||
function fakeArgs() {
|
||||
const next = jest.fn() as unknown as NextFunction;
|
||||
return { req: {} as Request, res: {} as Response, next };
|
||||
}
|
||||
|
||||
describe('asyncRoute', () => {
|
||||
it('passes a rejected handler to next so Express can answer the request', async () => {
|
||||
const boom = new Error('relation "categories" does not exist');
|
||||
const { req, res, next } = fakeArgs();
|
||||
|
||||
await asyncRoute(async () => { throw boom; })(req, res, next);
|
||||
|
||||
expect(next).toHaveBeenCalledWith(boom);
|
||||
});
|
||||
|
||||
it('passes a synchronous throw to next as well', async () => {
|
||||
const boom = new Error('sync failure');
|
||||
const { req, res, next } = fakeArgs();
|
||||
|
||||
await asyncRoute(() => { throw boom; })(req, res, next);
|
||||
|
||||
expect(next).toHaveBeenCalledWith(boom);
|
||||
});
|
||||
|
||||
it('leaves a successful handler alone', async () => {
|
||||
const { req, res, next } = fakeArgs();
|
||||
|
||||
await asyncRoute(async () => 'fine')(req, res, next);
|
||||
|
||||
expect(next).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('forwards the same arguments it was given', async () => {
|
||||
const handler = jest.fn(async () => undefined);
|
||||
const { req, res, next } = fakeArgs();
|
||||
|
||||
await asyncRoute(handler)(req, res, next);
|
||||
|
||||
expect(handler).toHaveBeenCalledWith(req, res, next);
|
||||
});
|
||||
});
|
||||
Reference in New Issue
Block a user