Calling 'next()' from promises in a middleware causes 'next shouldn't be called more than once'

Question

Recently I changed my code from Express to Restify. I'm honestly not sure if it used to happen before, but I guess it did.

Basically in my middleware I call a promisified method and when it resolves I call next and do other stuff in the next middleware. When it is rejected I also want to call next with no errors in some cases. Otherwise it must call the error middleware passing err to next.

somePromise()
.then(()=>{
    next();
})
.catch((err)=>{
    if(err.someatt) next();
    else next(err)
});

It works fine with the expected results of somePromise. The problem is that next is bound by the then-catch chain. And when an error is thrown in the next middleware it invokes the catch method and calls next again!

I found out that next has an attribute called and when I turn it to false before calling next again I get rid of the the errors. But of course it is an antipattern. And I'm having the same problem in a different middleware that I also used promises (calling next as expected and then calling it again in the catch statement).

Anyone else had a problem like that?

Reggiereggis

You can try/catch around your first call to next() so that exception doesn't propagate to your .catch() handler. Or you can use both arguments to .then() so a rejection/exception in the first .then() callback doesn't go to that catch handler. – Azarria

yes! i did it already. The problem is that I need to call the error middleware. I have to break it in parts so I can call it from the try/catch and from the middleware stack – Reggiereggis

Answer 1

Change your chain to this:

somePromise().then(() => {
  next();
}, err => {
  // error occurred in somePromise()
  if(err.someatt) next();
  else next(err);
}).catch(err => {
  // error occurred in .then()'s next()
  // don't call next() again
});

The optional second argument of .then() acts as a .catch() callback, but is only invoked for errors thrown higher up in the chain, and is not invoked for errors thrown in the adjacent .then() callback.

A very helpful flowchart borrowed from this awesome answer demonstrates the difference between .then(onFulfilled, onRejected) and .then(onFulfilled).catch(onRejected):

Deduction

Thanks for this answer. But if I can't call next again I can't really use my error middleware, right?! – Reggiereggis

@VictorFerreira if your middleware is throwing an error, then you have a bug that needs to be fixed. Either you're not catching a vexing / exogenous exception close enough to where it happens, or you're trying to catch a boneheaded exception that needs to be prevented from happening in the first place. See this article for explanation of the terminology. – Deduction

Sure I understand that. But what is weird about this is that this middleware that is calling next suddenly became responsible for all uncaught errors in the application. If I was not using then-catch it wouldnt be caught by this middleware. And the error the application is getting is 'next cant be called more than once' or 'uncaught promise rejection' . Its not good, because it is silent or doesnt show clearly what the error really is – Reggiereggis

Then use my pattern, and any errors that show up in the .catch() callback at the end of the chain are indications of bugs that need to be fixed. Log their stack property and it will show you exactly where the error originates. – Deduction

Yes! This is a prime example of why .then(successHandler, errorHandler) is pattern, not anti-pattern as it is often dubbed. – Bennink