Skip to content
This repository has been archived by the owner on Feb 2, 2023. It is now read-only.

How to handle "unhandled Promise rejections" #72

Closed
bminer opened this issue Oct 20, 2015 · 20 comments
Closed

How to handle "unhandled Promise rejections" #72

bminer opened this issue Oct 20, 2015 · 20 comments

Comments

@bminer
Copy link

bminer commented Oct 20, 2015

Promises can be "handled" after they are rejected. That is, one can call a promise's reject callback before providing a catch handler. This behavior is a little bothersome to me because one can write...

var promise = new Promise(function(resolve) {
    kjjdjf(); // this function does not exist
});

... and in this case, the Promise is rejected silently. If one forgets to add a catch handler, code will continue to silently run without errors. This could lead to lingering and hard-to-find bugs.

In the case of Node.js, there is talk of handling these unhandled Promise rejections and reporting the problems. This brings me to ES7 async/await. Consider this example:

async function getReadyForBed() {
  let teethPromise = brushTeeth();
  let tempPromise = getRoomTemperature();

  // Change clothes based on room temperature
  let temp = await tempPromise;
  // Assume `changeClothes` also returns a Promise
  if(temp > 20) {
    await changeClothes("warm");
  } else {
    await changeClothes("cold");
  }

  await teethPromise;
}

In the example above, suppose teethPromise was rejected (Error: out of toothpaste!) before getRoomTemperature was fulfilled. In this case, there would be an unhandled Promise rejection until await teethPromise.

My point is this... if we consider unhandled Promise rejections to be a problem, Promises that are later handled by an await might get inadvertently reported as bugs. Then again, if we consider unhandled Promise rejections to not be problematic, legitimate bugs might not get reported.

Thoughts on this?

This is related to the discussion found in the Node.js project here: nodejs/node#830

@ljharb
Copy link
Member

ljharb commented Oct 20, 2015

If I'm reading it correctly, your example code desugars to something like the following:

function getReadyForBed() {
  let teethPromise = brushTeeth();
  let tempPromise = getRoomTemperature();

  // Change clothes based on room temperature
  return Promise.resolve(tempPromise)
    .then(temp => {
      // Assume `changeClothes` also returns a Promise
      if (temp > 20) {
        return Promise.resolve(changeClothes("warm"));
      } else {
        return Promise.resolve(changeClothes("cold"));
      }
    })
    .then(teethPromise)
    .then(Promise.resolve()); // since the async function returns nothing, ensure it's a resolved promise for `undefined`, unless it's previously rejected
}

