How to handle http-errors in express in production?

user_4847251 0
Viewed 5659

I am developing express app and after I specify all my routes and middlewares I have this at the end of server.js:

// Log errors
app.use(function (err, req, res, next) {
    logger.error(err.stack);

    if(process.env.NODE_ENV === 'production')
        return res.status(500).send('Something broke!');

    next(err);
});

// Start server
app.listen(port, () => {
    logger.info('Server is up on port ' + port);
});

The purpose of this is to catch ALL the errors on production and to avoid leaking sensitive data to the client.

I have this code in one of my controllers:

const createHTTPError = require('http-errors')

async function(req, res, next) {
    try {
        invoice = await Invoice.create({
            // data
        });
    }catch (e) {
        if(e instanceof Sequelize.ValidationError){
             logger.error(e);
             return next(createHTTPError(400, 'Validation did not pass: ' + e.message));
        }
    }
}

The problem is, that when next() is called with http-errors object, it bubbles up to my catch-all error handler but information is lost and inside it the err object is a simple Error instance with these params:

message = "Validation did not pass: notNull Violation: invoice.clientEmail cannot be null"
name = "BadRequestError"
stack = "BadRequestError: Validation did not pass: notNull Violation: invoice.clientEmail cannot be null\n    at module.exports (/home/XXXX/create-new-invoice.js:109:33)"

Error code number is lost. Error object type is lost (well, converted to string in name).

What should I do? If I remove my catch-all, I am risking that some sensitive info will be leaked. Thanks

express node.js
1 Answers

So I ended up with this code:

const HTTPErrors = require('http-errors');
const HTTPStatuses = require('statuses');

// ... set up express, middlewares, routes...

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

    let messageToSend;

    if(err instanceof HTTPErrors.HttpError){
        // handle http err
        messageToSend = {message: err.message};

        if(process.env.NODE_ENV === 'development')
            messageToSend.stack = err.stack;

        messageToSend.status = err.statusCode;
    }else{
        // log other than HTTP errors (these are created by me manually, so I can log them when thrown)
        logger.error(err.stack);
    }

    if(process.env.NODE_ENV === 'production' && !messageToSend){
        messageToSend = {message: 'Something broke', status: 500};
    }

    if(messageToSend) {

        let statusCode = parseInt(messageToSend.status,10);
        let statusName = HTTPStatuses[statusCode];

        res.status(statusCode);

        // respond with html page
        if (req.accepts('html')) {
            res.send('<html><head><title>'+statusCode+' '+statusName+'</title></head><body><h1>'+statusCode+' '+statusName+'</h1>'+messageToSend.message+'<br/><br/>'+(messageToSend.stack ? messageToSend.stack : '')+'</body></html>');
            return;
        }

        // respond with json
        if (req.accepts('json')) {
            let responseObject = { error: statusName, code: statusCode, message: messageToSend.message };

            if(messageToSend.stack)
                responseObject.stack = messageToSend.stack;

            res.send(responseObject);
            return;
        }

        // default to plain-text. send()
        res.type('txt').send(statusName+' '+messageToSend.message);
        return;
    }

    // if this is not HTTP error and we are not in production, let express handle it the default way
    next(err);
});

This solution:

  • detects and displays HTTP errors from http-errors module (with stack trace for development and without for production)
  • for any other errors it depends if in production (then throw generic 500 Server error) or development (let express handle the error by default, which means print it out with stack trace)
  • formats error output based on the Accepts header (so if app expects JSON, it sends JSON)

I also take advantage of this new catchall function in 404 catchall:

// DEFAULT CATCH
app.use(function(req, res, next){
    next(HTTPErrors(404));
});
user_48472510
Related