Skip to content
This repository has been archived by the owner on Dec 3, 2019. It is now read-only.

Uncaught exceptions thrown not caught #473

Closed
catapult-bot opened this issue Aug 1, 2015 · 7 comments
Closed

Uncaught exceptions thrown not caught #473

catapult-bot opened this issue Aug 1, 2015 · 7 comments
Labels

Comments

@catapult-bot
Copy link

Issue by natduca
Monday Sep 22, 2014 at 20:20 GMT
Originally opened as google/trace-viewer#473


From [email protected] on September 30, 2013 16:16:50

Try patching in this https://codereview.appspot.com/13831043 Then break on exceptions. You'll see an exception is being thrown. Test is still passing.

Original issue: http://code.google.com/p/trace-viewer/issues/detail?id=468

@catapult-bot
Copy link
Author

Comment by natduca
Monday Sep 22, 2014 at 20:20 GMT


From [email protected] on September 30, 2013 16:21:29

Ah the issue is subtle:

test(function() {
var p = new Promise(function(r) {
setTimeout(function() {
r.resolve();
}, 10);
});
p.then(function() {
throw new Error('blam');
});
return p;
});

The problem here is that the promise returned (p) is resolved. We have an exception in a then clause of a "dangling" promise that makes this fail.

This is almost like the async version of an uncaught exception?

@catapult-bot
Copy link
Author

Comment by natduca
Monday Sep 22, 2014 at 20:20 GMT


From [email protected] on September 30, 2013 16:27:35

To be fair, i should mark available. :)

Status: Available
Owner: ---
Cc: [email protected]

@catapult-bot
Copy link
Author

Comment by natduca
Monday Sep 22, 2014 at 20:21 GMT


From [email protected] on September 30, 2013 16:54:48

Here is how I see it.

The interface to test() is a function that may return a Promise. If it does return a Promise, unittest will call then() on it and take action appropriate to pass or fail.

The code in comment #2 returns a Promise, but before resolve() it forks the promise. When resolve() is called it tells unittest "we're good here" and unittest is done. The other fork throws, but it's too late.

I don't think you should fork the promise you return to unittest.

I think you want something more like:

test(function() {
return new Promise(function(testResolver) {
var p = new Promise(function(r) {
setTimeout(function() {
r.resolve();
}, 10);
});
p.then(function() {
testResolver.reject('blam');
});
});
});

This case should async fail unconditionally.

@catapult-bot
Copy link
Author

Comment by natduca
Monday Sep 22, 2014 at 20:21 GMT


From [email protected] on September 30, 2013 17:03:02

Yes, that is correct. But there is a dangling, uncaught exception if you screw this up, and you wont catch it because absolutely nothing is done with this uncaught one. It seems like these dangling ones should cause test failure --- e.g. if a promise rejects and has no handler, it should be treated like a window.onerr?

@catapult-bot
Copy link
Author

Comment by natduca
Monday Sep 22, 2014 at 20:21 GMT


From [email protected] on September 30, 2013 18:22:11

Yes I agree that your original case should give some output. Promises simply swallows the exception. I don't see a way to solve this in the unittest.js. slightlyoff/Promises#74

@catapult-bot
Copy link
Author

Comment by natduca
Monday Sep 22, 2014 at 20:21 GMT


From [email protected] on September 30, 2013 23:48:36

Awesome! I think my naiéve thought here is that we want promise.js polyfill to at least fire some basic Promise.unhandledErrorHandler when nobody traps the error case so that we can then tie that to an overall test failure...

@catapult-bot
Copy link
Author

Comment by natduca
Monday Sep 22, 2014 at 20:21 GMT


From [email protected] on April 08, 2014 12:39:59

Fixed with tvcm overhaul which included unittest overhaul

Status: Fixed

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

No branches or pull requests

1 participant