1

I'm using async await in my NodeJs code and the code structure is something as follows.

async function main(){
    try {
        await someFunctionThatReturnsRejectedPromise()
    } catch(e) {
        console.log(e)
    }
}

async function someFunctionThatReturnsRejectedPromise() {
    try {
        await new Promise((resolve,reject) => {
            setTimeout(() => {
                reject('something went wrong')
            }, 1000);
        })
    } catch(e) {
        return Promise.reject(e)
    } finally {
        await cleanup() // remove await here and everything is fine
    }
}


function cleanup() {
    return new Promise(resolve => {
        setTimeout(() => {
            resolve('cleaup successful')
        }, 1000);
    })
}

main();

In the finally block, I'm doing some async cleanup that will surely resolve. But this code is throwing PromiseRejectionHandledWarning

(node:5710) UnhandledPromiseRejectionWarning: something went wrong
(node:5710) UnhandledPromiseRejectionWarning: Unhandled promise rejection. This error originated either by throwing inside of an async function without a catch block, or by rejecting a promise which was not handled with .catch(). (rejection id: 1)
(node:5710) [DEP0018] DeprecationWarning: Unhandled promise rejections are deprecated. In the future, promise rejections that are not handled will terminate the Node.js process with a non-zero exit code.
something went wrong
(node:5710) PromiseRejectionHandledWarning: Promise rejection was handled asynchronously (rejection id: 1)

As far as I understand, I'm not leaving any promise unhandled here. What am I doing wrong? Should finally block by synchronous by design? If yes, why so?

Update 1:

If I convert someFunctionThatReturnsRejectedPromise to good ol' then and catch, it works with no problems:

function someFunctionThatReturnsRejectedPromise() {
    return (new Promise((resolve,reject) => {
        setTimeout(() => {
            reject('something went wrong')
        }, 1000);
    })).catch(e => {
        return Promise.reject(e)
    }).finally(() => {
        return cleanup()
    })
}

Update 2: (Understood the problem)

If I await the returning Promise, problem is solved.

 return await Promise.reject(e)

And this makes me understand what I was doing wrong. I was breaking the await chain (partially synonymous to not returning a Promise in then/catch syntax). Thanks everyone :)

4
  • The "DeprecationWarning" is thrown because you're not providing a rejection strategy for your Promise in cleanup. You should never use the Promise constructor (unless in some very specific very rare cases). You should add a try-catch on await cleanup() as well to handle a possible unhandled rejection.
    – briosheje
    Apr 21, 2020 at 7:31
  • @briosheje as I mentioned, "cleanup that will surely resolve" Think of it as a function that never throws/rejects Apr 21, 2020 at 7:34
  • Once again: as long as an await is involved, a Promise is created. If you don't handle the rejection of that promise, node will give you such warning. This is intended, because the promise might be rejected for any kind of error (whatever generic JAVASCRIPT error, not necessarely promise-related). You must either remove the await, either surround the block with a try-catch.
    – briosheje
    Apr 21, 2020 at 7:39
  • @briosheje even if I handle cleanup, issue still exists Apr 21, 2020 at 7:50

2 Answers 2

4

When a Promise rejects, it must be handled before the current call stack clears, or there will be an unhandled rejection. You have:

} catch (e) {
  return Promise.reject(e)
} finally {
  await cleanup() // remove await here and everything is fine
}

If you remove await, the someFunctionThatReturnsRejectedPromise returns immediately after the rejected Promise is constructed, so the Promise.reject(e), the rejected Promise, is caught by the catch in main right after. But if there's any delay, the rejected Promise will not be handled immediately; your await cleanup() will mean that the rejected Promise goes unhandled for a period of time, before someFunctionThatReturnsRejectedPromise returns, which means that main's catch can't handle the rejected Promise in time.

Another method you could use would be to wrap the error in an Error instead of a Promise.reject, and then check if the result is an instanceof Error in main:

window.addEventListener('unhandledrejection', () => console.log('unhandled rejection!'));

async function main() {
  const result = await someFunctionThatReturnsRejectedPromise();
  if (result instanceof Error) {
    console.log('Error "caught" in main:', result.message);
  }
}

async function someFunctionThatReturnsRejectedPromise() {
  try {
    await new Promise((resolve, reject) => {
      setTimeout(() => {
        reject('something went wrong')
      }, 1000);
    })
  } catch (e) {
    return new Error(e);
  } finally {
    await cleanup()
  }
}


function cleanup() {
  return new Promise(resolve => {
    setTimeout(() => {
      resolve('cleaup successful')
    });
  })
}

main();

4
  • not sure I understand this correctly. Javascript is working on a single thread. When you say "Promise goes unhandled for a period of time", what does it mean? The interpreter is still in the function. Function is still being executed. How can Node take control of a Promise that is yet to be returned? Also, if you see, if I convert someFunctionThatReturnsRejectedPromise to then and catch, it works with no problems. try, catch and finally are just syntax, and they should mean the same, isn't it? Apr 21, 2020 at 7:47
  • The interpreter is still in the function Not exactly - control flow is inside someFunctionThatReturnsRejectedPromise during the time between when the rejected Promise is created and cleanup's setTimeout is queued, but then control is yielded back, and other tasks may run in the meantime. Javascript execution does not completely stop when an await is encountered - it only pauses the current function that the await is in. Apr 21, 2020 at 7:54
  • See this fiddle for an example of other JS code running while a Promise is being awaited Apr 21, 2020 at 7:56
  • thanks for your time :) I get it partially, but still unsure about why is this happening, will study more on this Apr 21, 2020 at 8:07
1

Updated answer replace the

Promise.reject(e) with throw e;

so the function becomes

async function someFunctionThatReturnsRejectedPromise() {
   try {
       await new Promise((resolve,reject) => {
           setTimeout(() => {
               reject('something went wrong')
           }, 1000);
       })
   } catch(e) {
       throw e;
   } finally {
       await cleanup() // remove await here and everything is fine
   }
}

Reason

someFunctionThatReturnsRejectedPromise method rejects the Promise first. So the control flow went to method main catch block. Later cleanup method tries to do the same. Which is to reject the already rejected promise. Thus you get the error

Promise.reject is a bit different from throw clause. Please refer throw vs Promise.reject

Which is why removing the await from cleanup() or removing the return from cleanup method works. Because that will detach the Promise from the current control flow.

2

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.

Not the answer you're looking for? Browse other questions tagged or ask your own question.