node express: should you always call next() in a get or post handler?

Question

Until now I've defined my get and post handlers with just (req, res) as arguments, with the assumption being that I put these handlers last in the chain of middleware, and make sure that I handle any responses and error handling properly within these handlers... hence it doesn't matter that I don't make any reference to next.

Is this a valid and sensible approach, or is it good practice always to call next() even if (at present) there is nothing coming afterwards? For example, perhaps in the future you might want to do some handling after these routes... or maybe there's a reason I haven't yet come across why it's good practice to always call next().

For example, there is the following simple example in the express routing guide:

app.get('/example/b', function (req, res, next) {
    console.log('the response will be sent by the next function ...')
    next()
}, function (req, res) {
    res.send('Hello from B!')
})

Of course, I appreciate that this is a very simple example to illustrate that handlers can be chained, and is not intended to provide a complete framework for a get handler, but would it be better to define and use next even in the second handler, as follows?

app.get('/example/b', function (req, res, next) {
    console.log('the response will be sent by the next function ...')
    next()
}, function (req, res, next) {
    res.send('Hello from B!')
    next()
})

Or is it actually common practice to assume that a handler function that sends a response back to the client should not call next()... i.e. the assumption should be that the chain will end at the handler that actually sends the response?

Or is there no established practice on this point?

I'm even wondering whether it might be common not to send any response in the get handler but to defer that to a dedicated response handler coming after... by which I mean an OK response handler rather than an error response handler (for which it seems to be common practice to defined a final error handler and call next(err)). So, in a non-error situation, you would call next() and in the following middleware you would do your res.status(200).send(req.mydata) where req.mydata is added in your get handler.

Answer 1

No. You should only call next() if you want something else to handle the request. Usually it's like saying that your route may match that request but you want to act like it didn't. For example you may have two handlers for the same route:

app.get('/test', (req, res, next) => {
  if (something) {
    return next();
  }
  // handle request one way (1)
});

app.get('/test', (req, res) => {
  // handle request other way (2)
});

Always the first matching handler is called, so for the GET /test request the first handler will be called, but it can choose to pass the control to the second handler, as if the first didn't match the request.

Note that if the second handler doesn't intend to pass the request to the next handler, it doesn't even have next in its arguments.

If there was no second handler, then the standard 404 handler would be used if the first one called next().

If you pass an argument to next() then an error handling middleware will be called.

Answer 2

My rule of thumb is to handle the response in the handler if you're going to give a 20x (Success) response code, and in centralized error handling if not. That looks something like this in practice:

// ./routes/things.js

const express = require('express');
const Thing = require('../models/thing');

const Router = express.Router();

// note, the handlers might get pulled out into a controllers file, if they're getting more complex. 
router.param('thingId', (req, res, next, id) => {
  Thing.findById(id, (e, thing) => {
    if (e) return next(e);
    // let's say we have defined a NotFoundError that has 'statusCode' property which equals 404
    if (!bot) return next(new NotFoundError(`Thing ${id} not found`));
    req.thing = thing;
    return next();
  });
});

router.get('/', (req, res, next) => {
  // possibly pull in some sort, limit, and filter stuff
  Thing.find({}, (e, things) => {
    if (e) return next(e);
    res.send(things);
  });
});

router.route('/:thingId')
  .get((req, res) => { 
     // if you get here, you've already got a thing from the param fn
     return res.send(req.thing);
  })
  .put((req, res, next) => {
    const { name, description } = req.body; // pull whitelist of changes from body
    let thing = req.thing;
    thing = Object.assign(thing, { name, description }); // copy new stuff into the old thing
    thing.save((e) => {
      if (e) return next(e);
      return res.send(thing); // return updated thing
    });
  });

Keeping each logical chunk in its own file can reduce repetition

// ./routes/index.js then mounts the subrouters to the main router
const thingsRoute = require('./things');
const express = require('express');

const router = express.Router(); 

/* .... other routes **/
router.use('/things', thingsRoute);

Error handling is then centralized, and can be mounted either in its own file or right on the app:

// in ./index.js (main app entry point)
const express = require('express');

// this will require by default ./routes/index.js
const routes = require('./routes'); 
const app = express();
const log = require('./log');// I prefer debug.js to console.log, and ./log.js is my default config file for it

/* ... other app setup stuff */

app.use(routes);

// you can mount several of these, passing next(e) if you don't handle the error and want the next error handler to do so.
app.use((err, req, res, next) => {
  // you can tune log verbosity, this is just an example
  if (err.statusCode === 404) {
    return res.status(404).send(err.message);
  }
  log.error(err.message);
  log.verbose(err.stack); // don't do stack traces unless log levels are set to verbose
  return res.status(500).send(err.message);

});