await in finally block of async function causes PromiseRejectionHandledWarning

Question

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

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 '20 at 07:31
@briosheje as I mentioned, "cleanup that will surely resolve" Think of it as a function that never throws/rejects — pranavjindal999, Apr 21 '20 at 07: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 '20 at 07:39

score 4 · Answer 1 · answered Apr 21 '20 at 07:35

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();

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? — pranavjindal999, Apr 21 '20 at 07: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. — CertainPerformance, Apr 21 '20 at 07:54
See [this fiddle](https://jsfiddle.net/pyx126t8/) for an example of other JS code running while a Promise is being `await`ed — CertainPerformance, Apr 21 '20 at 07:56
thanks for your time :) I get it partially, but still unsure about why is this happening, will study more on this — pranavjindal999, Apr 21 '20 at 08:07

Viswanath Lekshmanan · Answer 2 · 2020-04-21T08:25:02.947

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.

Updated with reason. Please check – Viswanath Lekshmanan Apr 21 '20 at 08:25 — Viswanath Lekshmanan, Apr 21 '20 at 08:25

await in finally block of async function causes PromiseRejectionHandledWarning

2 Answers2