Stop putting business logic in Express middleware
Middleware that mutates the request object reads like dependency injection and behaves like a global variable. Here is the refactor that finally stuck.
Every Express codebase I have inherited has the same layer of middleware that loads things and hangs them off req. It starts reasonably:
app.use(async (req, res, next) => {
req.user = await users.findById(req.session.userId);
next();
});
And ends up here:
router.post(
'/invoices',
requireAuth,
loadWorkspace,
checkSeatLimit,
attachBillingProfile,
normaliseLineItems,
createInvoice
);
By the time createInvoice runs, req is carrying five properties put there by five different files, in an order defined by this list and nowhere else. Change the order and something breaks at runtime, in production, in a code path nobody tested.
Why this is worse than it looks
The route handler has dependencies. That is fine and normal. The problem is that those dependencies are invisible: nothing in the signature of createInvoice says it needs a workspace and a billing profile.
That produces three concrete costs:
- Tests get heavy. To unit-test the handler you must construct a request object that looks exactly like whatever the chain produced.
- Types stop helping. In TypeScript,
req.workspaceeither does not exist or is declared globally as possibly-undefined, so every handler gets a non-null assertion. - Reuse dies. Want to create an invoice from a cron job? None of that middleware runs, so you reimplement the logic.
The refactor
Keep middleware for things that are genuinely about the HTTP request: parsing, auth, rate limiting, logging, correlation IDs. Move everything else into plain functions that take arguments.
// services/invoices.js — no Express anywhere in this file
export async function createInvoice({ workspace, actor, lineItems }) {
assertSeatsAvailable(workspace);
const normalised = normaliseLineItems(lineItems);
return invoices.insertOne(buildInvoice(workspace, actor, normalised));
}
The route becomes a thin translation layer whose only job is turning HTTP into arguments and a result back into HTTP:
router.post('/invoices', requireAuth, async (req, res, next) => {
try {
const workspace = await workspaces.findForUser(req.user.id, req.body.workspaceId);
const invoice = await createInvoice({
workspace,
actor: req.user,
lineItems: req.body.lineItems,
});
res.status(201).json(serialise(invoice));
} catch (error) {
next(error);
}
});
Yes, the handler got longer. That is the point — the work was always happening, it was just spread across files where you could not see it.
The objection, and the answer
The usual objection is duplication: now every route that needs a workspace loads one.
In practice that duplication is one line, it is explicit, and it is often not duplication at all. The invoice route needs the workspace with its billing profile; the settings route needs it with its member list. The shared middleware was fetching both for everyone.
Where middleware still earns its place
Middleware is genuinely good at cross-cutting concerns that do not vary by route:
- Attaching a request ID and a child logger
- Verifying a session and rejecting early
- Applying CORS, compression, body limits
- Catching errors and mapping them to status codes
The test I use now: if removing the middleware would change what the endpoint means rather than how the request is handled, it does not belong in middleware.
That rule has held up across two codebases and roughly thirty routes. The handlers got longer and the bugs got rarer, which is a trade I will take every time.