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;
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;
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.
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.