1

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?

Victor Ferreira
  • 6,151
  • 13
  • 64
  • 120
  • 1
    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. – jfriend00 Feb 02 '18 at 21:12
  • 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 – Victor Ferreira Feb 02 '18 at 21:16

1 Answers1

4

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):

promise then promise then-catch

Patrick Roberts
  • 49,224
  • 10
  • 102
  • 153
  • Thanks for this answer. But if I can't call next again I can't really use my error middleware, right?! – Victor Ferreira Feb 02 '18 at 21:14
  • @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](https://blogs.msdn.microsoft.com/ericlippert/2008/09/10/vexing-exceptions/) for explanation of the terminology. – Patrick Roberts Feb 02 '18 at 21:17
  • 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 – Victor Ferreira Feb 02 '18 at 21:21
  • 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. – Patrick Roberts Feb 02 '18 at 21:22
  • 2
    Yes! This is a prime example of why `.then(successHandler, errorHandler)` is pattern, not anti-pattern as it is often dubbed. – Roamer-1888 Feb 02 '18 at 21:42