Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Unhandled Promise errors #311

Open
timoxley opened this issue Dec 15, 2014 · 15 comments
Open

Unhandled Promise errors #311

timoxley opened this issue Dec 15, 2014 · 15 comments

Comments

@timoxley
Copy link

Currently, if there's no .catch errors will vanish mysteriously. Native promises in chrome 41.0.2249.0 (current canary) will print a message about unhandled promise errors:

image

but if the es6-shim is loaded, it clobbers the native Promise because it doesn't seem to support subclassing, and replaces it with an insidious version with total error suppression:

image

Not sure what the best approach is to solve this.

@timoxley timoxley changed the title Promise error handling Unhandled Promise errors Dec 15, 2014
@timoxley
Copy link
Author

Not even sure if chrome implementation is valid, i.e. are you supposed to be able to attach catch handlers asynchronously?

@cscott
Copy link
Collaborator

cscott commented Dec 15, 2014

Yes, you are supposed to be able to attach catch handlers asynchronously. But I think Chrome is supposed to remove the message from console if that happens, or something like that?

It would be nice to chain through native promises. The most straightforward way to do that would be to subclass them.... but of course that's what Chrome isn't supporting yet. Boo.

@ljharb
Copy link
Collaborator

ljharb commented Dec 15, 2014

I wonder if it would be worth it, in the case where there's a native Promise that doesn't support subclassing but otherwise works, to use an alternative Promise shim that is a subclassable proxy wrapper around the native one? If that worked, it should preserve this behavior.

At any rate, the Chrome behavior of console warning when you forget .catch isn't part of the spec, and is something the browser is choosing to do, so it's really not the shim's responsibility to support or maintain that functionality.

@Yaffle
Copy link
Contributor

Yaffle commented Dec 15, 2014

@ljharb ,
Did you try to output an error with console.log or console.error ?
I tried, but it is printed without "stack", so it is not easy to jump to a source code.
Although, I think, the console API should allow this, because it is useful when you have "onReject", but you do not handle all kinds of errors inside.

@cscott
Copy link
Collaborator

cscott commented Dec 15, 2014

@ljharb what's the status of proxy support in browsers?

My initial thought was that maybe our Promise implementation could attach a "native" promise to the end of its promise chain, and then remove it as soon as then/catch was called (since there would presumably be a new native promise chained after that). This ought to preserve the browser's "uncaught exception" behavior, although perhaps it wouldn't capture the stack. It would also have a performance impact, although perhaps that would be slight.

@ljharb
Copy link
Collaborator

ljharb commented Dec 15, 2014

I don't think anything has proxies yet. That's definitely a use case for it though.

@Yaffle
Copy link
Contributor

Yaffle commented Jul 1, 2015

@ljharb I cannot understand this issue, but

  var x = Promise.reject(new Error("!"));
  setTimeout(function () {
    x.then(undefined, function (e) {
      console.log(e);
    });
  }, 0);

this causes a error message in the log in Chrome. And if replace setTimeout with Promise.resolve(undefined) - nothing will be printed to the log.
So to simulate same behavior you could use setTimeout, right?

@cscott
Copy link
Collaborator

cscott commented Jul 1, 2015

@Yaffle yes, many utility libraries for Promises (such as http://github.com/cscott/prfun) add a Promise.prototype.done method which uses setTimeout in exactly this way.

There's also a web spec in-process for triggering the "uncaught promise exception" handler in the same way that there is an "uncaught exception" handler in the web stack for synchronous code. I don't know if Chrome has exposed this to users yet, though. If they have, we could hook it.

@AlastairTaft
Copy link

The try catch around line 1867 is currently eating all my Promise errors (in the promiseReactionJob method). All my promise errors are caught and not thrown so it fails silently. I'm going to have to run with a flag to not include the es6-shim when developing in chrome. That also makes me wonder that the shim shouldn't be overriding native chrome promise support.

@ljharb
Copy link
Collaborator

ljharb commented Aug 22, 2015

@AlastairTaft I'm not sure what you mean - that's what native Promises do by spec. All Promises swallow all errors, and the only way to catch them is with a .catch.

The additional browser behavior of notifying the console when you have an unhandled rejection is not part of the spec, it's bonus behavior some promises provide.

@AlastairTaft
Copy link

@ljharb If it's by design then no worries. I'll provide some code though to explain my point a bit better just encase it is a problem.

So I'm doing something like this in my code

var self = this
// Get the content
Promise.all([
  api.client.getProfileP(),
  api.client.getInfoSummariesP()
])
.then(function(args){
  var profile = args[0]
    , infoSummaries = args[1]

  self.loadContent_(profile, infoSummaries)
})
.catch(function(err){
  throw err
})

When I hit a run time error in the self.loadContent_(profile, infoSummaries) method the throw err is swallowed and nothing is output to the console.

@ljharb
Copy link
Collaborator

ljharb commented Aug 22, 2015

Correct. You can never throw an error outside of a Promise - that's how they work everywhere. You can do setTimeout(function () { throw err; }, 0) but that's basically your only option.

Handle the error in the right place: inside the promise :-)

@AlastairTaft
Copy link

Fair enough :)

@cscott
Copy link
Collaborator

cscott commented Aug 22, 2015

The prfun library (and others) have a Promise #done method that can also be helpful in making your thrown errors visible.

@ertant
Copy link

ertant commented Oct 25, 2015

At least for debug versions I have changed the rejectPromise (line 1913) function like below, otherwise finding an error almost impossible.

var rejectPromise = function (promise, reason) {
   .....
   if(!reactions.length) {
      console.warn('Uncaught (in promise)', reason, reason.stack);
   }
   triggerPromiseReactions(reactions, reason);
};

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

No branches or pull requests

6 participants