promises-aplus / unhandled-rejections-spec

Discussion and drafts of possible ways to deal with unhandled rejections, across implementations
6 stars 0 forks source link

`done` #5

Open domenic opened 11 years ago

domenic commented 11 years ago

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 commented 11 years ago

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 commented 11 years ago

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 commented 11 years ago

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 commented 11 years ago

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 commented 11 years ago

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