Express js middleware res.locals after next

Question

I implemented a piece of middleware (for caching) like this (trivialised from the real example):

import asyncHandler from 'express-async-handler';
import cache from '../cache';

export async function cacheMiddleware(req, res, next) {

  // Check if present in cache
  res.locals.fromCache = cache.get(res.locals.params.cacheKey);

  // Call subsequent middleware, but DO NOT return
  next();

  // After subsequent middleware chain, Update the cache if needed
  if (res.locals.accessGranted === true) {
    cache.set(res.locals.params.cacheKey, true);
  }

}

export default asyncHandler(cacheMiddleware);

In subsequent middleware I make use of the res.locals.fromCache to determine if the request has a cached value. That works perfectly.

As you can see, I also wanted to handle the process of adding to the cache in the same middleware. By not returning after next, upon completion of the middleware chain, the call stack returns to this middleware to perform some additional actions.

What caught me out is that I was expecting res.locals to be an object reference and thus any modifications made in subsequent middleware would now be available here. As you can see, in this case I'm trying to access res.locals.accessGranted (set in a subsequent middleware) to determine whether I need to update the cache, but this property does not exist on the res.locals in this scope. This was surprising to me, I guess res must be a deep copy!?

Is there a best practice for doing this kind of operation or is there some reason not to do this at all? It seemed such a tidy solution (and a pattern I have used in similar frameworks in Python successfully) and preferable to having 2 pieces of middleware for getting and setting the cache.

Answer 1

Good question! Personally I think this approach might tie you in knots a bit, first I'll explain the issue you're having and then I'll explain why I think you should change approach a bit.

The problem you're seeing is a race condition. In this particular instance, next() is non blocking. It's saying "okay I'm done, Express please continue on" and express goes on it's merry way. If you add some console logs into your program you'll notice that the lines after next() is called are probably being executed before your later handlers are having time to finish, in particular if they're making async calls to a database or something.

Effectively you're running your if statement and cache.set before the next piece of middleware has finished, thus causing a race condition.

I don't believe, although I have not checked recently, that next() will allow you to pass and receive a callback, this means you're best off splitting this middleware in two. I would have the cache retreival happen as your first function, then call your other middleware functions, and finally call a function which does any cache updates. Think of middleware less like a module, and more like a plain old function and this makes a lot more sense than having one overloaded middleware function.

So now you would have

fetchFromCache -> [various middleware, perhaps some hefty controllers] -> updateCache