Express Error handling middleware for production and development

Question

I am trying to make the production error log the default and show the user the additional stuff only if the environment variable is development. I am trying to do this in the following way below, but I get a message saying Cannot set headers after they are sent to the client .

    const ErrorClass = require('../routes/utils/ErrorClass');

const prodDBCastError = err => {
    const message = `Invalid ${err.path}: ${err.value}`;
    return new ErrorClass(message, 400);
};

const prodDBDuplicateFieldsError = err => {
    const value = err.errmsg.match(/(["'])(\\?.)*?\1/)[0];

    const message = `Duplicate field value: ${value}.`;
    return new ErrorClass(message, 400);
};

const prodDBDValidationError = err => {
    const errors = Object.values(err.errors).map(el => el.message);

    const message = `Invalid input data. ${errors.join('. ')}`;
    return new ErrorClass(message, 400);
};

const handleBadRequestDB = err => {
    const errors = err.message;
    const message = `Fixes: ${errors}`;
    return new ErrorClass(message, 400);
};

const sendProdError = (err, res) => {
    if (err.isOperationalError){
        res.status(err.status).json({
            status: err.status,
            message: err.message,
        });
    } else {
        res.status(500).json({
            status: 'error',
            message: 'Server Issue.',
        });
    }
};

const sendVerboseDevError = (err, res) => {

    logger.error(err);
    err.status = err.status || 500;
    res.status(err.status).json({
        status: err.status,
        name: err.name,
        path: err.path,
        errors: err.errors,
        message: err.message,
        stack: err.stack,
    });
};

module.exports = (err, req, res, next) => {

    if (process.env.APP_ENV === 'development'){
        sendVerboseDevError(err, res);
    }
    if (err.name === 'CastError') {err = prodDBCastError(err);}
    if (err.name === 'MongoError') {err = prodDBDuplicateFieldsError(err);}
    if (err.name === 'ValidationError') {err = prodDBDValidationError(err);}
    if (err.name === 'Bad Request') {err = handleBadRequestDB(err);}

    sendProdError(err, res);
};

This is what my ErrorClass looks like:

class ErrorClass extends Error {
    constructor(message, status) {
        super(message);

        this.status = status;
        this.isOperationalError = true;

        Error.captureStackTrace(this, this.constructor);
    }
}
module.exports = ErrorClass;

In your error handler you call both `sendVerboseDevError(err, res);` and `sendProdError(err, res);` this means you are calling `res.status(..).json(..)` twice but after the first call the connection to the client is closed and no more data can be sent which causes the error. So call only one of the functions never both. — Molda, Feb 26 '21 at 17:57
I could not think of any other way to make `sendProdError` as the base case and always send the output without giving away too many details unless the `env var` is `development` — boomchickawawa, Feb 26 '21 at 18:05

score 2 · Accepted Answer · answered Feb 26 '21 at 18:15

2

I would recommend you to change your code like this.

First you avoid calling res.json multiple times and also you only check if the app is running in a development mode once. There's no need to check it with every request.

var devHandler = (err, req, res, next) => {
    logger.error(err);
    err.status = err.status || 500;
    res.status(err.status).json({
        status: err.status,
        name: err.name,
        path: err.path,
        errors: err.errors,
        message: err.message,
        stack: err.stack,
    });
};

var prodHandler = (err, req, res, next) => {
    
    if (err.name === 'CastError') {err = prodDBCastError(err);}
    if (err.name === 'MongoError') {err = prodDBDuplicateFieldsError(err);}
    if (err.name === 'ValidationError') {err = prodDBDValidationError(err);}
    if (err.name === 'Bad Request') {err = handleBadRequestDB(err);}

    if (err.isOperationalError){
        res.status(err.status).json({
            status: err.status,
            message: err.message,
        });
    } else {
        res.status(500).json({
            status: 'error',
            message: 'Server Issue.',
        });
    }
};

module.exports = process.env.APP_ENV === 'development' ? devHandler : prodHandler;

answered Feb 26 '21 at 18:15

Molda

5,619
2
23
39

This is awesome, works great and makes a lot of sense to just call the development once, but how about checking for the `env var` inside one function which is the `prodHandler` and making this the single point of entry and then sending the output accordingly? I included an` if` block inside the handler that checks for the `env var` and calls the `devHandler` if true, but this again sends the same error – boomchickawawa Feb 26 '21 at 18:28
The APP_ENV value will not change while the app is running so why did you include if statement? – Molda Feb 26 '21 at 18:32
`module.exports = prodHandler` doing this makes it the single point of entry for the errors – boomchickawawa Feb 26 '21 at 18:33
1

In your code all you need is to add `return` after `sendVerboseDevError(err, res);` or in front of it like so `if (process.env.APP_ENV === 'development') return sendVerboseDevError(err, res);` to prevent the rest of the code to run. – Molda Feb 26 '21 at 18:43
So this is the silly mistake I have been doing since forever, thank you so much! – boomchickawawa Feb 26 '21 at 18:56

rahulroy9202 · Answer 2 · 2021-02-26T18:09:03.413

This is because you are trying to send data from sendProdError after sending data from sendVerboseDevError . res.json is sending the json to client.

The reason behind this is explained here https://stackoverflow.com/a/7086621/2232902

The res object in Express is a subclass of Node.js's http.ServerResponse (read the http.js source). You are allowed to call res.setHeader(name, value) as often as you want until you call res.writeHead(statusCode). After writeHead, the headers are baked in and you can only call res.write(data), and finally res.end(data)

I would recommend you to modify the sendProdError and sendVerboseDevError into constructProdError and constructVerboseDevError and then send from the same point in code.

ref: https://stackoverflow.com/a/733858/2232902

score 0 · Answer 3 · answered Feb 26 '21 at 18:13

I think the issue here is with the multiple if conditions. If the first condition becomes true then "sendVerboseDevError" function will be executed. Which has following code

res.status(err.status).json({
    status: err.status,
    name: err.name,
    path: err.path,
    errors: err.errors,
    message: err.message,
    stack: err.stack,
});

after this function the response header will be set. Then the flow if going into the rest of the if conditions if the conditions is true and then again some other function is getting called which is trying to set response again. That's why you are getting error "Cannot set headers after they are sent to the client" You need to call "next()" method after setting the response. So you should add "next()" method at the bottom in each if conditions.

something like

    module.exports = (err, req, res, next) => {

    if (process.env.APP_ENV === 'development'){
        sendVerboseDevError(err, res);
        next();
    }
    if (err.name === 'CastError') {
        err = prodDBCastError(err);
        next();
    }
    if (err.name === 'MongoError') {
        err = prodDBDuplicateFieldsError(err);
        next();
    }
    if (err.name === 'ValidationError') {
        err = prodDBDValidationError(err);
        next();
    }
    if (err.name === 'Bad Request') {
        err = handleBadRequestDB(err);
        next();
    }

    sendProdError(err, res);
    next();
};

the next function will terminate the API call and return the response and preventing it to set the response again after response has been set by any function executed before the current one.

P.S: You can also call "res.end()" instead of "next()" function.

How exactly does the `next()` function prevent calling both `sendVerboseDevError(err, res);` and `sendProdError(err, res);` ??? In your code both `sendVerboseDevError(err, res);` and `sendProdError(err, res);` will be called in development mode just like in the question. — Molda, Feb 26 '21 at 18:19
you can refer to this link. https://stackoverflow.com/questions/14709802/exit-after-res-send-in-express-js https://www.geeksforgeeks.org/express-js-res-status-function/#:~:text=The%20res.,chainable%20alias%20of%20Node's%20response. — Mr.UV, Feb 26 '21 at 18:20
I tried the code from example 2 on that page and it causes the same error `Cannot set headers after ...` You simply cannot call `res.send` and/or `res.json` twice and `next` does not prevent that in any way. — Molda, Feb 26 '21 at 18:35

Express Error handling middleware for production and development

3 Answers3