48

For certain pages I have custom 500, 404 and 403 error handling in my app. So for instance after an unsuccessful database query I'd go:

return next({status: 404, message: 'Record not found'});

or

return next(new Error('Bad things have happened')});

In my middleware I have an error handler:

app.use(function (err, req, res, next) {
    // handle error
});

Problem is that the error handler is never called, instead the error callstack is being printed into the browser. I want the handler to render a custom error page.

app.js

var express = require('express')
    , app = express()
    , swig = require('swig')
    , config = require('./lib/config')
    , env = process.env.NODE_ENV || 'development'
    , path = require('path');

config.configure(env);

app.engine('html', swig.renderFile);
app.set('view cache', false);

swig.setDefaults({
    cache: config.get('swigCache')
});

app.set('view engine', 'html');
app.set('views', __dirname + '/lib/views');

require('./lib/util/swig');
require('./lib/initialisers/mongodb')();
require('./lib/initialisers/aws')();
require('./lib/middleware')(app); // first load middleware
require('./lib/routes')(app); // then routes

var server = app.listen(config.get('port'), function() {
    console.info('config: ' + JSON.stringify(config.getCurrent()));
    console.info('NODE_ENV: ' + env);
    console.info('server running: ' + JSON.stringify(server.address()));
});

routes.js