When getReadyForBed is invoked, it will synchronously create the final (not returned) promise - which will have the same "unhandled rejection" error as any other promise (could be nothing, of course, depending on the engine). (I find it very odd your function doesn't return anything, which means your async function produces a promise for undefined.

If I make a Promise right now without a catch, and add one later, most "unhandled rejection error" implementations will actually retract the warning when i do later handle it. In other words, async/await doesn't alter the "unhandled rejection" discussion in any way that I can see.

@bterlson
Copy link
Member

I think this concern is about promises not about async functions. Proposals for unhandled rejection tracking exist, eg here: https://github.com/domenic/unhandled-rejections-browser-spec/ (and a proposal to add the appropriate machinery to test262 is here: tc39/ecma262#76).

@bminer
Copy link
Author

bminer commented Nov 4, 2015

@bterlson - Good point. Also, as one of my colleagues pointed out, my example could easily be re-written to avoid this pitfall:

async function getReadyForBed() {
  let teethPromise = brushTeeth();
  let tempPromise = getRoomTemperature();

  // Change clothes based on room temperature
  var clothesPromise = tempPromise.then(function(temp) {
    // Assume `changeClothes` also returns a Promise
    if(temp > 20) {
      return changeClothes("warm");
    } else {
      return changeClothes("cold");
    }
  });
  /* Note that clothesPromise resolves to the result of `changeClothes`
     due to Promise "chaining" magic. */

  // Combine promises and await them both
  await Promise.all(teethPromise, clothesPromise);
}

Note that this should prevent any unhandled promise rejection.

@ljharb
Copy link
Member

ljharb commented Nov 4, 2015

It certainly won't - if either teethPromise or clothesPromise rejects, it will throw, and the promise returned by getReadyForBed will be rejected, and unhandled.

The only way to prevent it is to always remember to add a .catch on any promise chain, including invocations of async functions.

@bminer
Copy link
Author

bminer commented Nov 5, 2015

@ljharb - What do you mean? getReadyForBed is just an async function; it doesn't return a Promise.

EDIT: I meant that we don't care about the Promise returned by calling getReadyForBed. But, to be clear, yes all async functions return Promises.

You are right that if teethPromise or clothesPromise rejects, an exception will be thrown; this is the desired behavior. My point is that no rejection is left "unhandled"; it is thrown instead. My example intends to illustrate that all Promise rejections turn into thrown exceptions instead of a rejection being unhandled (silently ignored), even for a short while.

@ljharb
Copy link
Member

ljharb commented Nov 5, 2015

@bminer all async functions return Promises. If they don't return anything, it returns Promise.resolve(undefined).

@molatif81
Copy link

molatif81 commented Apr 6, 2017

Not sure if this thread is still active. I am getting a similar error and it's preventing our users on IE11 from accessing the site (they click the login button after typing their credentials but nothing happens). I looked at developer tools to see if there's any errors and found the following

"Unhandled promise rejection error: access is denied"

Any ideas?

@madhusudhand
Copy link

if we consider the following example

async function one() {
    await Promise.reject('err');
}

function two() {
    try {
        one();
    } catch (e) {
        console.log(e);   // uncaught
    }
}

When I am trying to handle the error (outside async function), it isn't being caught.
To catch the error, I must put a try-catch inside my async function.

async function one() {
  try {    
    await Promise.reject('err');
  } catch (e) {
     // e caught here
  }
}

Should it not behave as follows?

function one() {
  return Promise.reject('error');
}

function two() {
  one().catch(e => {
     // error caught
  });
}

Promise propagates rejections to its promise chain, where as await stops.

@bminer
Copy link
Author

bminer commented Jun 7, 2017

@madhusudhand - In both examples, function one returns a rejected Promise. When you call one() even with a try/catch block around it, you aren't capturing the error from the Promise; instead, the rejected promise is left unhandled. You must use .catch on the Promise or use await to capture the Error. If you use await in function two, it must also be an async function.

@gur111
Copy link

gur111 commented Jul 6, 2017

I've tried to fix this thing for hours today. The latest version I can use without getting this warning is 5.12.
When will this be fixed?
FYI, I do not use await and I just added .catch to ALL places in the code where there's a get (And still getting this)

@bterlson
Copy link
Member

bterlson commented Jul 6, 2017

@gur111 I don't think you meant to comment here.

@gur111
Copy link

gur111 commented Jul 6, 2017

True. I meant to comment on the github issue. Sorry

@ghost
Copy link

ghost commented Jun 11, 2018

@madhusudhand, the two() function should be also async

async function one() {
    await Promise.reject('err');
}

function async two() {
    try {
        await one();
    } catch (e) {
        console.log(e);   // caught
    }
}

@ghost
Copy link

ghost commented Jun 11, 2018

with async await, you need to respect "await" promise chain to catch errors..

@TeamYok
Copy link

TeamYok commented May 7, 2019

(node:26359) 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: 5)

i have facing this error anybody sloved it please

@ljharb
Copy link
Member

ljharb commented May 7, 2019

@NOMAN-JRK add .catch() on the end of the promise that's failing.

@TeamYok
Copy link

TeamYok commented May 7, 2019

dear sir; please tell me where and which file please tell me name of file🙏🙏

@ljharb
Copy link
Member

ljharb commented May 7, 2019

@NOMAN-JRK please ask on IRC, or stack overflow; this repo is for a completed language proposal.

@akinyelefast
Copy link

I have been facing and trying to resolve the unhandled promise rejection for weeks now.
Please help

error
watch.zip

@JankesYT
Copy link

JankesYT commented Mar 1, 2020

K

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

9 participants