0

I am using express (v:4.17.1) and here is my code snippet:

//upload.array is  for multer npm file uplaoding....
//LIMIT is how many files you can upload via multer ....
app.post('/processFiles', upload.array('myFile', LIMIT), (req, res, next) => {

  if (!validEnv) {
    res.setHeader('X-Correlation-ID', corrId);
    res.status(400).json({'error':'invalid env'});

    return next();    //A
  } 

  //checking how many file hvae been uplaoded
  if (totalFiles > LIMIT) {
    res.status(400).json({'error':'Too many files uploaded'});

    return next();  //B
  } 

  //Some xml parsing code and later it inserts into db  
}

Few questions on the way I have added 'return next();' in the routes. Please note that I have not added any explicit middleware, just relying on what express is providing:

  1. If I keep [A] or [B] as it is, then it gives a proper error message to the browser if the conditions get TRUE and returns the response.

  2. But if I comment the line //A or //B, it gives the error response back to browser but also prints few error logs as mentioned below:

Error [ERR_HTTP_HEADERS_SENT]: Cannot set headers after they are sent to the client

I don't know why the above line is appearing when I comment the //A or //B (but wont complain if I keep as it is without commenting them) as the response was already out to the browser and the express will not execute any piece of code after the response has gone.

Please correct my understanding ?

Also, Is this the correct way to handle errors as I have coded? If No, what could be the proper way, please advice? Is there any way by which we can see the complete middleware execution in sequence until the last one via any debug flags

user8479984
  • 451
  • 2
  • 9
  • 23

3 Answers3

1

This is happening because of the //A. See, what is going on here in your code :

  • If you comment line //B it reaches the next if (totalFiles > LIMIT) which is sending another response but above the return next() is called. That is why it is generating error that once they are sent in if(!validEnv) how can Headers be sent again!
Akif Hussain
  • 475
  • 5
  • 22
1

There are a several ExpressJS things to know that apply here:

  1. next() tells Express to continue routing to other requests. Only call next() if you want other routes to continue to be able to process this request and send a response. Or the corallary, don't call next() if you've already sent a response.
  2. You can only send one response for a given request. Once, you've called res.send() or .res.json() or any other way of sending a response, you should be completely done with your request processing and no other code should try to send a response.
  3. res.send() and next() are not Javascript flow control. If you want to stop further processing of the request in your function, you need to code that with traditional Javascript flow control such as return, if/else, etc... to prevent the rest of your code from continuing to run or process the request.

To that end, your code should look like this:

//upload.array is  for multer npm file uplaoding....
//LIMIT is how many files you can upload via multer ....
app.post('/processFiles', upload.array('myFile', LIMIT), (req, res, next) => {

  if (!validEnv) {
    res.setHeader('X-Correlation-ID', corrId);
    res.status(400).json({'error':'invalid env'});
    return;
  } 

  //checking how many file hvae been uplaoded
  if (totalFiles > LIMIT) {
    res.status(400).json({'error':'Too many files uploaded'});
    return;
  } 

  // more code here that sends some other response

}

But if I comment the line //A or //B, it gives the error response back to browser but also prints few error logs as mentioned below: Error [ERR_HTTP_HEADERS_SENT]: Cannot set headers after they are sent to the client

This error occurs when you attempt to send more than one response for the same request (e.g. call res.send(...) more than once. Your code should not allow that. Once you send a response, your code should use normal Javascript flow control (such as return or if/else) to avoid executing any more code that might send a response. Typically after you send a response, you are done processing the request and would just return from the request handler.

jfriend00
  • 683,504
  • 96
  • 985
  • 979
  • Thanks @jfriend00. I tried your approach and it working as expected. However I have seen lot of link (example [link](https://stackoverflow.com/questions/16810449/when-to-use-next-and-return-next-in-node-js) ) where they have recommended to use 'return next()' and hence I was trying that option too – user8479984 Dec 29 '19 at 03:59
  • @user8479984 - As I explained in my answer and as is appropriate in that other referenced answer, you ONLY call `next()` when you are NOT sending a response yourself and when you WANT other middleware or routes to handle the request. In that other answer, they are NOT sending a response when they call `next()`. In fact, they're doing it inside an `if` when they realize that they don't have the necessary information to process the request in this route. – jfriend00 Dec 29 '19 at 04:13
  • Thanks for clarifying. Appreciate your comments. Thanks – user8479984 Dec 29 '19 at 04:33
0

You have two if conditions that mean both might get executed, in both the conditions you are setting the headers and ending the request. Once the request is ended the headers cannot be set. So if you comment [A] which has the return statement and if the second condition is also satisfied, it will set the headers again after sending the request. so it has nothing to do with the next() function or express since there is no next middleware in the stack.

Also the error handling is looks fine. however you can use the node --inspect flag while starting the script and set breakpoint in your middleware to debug entire node code.

Vipul Dessai
  • 180
  • 2
  • 11