Promise .finally gives UnhandledPromiseRejection even if .catch has been used. #28636

Activity

1. Hakerh400 commented on Jul 11, 2019

   

   Hakerh400

   on Jul 11, 2019

   Last edited by Hakerh400

   Contributor

   More actions

   That is how promises work. You defined catch and finally handlers on the same promise. The promise then follows both "paths" when resolving/rejecting. One path catches it, but the other contains only finally, making the rejection pass through it, then leading to unhandled rejection. What you probably want is to define finally on the promise returned by the catch call, for example:

   const p = Promise.reject(new Error('BOOM'));
   p.catch(() => {}).finally(() => {});

   Similar question was asked before: nodejs/help#1208

   👍React with 👍5Reacted by Jeremiah Senkpiel, AgBorkowski, laggingreflex, Nick and August Lilleaas
2. Fishrock123

   added

   promisesIssues and PRs related to ECMAScript promises.Issues and PRs related to ECMAScript promises.

   on Jul 12, 2019
3. MuraliSRao commented on Jul 13, 2019

   

   MuraliSRao

   on Jul 13, 2019

   More actions

   Catch is simply to catch the errors.
   As a defintion of finally, - the finally block must excute at least once in whatever situation.
4. targos commented on Jul 21, 2019

   

   targos

   on Jul 21, 2019

   Member

   More actions

   @Hakerh400's explanation is correct.
5. targos

   closed this as completedon Jul 21, 2019
6. jedwards1211 commented on Jun 15, 2021

   

   jedwards1211

   on Jun 15, 2021

   Last edited by jedwards1211

   More actions

   @targos I just ran into this and it was a really nasty surprise, had me staring at my screen thinking wtf for 30 minutes, even though I've been using promises for years and years. I think the intended behavior could be changed to be more humane.

   Is there no way that node could internally flag the promise returned by .finally such that it won't trigger an unhandled rejection like other promises, as long as the finally handler doesn't throw its own error? It would alleviate a lot of suffering.

   If there's no catch the original promise would still trigger an unhandled rejection as desired. But unhandled rejections coming from a non-throwing .finally don't provide any concrete benefit that I can imagine. I think they just cause harm.

   👍React with 👍2Reacted by Lays Dragon and eklenin
7. targos commented on Jun 16, 2021

   

   targos

   on Jun 16, 2021

   Member

   More actions

   I don't know if Node.js could do something like that, but I find the behavior consistent with the equivalent synchronous code:

   try {
     throw new Error('boom')
   } finally {
     console.log('hello')
   }

   This will log "hello" and also leave an unhandled exception.

   👍React with 👍1Reacted by Timo Tijhof
8. jedwards1211 commented on Jun 17, 2021

   

   jedwards1211

   on Jun 17, 2021

   Last edited by jedwards1211

   More actions

   @targos yes I know, but consider this difference -- unlike sync code where you only need to catch an error once, with async code you can end up having to catch the same error multiple times to prevent unhandled rejection, even if only one of those catch handlers actually needs to do anything about it. .finally() can make the same error rear its head after you've already handled it elsewhere.

   I realize now, the problem can happen with .then() too. I wish Node would just help me make sure I catch a given error at least once, rather than every place it gets "passed through" to the next link in the promise chain.

   Because if I create a dead-end .finally(), I was intending to respond to errors elsewhere, so the error handler for the dead-end is guaranteed to be a no-op.

   👍React with 👍1Reacted by Lays Dragon