ExpressJS antipattern: making everything middleware

Originally published at coreycleary.me. This is a cross-post from my content blog. I publish new content every week or two, and you can sign up to my newsletter if you'd like to receive my articles directly to your inbox! I also regularly send cheatsheets and other freebies.

Something I see in many ExpressJS APIs is the overuse / incorrect use of middleware. Sometimes to the extent that almost everything is middleware.

What this usually ends up looking like is this:

const router = express.Router()

const getCustomerData = async (req, res, next) => {
  try {
    const customerId = req.body.customerId

    const customerDetails = await fetchUserDetails(customerId)

    res.locals.transactionHistory = await fetchCustomerTransactionHistory(customerDetails)

    next()

    return
  } catch (err) {
    next(error)

    return
  }
}

const processOrder = async (req, res, next) => {
  try {
    const customerDiscount = await calculateDiscountFromCustomerTransactionHistory(res.locals.transactionHistory)

    let recalculatedOrderTotal

    if (customerDiscount) {
      recalculatedOrderTotal = req.body.orderTotal - (req.body.orderTotal * customerDiscount)
    }

    const itemsAreInStock = await doubleCheckStock(req.body.orderItems)

    if (!itemsAreInStock) {
      return res.send('Item(s) out of stock')
    }

    await insertOrder(recalculatedOrderTotal)
    await chargeCustomerPayment(recalculatedOrderTotal || orderTotal, req.body.paymentDetails)

    next()

    return
  } catch (err) {
    next(error)

    return
  }
}

const sendConfirmationEmailToCustomer = async (req, res, next) => {
  try {
    await dispatchConfirmationEmailJob(req.body.customerId, req.body.orderItems)

    res.send('Order complete')

    return
  } catch (err) {
    return
  }
}

router.post('/order', getCustomerData, processOrder, sendConfirmationEmailToCustomer)

The "middleware" here is anything that depends on the ExpressJS req/res/next context. You can see that they are also chained where the route is defined:

router.post('/order', getCustomerData, processOrder, sendConfirmationEmailToCustomer)

Note: your controller will also usually depend on the Express context but it won't act like middleware in that it's chained from one call to the next in the route definition.
The controller will usually have a single entrypoint - so one controller function per route. This isn't a hard-set rule but is generally a best practice.

You usually see middleware in ExpressJS using app.use(someMiddleware) to register the chain of middleware in order. And while this is not an example of that, I'd argue it's still coded essentially as middleware because of the hard dependency on the ExpressJS context.
It's just in a different place in the code - in the route definition instead of the index.js or app.js part of your code where you see the app.use(someMiddleware) setup.

What is this code doing? A few things:

• getCustomerData()
  • fetches user details (likely from a database)
  • fetches the customer transaction history (also likely from a database)
• processOrder()
  • calculates any potential discount for the user
  • checks the item(s) are in stock
  • inserts the item order into the database
  • charges the customer's credit card or other form of payment
• sendConfirmationEmailToCustomer()
  • send the user a confirmation email with their order details

What makes this a problem?

The problem is not really what the code is doing but how, for the following reasons:

• These three functions now depend on the request context. If you want to reuse them / use them in multiple places, every function that calls this must have req, res, and next (the Express "context").
  • You also have to assume sequence of calls and next(), so even though they may be individual functions, they aren't reusable.
• If you have to pass one value from one middleware function to the next, you have to use res.locals to (when we could just return it and pass it via a function argument).
• It makes writing automated tests more difficult.

Request context dependency

One of the biggest issues in my opinion is that these functions are not reusable. Because the function definition is now coupled via its arguments to req, res, and next, and those are coupled to ExpressJS, you can't call them anywhere else in your code.
Unless it's somewhere you have the ExpressJS context (more on this a bit further down).

If these were just "regular" functions, the context would not matter. That is, if you could just pass in "agnostic" values/objects/arrays etc, then you could reuse them elsewhere in your code.
Sure, the expected types and expected arguments matter but you can reuse a functions in ways that make sense for your application.
You can call your utility functions in your service layer code or your database code, for example.
And obviously the business logic still matters, i.e. you're not going to arbitrarily call functions.
Similarly, you're not going to call controller functions from within another controller either.

But by not being totally coupled to the core Express objects/functions this gets us a long ways towards reusability. We should always strive for loose coupling when designing our software.

You might be able to "reuse" that middleware elsewhere but only as middleware and even then it might not be reusable.
Consider a function that is supposed to end the request by calling res.send(response). You can't really reuse that (without changing the function definition), because it ends the request so you couldn't call it in the middle of your chain.
And if you need to pass value(s) from one middleware function to the next, this pesudo-middleware reusability becomes even more difficult, as explained in the next section.

Passing values from one function to the next

In our code above, getCustomerData() calls fetchCustomerTransactionHistory() and then needs to pass it to the next middleware function, processOrder(). Because these functions get called in a chain, we need some way of passing that value to processOrder(), since we have no intermediary variable to store the result in.

You can do that thru res.locals.transactionHistory = transactionHistory or by attaching a new property to the res object arbitrarily, like res.transactionHistory = transactionHistory.
Any property added to res.locals is only available for the lifecycle of the request, so when the request completes you can't access it again.