module.exports = function(app){

    app.get('/', require('./views/').index);
    app.get('/blog', require('./views/blog').index);
    app.get('/blog/:slug', require('./views/blog').getBySlug);

    app.route('/report/:slug')
        .get(require('./views/report/').index)
        .post(require('./views/report/').doReport);

        // Very long file with tons of routes. Simplified version.

middleware.js

var express = require('express')
    , app = express()
    , path = require('path')
    , logger = require('morgan')
    , cookieParser = require('cookie-parser')
    , bodyParser = require('body-parser')
    , passport = require('passport')
    , session = require('express-session')
    , mongoStore = require('connect-mongo')(session)
    , compression = require('compression')
    , favicon = require('serve-favicon')
    , config = require('./config')
    , flash = require('connect-flash')
    , multer = require('multer')
    , csrf = require('csurf');

module.exports = function(app) {

    app.use(bodyParser.urlencoded({ extended: false }))
    app.use(bodyParser.json());
    app.use(cookieParser());
    app.use(csrf({ cookie: true }));
    app.use(express.static(path.join(__dirname, config.get('staticContentPath')), {
        maxAge: (60 * 60 * 24) * 1000
    }));

    app.use(session({
        resave: true,
        saveUninitialized: true,
        secret: 'da755fc0-6882-11e4-9803-0800200c9a66',

        cookie: {
            maxAge: 24 * 60 * 60 * 1000 // 24 hrs
        },

        store: new mongoStore({
            url: config.getMongoConn()
        })
    }));

    app.use(logger('dev'));
    app.use(flash());

    /**
     * 301 redirects
     */
    app.use(function(req, res, next) {

        var host = req.get('host');

        // AWS IP --> http
        if (host == 'xx.xxx.xxx.xxx') {
            return res.redirect(301, config.get('url') + req.originalUrl);
        }

        // AWS origin --> http
        if(host == 'xxx-xxx-xxx-xxx-xxx.ap-southeast-2.compute.amazonaws.com'){
            return res.redirect(301, config.get('url') + req.originalUrl);
        }

        // www --> http
        if (/^www\./.test(host)) {
            host = host.substring(4, host.length);
            return res.redirect(301, req.protocol + '://' + host + req.originalUrl);
        }

        // Trailing slash --> http
        if (req.path.substr(-1) == '/' && req.path.length > 1) {
            var query = req.url.slice(req.path.length);
            return res.redirect(301, req.path.slice(0, -1) + query);
        }

        next();
    });

    // Delete expired Mongo sessions from DB
    app.use(function (req, res, next) {
        req.session._garbage = new Date();
        req.session.touch();
        next();
    });

    /**
     * Setting Cache control header for Ajax requests to 30 minutes
     */
    app.use(function (req, res, next) {

        if(req.xhr){
            res.header('Cache-Control', 'max-age=' + 1800 + ', public');
        }

        next();
    });

    app.use(compression());

    app.use(
        multer({
            dest: config.get('uploads').folders.temp
        })
    );

    app.use(passport.initialize());
    app.use(passport.session());
    var initPassport = require('./passport/init');
    initPassport(passport);

    app.use(function (req, res, next) {

        res.locals = {
            root : 'http://' + req.headers.host,
            sitename : require('./config').get('sitename'),
            config: config.get('env'),
            url : config.get('url'),
            user : req.user,
            flash : req.flash()
        };

        next();
    });

    app.use(function (err, req, res, next) {

        if (err.code !== 'EBADCSRFTOKEN'){
            return next(err);
        }

        if(req.xhr){
            return res.ok({payload: null}, '403 invalid csrf token');
        }

        // TODO handle CSRF token errors here
        res.status(403);
        res.send('form tampered with')
    });

    // This is never called when throwing errors like
    // next(new Error('some error') or
    // next({status: 500, message:'server error'});
    app.use(function (err, req, res, next) {
        console.error(err.stack);
        // render an error page
    });
};
ChrisRich
  • 8,300
  • 11
  • 48
  • 67
  • You've got "app.use(function (err, req, res, next) {" declared twice. Is the res.status (403) being returned? – Dominic Scanlan Apr 17 '15 at 13:19
  • But that returns next(error) unless it is a bad CSRF token error. Even if I take that block of code out and just have a single app.use(function (err, req, res, next) it is never called. – ChrisRich Apr 17 '15 at 13:23
  • 18
    Routes are classed as middleware, as everything uses the same router. Error handlers should always be at the end of your call stack. Add them to their own file and add it after your routes. [Error handling docs](http://expressjs.com/guide/error-handling.html) If you want to keep your middleware error handler then you need to add after your routes too. – Ben Fortune Apr 17 '15 at 13:26
  • Ben Fortune, your answer seems to have solved my problem. Post the answer and I will approve it. – ChrisRich Apr 17 '15 at 23:56

6 Answers6

91

The problem is, that your error handlers should always be at the end of your application stack. This means, that you can either move the error handler from your middleware to your app.js and use them after your requires (app.use()) or include your routes before your middleware.

djKartright
  • 1,017
  • 8
  • 11
  • 7
    Dang! I took middlewares literally and placed it in the middle of app initialization and the routes definition. – JohnnyQ Mar 25 '17 at 16:27
  • I just realized a problem with just transferring the routes entirely under the routes in the context of the OP. The `bodyParsers` and `urlEncoders` won't work, the routes should be placed in between these 2 middlewares. – JohnnyQ Mar 26 '17 at 05:00
  • Gotcha! Struggled with this all day along : https://stackoverflow.com/questions/46929330/express-async-await-error-handling - will post what was my problem and how this solved. – npr Oct 26 '17 at 17:58
  • Not working on Firebase Functions... I created an app with all routes on that app and app.use(customErrorHandler) before exporting the app as a functions.https.onRequest(app). Really not sure why it never gets called, but HTML response is not acceptable from a RESTful API. – SacWebDeveloper Sep 12 '19 at 08:51
  • Yes working now after moving it to the last, thank you. – M_abosalem Feb 13 '22 at 19:02
  • Ah, I didn't consider that it had to go after the routes as well. Thanks! – Clonkex May 11 '22 at 05:38
71

Note: your error handler middleware MUST have 4 parameters: error, req, res, next. Otherwise your handler won't fire.

Van SallyOne
  • 833
  • 6
  • 9
18

I had the same issue, spend the whole day on troubleshooting the issue. Finally, found a simple fix. This worked for me perfectly. You need to place the customer error handler right before the listener handler as below on the server instance file (App.js / server.js). Good luck :)

app.use((error, req, res, next) => {

    if (res.headersSent) {
        return next(err)
    }
    res.status(500).send('INTERNAL SERVER ERROR !')
  });

app.listen(3000, function() {
    console.log('Node app is running on port 3000');
});
module.exports = app;
Chamath Jeevan
  • 5,072
  • 1
  • 24
  • 27
18

For handling errors that are thrown during asynchronous code execution in Express (versions < 5.x), you need to manually catch and invoke the in-built error handler (or your custom one) using the next() function.

app.get('/', (req, res, next) => {
  setTimeout(() => {
    try {
      console.log('Async code execution.')
      throw new Error('Unexpected error on async!')
    } catch (error) {
      // manually catching and propagating to your error handler
      next(error)
    }
  }, 100)
})

// as a last declaration in your entrypoint
app.use((err, req, res, next) => {
  // do some error handling here. if you do not call next, you must return a response from here.
})
infinity
  • 530
  • 5
  • 6
10

Your error handler should always be at the end of your application stack. Apparently it means not only after all app.use() but also after all your app.get() and app.post() calls.

Lukasz Czerwinski
  • 13,499
  • 10
  • 55
  • 65
  • Thanks for this comment; I had an error handler at the end of my middleware but NOT after the routes were added. Moving it to the end fixed everything for me. – Appurist - Paul W Mar 01 '23 at 21:48
5

If you don't want to write three parameters for every async router handler to be able to catch errors globally:

npm install express-async-errors

import 'express-async-errors';

app.get('/api/endpoint', async (req, res) => {
  const user = await User.findByToken(req.get('authorization'));
 
  if (!user) throw Error("access denied"); //will propagate to global error handler
});
besthost
  • 759
  • 9
  • 14
  • 1
    thanks so much. I was missing this in my project. – CodeConnoisseur Dec 28 '22 at 15:34
  • I unintentionally inserted `async` into the router handler definition and the error handler stopped working. After a few hours, I "fixed" it by removing `async`. It turns out, ExpressJs doesn't handle exceptions from `async` handlers. However, I found I still need async routers, so finally I found that `express-async-error` makes the error handler work with it, you have to put `require('express-async-error')` at the beginning of your app to make it work. – Alexander Fadeev Feb 08 '23 at 19:11
  • Yoo thanks, this should be built into express fr – Angelo II May 16 '23 at 18:09