1

I should say this is purely a statistical thing that needs to be 99% accurate and directionally correct. In other words, there's a little room for error.

I'm currently doing this with a piece of middleware that checks req.headers.referer and updates Mongo if the user hasn't come from one of my pages. That's resulted in me making an addition to almost every instance of app.get. It works, but I don't know if it's the right solution.

Here are my concerns -

  1. It's starting to look messy. I feel like I'm on a slippery slope to middleware hell, which almost every piece of middleware I add being needed for almost every route. Lots of repeated calls. Lots of samey looking code.

  2. Is it efficient? I know it'll only get called once per request, so my concern really is related to the above. I'm I beginning something that will result in a long chain of middleware before the user gets a response?

(I'm new to Node, Express, and Mongo, so if you're wondering why I haven't done something obvious, that's probably the reason)

  • So you need to do something for every new user that enters your website, right? If I got it right, you can just set a cookie that indicates that the user already visited the website, and then when user enters the website you check if he has that cookie, if he doesn't you do what you need to do and set the cookie. If the cookie exists you just go on to the next middleware – habiiev Jun 22 '20 at 13:54
  • That's correct habiiev, but I'm doing this to avoid setting cookies because I'm trying to make a flow that obeys cookie laws, with minimal inconvenience to the user. – Sean Thompson Jun 23 '20 at 16:04

1 Answers1

1

Instead of having to add your middleware to every route, you can define this middleware as an application-level one that gets executed for every request. Inside the middleware you can then filter out the few routes for which it should not run:

const excludedPathsForStatistics = ['/your/exlcude-paths-here'];
const yourStatisticMiddleware = function(req, res, next) {
    if(!excludedPathsForStatistics.some(path => path === req.path) {
        YourStatisticService.updateStatistics(req);
    }
    next();
}

app.use(yourStatisticMiddleware);

Regarding performance - yes, this would run for every request, but it should not be a big deal as long as you make sure you're not blocking the event-loop inside updateStatistics().

eol
  • 23,236
  • 5
  • 46
  • 64
  • 1
    Thanks for this. It's revealed a gap in my knowledge about the life cycle of items that are added through app.use, get, post et al! ```app.use((req, res) => res.render('notfound', {"path":req.originalUrl}))``` I have this line right down the the bottom of my main js file. Because it's added last to app, it's lowest priority during routing. ie, if something doesn't get toured somewhere else, you get my notfound page. That's obviously not the case above. Is that because it's middleware? What's it looking for during routing to stop executing? A page render? – Sean Thompson Jun 23 '20 at 15:58
  • Good point, in the code I've posted the middleware chain (i.e. all middlewares after this one) would be broken if a non-excluded path is hit. So you should call `next()` in either case. I'll edit my answer. – eol Jun 23 '20 at 16:09