CODEWITHSUNDEEP
node.jsexpress
Eggon
Eggon

Reputation: 2356

Express middleware ignores next(new Error()) and continues execution of the code

I'm throwing an error in my express middleware using next(new Error()) method. Express is supposed to redirect the code to error handling middleware. And it ultimately does, but not until the first middleware is executed(!) to the end.

// MIDDLEWARE FUNCTION
exports.getData = (req, res, next) => {
    Report.findByPk(req.body.ID) // getting data from server with sequelize
        .then(report => {
            if (errorTriggerCondition) { // This condition is met and inside of if statement is executed
                next(new Error('error')); // tried also return next(new Error())
            };
            return report.getData(req.userID); // is executed
        })
        .then(output => {
            res.status(200).send(output); // is executed
        })
        .catch((err) => {
            return next(err);
        });
};

// ERROR HANDLING MIDDLEWARE AT THE END OF SERVER.JS
app.use((err, req, res, next) => {        
    res.send(err);
});

Code is executed as follows:

  1. Data is obtained from server with sequelize

  2. Report is returned correctly, but validity check is not met by the data, so the if statement throwing error is executed.

  3. Despite that the remainder of current then() block is executed and the next as well, in result response is sent.

  4. Finally the code reaches the error handling middleware (and I'm sure it is this particular error) where another error occurs, because it tries to send a response and response has already been sent.

  5. I tried also to use return next(new Error()), but it didn't help.

What can be wrong here? I'm pretty sure this code worked in the past. I did some changes to my app, but I don't see how they could affect this middleware.

Upvotes: 3

Views: 3013

Answers (2)

jakemingolla
jakemingolla

Reputation: 1639

Mixing Promise chains with callbacks can get messy:

        .then(report => {
            if (errorTriggerCondition) { // This condition is met and inside of if statement is executed
                next(new Error('error')); // tried also return next(new Error())
            };
            return report.getData(req.userID); // is executed
        })

In this section, if an error occurred the rest of the Promise chain will still continue. This will eventually result in the res.send() within the Promise chain to occur even though you wanted the error handling middleware to take precedence.

I can think of two options:

  • Make the error check a separate express middleware function so that any errors it generates would never reach the rest of the middleware chain.
  • Use async/await to make this easier to manage with the callbacks

Using async/await, your middleware function would look something like:

async function(req, res, next) {
  try {
    const report = await Report.findByPk(req.body.ID);
    if (errorTriggerCondition) {
      throw new Error('Some informative message used by error middleware');
    }
    const output = await report.getData(req.userID);
    res.status(200).send(output);
    return next();
  } catch (err) {
    return next(err);
  }
}

Upvotes: 1

jfriend00
jfriend00

Reputation: 707178

next(new Error('error') does not stop your function from running. So, the next line of code right after it will still execute. And, if you don't reject the promise, the .then() handlers that follow will also execute.

You will need to properly use if/else or return or throw to stop the following code from executing. In other words, you still have to write code that does proper branching and code flow control.

In the midst of a promise chain, it's probably best to throw to abort the promise chain and let your .catch() handler process the error. The throw will do the right flow control (to stop executing other code) and it will also reject the promise which will stop the other .then() handlers from running. Your .catch() at the end of the chain will then see the error and call next(err) for you.

// MIDDLEWARE FUNCTION
exports.getData = (req, res, next) => {
    Report.findByPk(req.body.ID) // getting data from server with sequelize
        .then(report => {
            if (errorTriggerCondition) { // This condition is met and inside of if statement is executed
                // abort the promise chain, let `.catch()` call next(err)
                throw new Error('error')
            };
            return report.getData(req.userID);   // won't execute on error
        .then(output => {
            res.status(200).send(output);        // won't execute on error
        })
        .catch((err) => {
            next(err);                    // will be executed on error
        });
};

// ERROR HANDLING MIDDLEWARE AT THE END OF SERVER.JS
app.use((err, req, res, next) => {        
    res.send(err);
});

Upvotes: 4

Related Questions