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

done #5

Open
domenic opened this issue Dec 11, 2012 · 5 comments
Open

done #5

domenic opened this issue Dec 11, 2012 · 5 comments
Labels

Comments

@domenic
Copy link
Member

domenic commented Dec 11, 2012

The solution employed by Q and WinJS is a function called done on each promise, which is much like then except it signals that if there is an unhandled rejection for that promise, the program should crash. That is:

var f = fulfilledPromise();
var r = rejectedPromise(new Error("foo!"));

var noop = function () { };
var thrower = function () { throw new Error("bar!"); }

r.done(); // crashes with "foo!"
r.done(undefined, function () { }); // doesn't crash
r.done(undefined, thrower); // crashes with "bar!"

f.done(); // doesn't crash
f.done(thrower); // crashes with "bar!"

Notably, p.done(f, r) is equivalent to p.then(f, r).done(), so you can choose either style as convenient; the latter is a bit nicer for refactoring, but the former signals intent nicely in other situations.

Also, done returns undefined.

The guidance is that when creating a promise (either directly or via then), you should always either (a) return that promise so that the caller takes care of it, or (b) call .done() on it.

For more info on this see the MSDN article: http://msdn.microsoft.com/en-us/library/windows/apps/hh700337.aspx


Notably, this idea is composable with other ideas: if you "cap" a promise with .done(), that handles the rejection immediately with a sort of "default behavior" (i.e. crashing). So the other ideas can still be implementing to catch cases where you forget to cap.

In other words, this idea reduces the number of unhandled rejections present in a program, and with inhuman programmer diligence brings that number down to zero. But in cases where that number is not brought down to zero, other ideas still apply.

@ForbesLindesay
Copy link
Member

This works well, although it can be frustrating when you forget to add a .done(). I think it could combine nicely with #2, which could even be modified so that we tracked missing .done() handlers even if the promise wasn't rejected. That way #2 could help catch the cases where you forget to add .done()

@domenic
Copy link
Member Author

domenic commented Dec 11, 2012

Static analysis may also help catch missing .done()s, especially if we had a foolproof way of determining that an object was a promise (like TypeScript!). I can't imagine trying to work out the edge cases there though.

@ForbesLindesay
Copy link
Member

hmm, but if any object that has a method .then on it is treated as a promise, we know it's a promise if you ever call .then on it, which should propagate through quite a long way with fairly minimal effort. Could be implemented as an extension to jshint

@briancavalier
Copy link
Member

I'm strongly against requiring something like done. I think it introduces extra cognitive load (I have to think about whether to use then or done), and refactoring hazards (forgetting to add or remove done). Even though the refactoring hazard can be reduced a bit by always doing .then(f, r, p).done() instead of .done(f, r, p), that doesn't completely remove the hazard. For example, you can't simply add/remove a return anymore--you have to think about whether done needs to be added/removed as well.

Also, it seems impossible to introduce done in a debugging-only way. People simply will tend to sprinkle it through their code, and will not remove it, so any promise that doesn't support done will break their code. That means that done would need to be promoted to the same status as then in order for anyone to use it in an interoperable way.

Personally, I'm much more interested in something like #2.

@ForbesLindesay
Copy link
Member

I think we need something to help support going back to node-style callbacks. Since done style callbacks crash on error, there needs to be a way of escaping promises.

It doesn't need to hit quite the level that .then does, because unlike then it's easilly shimmed

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

No branches or pull requests

3 participants