es6-shim icon indicating copy to clipboard operation
es6-shim copied to clipboard

Unhandled Promise errors

Open timoxley opened this issue 9 years ago • 15 comments

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 avatar Dec 15 '14 01:12 timoxley

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

timoxley avatar Dec 15 '14 01:12 timoxley

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.

cscott avatar Dec 15 '14 03:12 cscott

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.

ljharb avatar Dec 15 '14 04:12 ljharb

@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.

Yaffle avatar Dec 15 '14 09:12 Yaffle

@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.

cscott avatar Dec 15 '14 15:12 cscott

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

ljharb avatar Dec 15 '14 19:12 ljharb

@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?

Yaffle avatar Jul 01 '15 09:07 Yaffle

@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.

cscott avatar Jul 01 '15 18:07 cscott

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.

AlastairTaft avatar Aug 22 '15 14:08 AlastairTaft

@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.

ljharb avatar Aug 22 '15 16:08 ljharb

@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.

AlastairTaft avatar Aug 22 '15 16:08 AlastairTaft

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

ljharb avatar Aug 22 '15 16:08 ljharb

Fair enough :)

AlastairTaft avatar Aug 22 '15 16:08 AlastairTaft

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

cscott avatar Aug 22 '15 20:08 cscott

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

ertant avatar Oct 25 '15 15:10 ertant