I'm new programming in node.js. I'm doing an app to pull code from Github, etc. I just check if some variable is set in query param (URL), like localhost:3000/gitservices/gitpull?development.
I'm in a piece of code where i'm doing the git pull. If the path passed by URL exists, then redirect to the success page. If the query param passed by url does not exists, It will "throw" an error, handled by a middleware.
But even passing a res.render, for example, the code continues and my line of code responsable to redirect to the error page (handled by the middleware) throws this error: Error [ERR_HTTP_HEADERS_SENT]: Cannot set headers after they are sent to the client
environments = require('../config/paths').env;
const { exec } = require("child_process");
//Controller to do git pull
exports.gitPull = (req, res, next) => {
//query param (example: localhost/gitservices/gitpull?development
const env = req.query.env;
//checking if the query param matches with an array defined in config file.
//Ex. ([{name: development, path: '/var/www/development'}])
environments.map(e => {
if (e.name == env) {
//doing the command
let command = "cd " + e.path + " && git pull origin main";
exec(command, (err, stdout, stderr) => {
//redirecting to the success page
res.status(200).render('success', {
pageTitle: 'Success',
result: stdout
});
});
}
});
//if none path is equal to query param, return an error.
next(new Error('Ambiente ' + env + ' errado'));
};
As @deekep says, render()
does not terminate the method, so after invoking that method it continues to execute everything after.
Furthermore, you are calling render()
inside a callback method, which means it's asynchronous and that the code probably does execute next(...)
even before render()
.
Since it's asynchronous, you need to change the structure of your code.
First of all, since you want to find an item in an array, you should use find()
instead of map()
. Please think about if the method you are using is for what you intend to use it for. Also forEach()
or some()
would be more suited for this. This is important for not just "using the correct method" and performance, but also improves readability of your code.
Utilizing the find()
method does also make this a lot simpler, since you can just throw the error if nothing was found.
To sum it all up, your controller should look like:
exports.gitPull = (req, res, next) => {
//query param (example: localhost/gitservices/gitpull?development
const env = req.query.env;
//checking if the query param matches with an array defined in config file.
//Ex. ([{name: development, path: '/var/www/development'}])
let found = environments.find(e => e.name === env);
if(!found) {
//if none path is equal to query param, return an error.
next(new Error('Ambiente ' + env + ' errado'));
}
//doing the command
let command = "cd " + found.path + " && git pull origin main";
exec(command, (err, stdout, stderr) => {
if(err) {
// error handling
}
//redirecting to the success page
res.status(200).render('success', {
pageTitle: 'Success',
result: stdout
});
});
};
Also, just as random addition info, you can always check if the headers were already sent with res.headersSent
, so if you are unsure about that part just wrap it with if(!res.headersSent)
.