This is much messier than if we could just call getCustomerData(), store the result in a variable customerData or whatever and then pass that to processOrder().

Also, this further reinforces that the order of middleware function calls matters when going about it this way. Because one function willy rely on previous res.locals being set, the order of calls must stay the same.
And if you want to change the value that gets passed, you inevitably have to change the implementation of more than one function, you can't just change the one function.

While res.locals is supported by ExpressJS, and you can of course set new properties on objects if you go the custom property on res route, I don't recommend this unless it's something you absoluetly need to do as it can make troubleshooting mroe difficult.
But anyways, it's best to avoid this altogether and have your utility/business/DB logic in non-middleware code.

Makes writing automated tests more difficult

In order to write tests for this type of code, we now either need to stub req and res or we need to test this end-to-end using something like supertest.
Endpoint/end-to-end tests are good to have, but these functions we want to test are individual/modular (or at least, should be modular/resuable) and should be able to be tested more as units.
We shouldn't have to test them by spinning up a mock server, or by manually stubbing req and res - that's unnecessary complexity and work.
And stubs for request and response objects can require more maintenance, tight coupling, etc.
Not that stubs are bad - quite the opposite - and in the case of the functions above we'd likely want to stub some of the database and async calls.
But in this case we don't want to have to write them for req/res. They would have to be more like mocks, where we define the next() function and make assertions that it was called, stub the res.send() function, which is an implementation we don't care about, etc.

Instead if we could just break these pesudo-middlewares into resuable funtions without the ExpressJS context, we could test them by passing expected parameters to the functions which makes the test setup much easier.

What middleware is really for

This topic could be a few blogposts by itself, but to get the general idea across middleware should be used for things that are common to all HTTP requests but don't contain business logic, and that need to be processed before everything else.

Things like:

• Authorization/authentication
• Caching
• Session data
• CORS
• HTTP request logging (like morgan)

All of the above are their own category of API concern, separate conceptually from code that is concerned with fetching data from the database, sending out a user registration email, etc.
Authorization and authentication need to happen before a user or client application access a service. That's something that is common to every (or most) requests.
Caching, that is generally common to most requests, and is a utility that is a separate concern from business or view logic.
Same with session data, same with CORS, same with request logging.

While there are always exceptions to any rule, middleware almost always should not contain the core of your code that handles business logic, that handles code specific to your REST API, that is "further down" the chain of function calls.

I like to think of business logic as the more "pure" form of logic. It is logic that shouldn't care about validating the request or handling anything framework-specific. It just handles algorithms/rules for processing data, storing of data, fetching data, formatting that data, etc. These rules are usually determined by business requirements.

For example, if you had an API that returned how many users had been registered on your platform within the last X amount of days, the business logic here would be querying the database and doing any formatting of that data before it returned it to the controller, which returns the HTTP response.
That logic won't handle caching or auth or session data. The middleware takes care of that.

How to fix it

If we make these "normal" functions rather than "middleware" functions coupled to ExpressJS, this is what they could look like. Of course you could refactor it further but this is the general idea:

const getCustomerData = async (customerId) => {
  const customerDetails = await fetchUserDetails(customerId)

  return fetchCustomerTransactionHistory(customerDetails)
}

const processOrder = async (orderTotal, orderItems, paymentDetails, transactionHistory) => {
  const customerDiscount = await calculateDiscountFromCustomerTransactionHistory(transactionHistory)

  let recalculatedOrderTotal

  if (customerDiscount) {
    recalculatedOrderTotal = orderTotal - (orderTotal * customerDiscount)
  }

  const itemsAreInStock = await doubleCheckStock(orderItems)

  if (!itemsAreInStock) {
    return null
  }

  await insertOrder(orderTotal, orderItems)
  return chargeCustomerPayment(recalculatedOrderTotal || orderTotal, paymentDetails)
}

const sendConfirmationEmailToCustomer = (customerId, orderItems) => {
  return dispatchConfirmationEmailJob(customerId, orderItems)
}

Note: sendConfirmationEmailToCustomer() is just a wrapper function basically. We could just call dispatchConfirmationEmailJob() directly now, but I'm leaving it in to demonstrate the before and after.

Now we have functions that are more reusable, not coupled to ExpressJS, and require less test setup to write tests for.

You might call these functions in your controller like so:

// Controller
const createOrder = async (req, res, next) => {
  const {customerId, orderTotal, orderItems, paymentDetails} = req.body

  try {
    const customerData = await getCustomerData(customerId)
    await processOrder(orderTotal, orderItems, paymentDetails, customerData)
    await sendConfirmationEmailToCustomer(customerId, orderItems)

    res.sendStatus(201)

    return
  } catch (err) {
    res.sendStatus(500) // or however you want to handle it

    return
  }
}

// Route
router.post('/order', createOrder)

You could of course use these individual functions elsewhere in your code though, now that they are reusable!

Love JavaScript but still getting tripped up by local dev, architecture, testing, etc? I publish articles on JavaScript and Node every 1-2 weeks, so if you want to receive all new articles directly to your inbox, here's that link again to subscribe to my newsletter!