.done() and .ndone() [was: A useful short-cut for express.js]

[ForbesLindesay]

I have been using Q promises with express.js a lot lately, and I'm therefore terminating all promise chains with:

.fail(next).end();

This is because express.js says to pass all unhandled exceptions to next. The end is still necessary in case next throws an exception, which we'd want to just throw globally and exit the program.

It would be really nice if this could become:

.end(next);

Which would just be a shorthand for the first thing?

[kriskowal]

I have considered making .end forward to non-catching callbacks, exiting the purview of the promise system and returning to plain CPS.

[ForbesLindesay]

So it would call next(null, promiseValue) if promise just exited normally? I guess some might exect that behaviour, whereas my suggestion only really applies to express, hmm.

[kriskowal]

Oh, not necessarily Node’s CPS style. .end(callback, errback) is what I have in mind, and both callback and errback would not consume exceptions. A different method that explicitly mentions Node would be appropriate for transferring control to a node callback. I don’t think we have one of those yet either.

[ForbesLindesay]

ah, yeh I like that suggestion, both for a node-end and for a .end(callback, errback) assuming both could be null/undefined. For a node end, a really nice pattern could then be:

function usePromisesIfYouWant(in, cb) {
    return promisedAPI(in).endNode(cb);
}

That way if cb is undefined, we return a promise for someone to use, but if it is defined, we call it in the normal node style.

[domenic]

That way if cb is undefined, we return a promise for someone to use, but if it is defined, we call it in the normal node style.

That is really cool.

[ForbesLindesay]

If we agree that both these methods would be useful end and endNode then shall we add some tests and code and get implementing?

Can we agree what the methods should be called? I think end is fairly obvious, but endNode or nodeEnd ? Neither is perfect, but neither is terrible so I'm open to suggestions personally.

[domenic]

Oh, not necessarily Node’s CPS style. .end(callback, errback) is what I have in mind

This is the same as WinJS's .done and pretty similar to jQuery's done (which just doesn't have an errback). Should we consider renaming it?

[ForbesLindesay]

+1 for done over end if it matches other libraries

[ForbesLindesay]

We could do done(callback, errback) and end(nodeCallback) ?

[domenic]

I'd say .done(callback, errback), .nodeCallback(nodeCallback), and maybe keep .end() for compat? So far we've been good about keeping node idioms prefixed.

Maybe .nend(nodeCallback), but it's not really the same as .end(), whose purpose is to guarantee errors are never silenced---if you did .nend(function () { }), you'd silence the error.

@kriskowal, thoughts? Agreed on .done()?

[ForbesLindesay]

Yes, although .fail(function () {}).end() would silence all errors, nothing can guarantee errors are never silenced. The key point is that .nend(function (err) { if (err) throw err;}) would not silence the error. On the other hand, I'm happy to keep things moderately verbose if that makes it clearer what's going on, so perhaps .nodeCallback or we could go for .ndone as it's very similar to the new .done method

[domenic]

After sleeping on it for a few days, I like .done(callback, errback) and .ndone(nodeCallback). We'd deprecate .end (and remove in the next major).

[domenic]

Some initial work here. https://github.com/domenic/q/compare/done

[ForbesLindesay]

Should it be:

when(promise, fulfilled, rejected).fail(onUnhandledError);

instead of:

when(promise, fulfilled, rejected || onUnhandledError);

The idea is to throw exceptions even if they happen in the fulfilled or rejected handlers.

[domenic]

Yup, that makes sense. I need a test "when the promise is fulfilled / and the callback throws / it should rethrow the error in the next turn", and then I need to implement all three of the "it should rethrow the error in the next turn" tests.

[ForbesLindesay]

And another for 'when the promise is rejected, and the errback throws, it should rethrow the error in the next turn'

[medikoo]

Just to share my experiences. I'm working as well on promise implementation - Deferred, and once I've approached same problems, the way I've solved them, works well on my side:

Currently I have three functions that have same signature as then

• then(callback, errback) - As we know it, extends the chain by returning newly created promise
• aside(callback, errback) - Returns input promise, does not create a new one and doesn't end the chain. It's useful when we want to return promise for further processing but also need to do something with result on the side.
• end(callback, errback) - Ends promise chain. doesn't return anything and doesn't create any new promise. Both callbacks are optional. If errback is not provided eventual error will throw. As I think of it now done name also describes very well this case.

So currently more often I use end or aside than then as in many cases I don't need promises returned by then, it's much cleaner and definitely more performant.

For bridging Node CPS I have cb function, that takes callback, and calls it in Node.js style. Callback is optional and if provided it is run in next tick the earliest, this function also returns input promise, so it's handy when building hybrid functions that both return promise and handle Node.js style callback, e.g.:

var asyncFunc = function (a, b, cb) {
    // ...
    return promise.cb(cb);
};

[ForbesLindesay]

How does this relate to nend etc. ?

[medikoo]

@ForbesLindesay not sure what you're asking, but as I mentioned for transforming back to Node CPS style Deferred has promise.cb.

When talking about nend specifically, I see that nend additionally creates and returns new promise which would be resolved right before callback is called. I have problems understanding what use case does it address, as technically all that information (if needed) is already accessible on input promise.

[domenic]

My original plan was for ndone to replace nend. But as @medikoo points out, nend returns a promise. This, I believe, is designed to enable APIs like that of node-flurry, like so.

[kriskowal]

nend(cb) does return a promise but it’s only to make it easier to test. I might remove it and use waitsFor and a spy in the spec.

[kriskowal]

@domenic I need to read this thread top-to-bottom again. I haven’t made a decision, and have not committed to nend and done and ndone are still on the table.

[ForbesLindesay]

I do like the idea of having an easy way to make my public interfaces directly support node-style and promise-style use just by either passing or not passing a callback.

[domenic]

To summarize and bring in some thoughts:

.done(f, r, p)

Acts like then except that it does not return anything, and causes a next-tick-throw upon rejection. Equivalent to today's .then(f, r, p).end(). Replaces .end() since .done() is equivalent.

Matches WinJS. Similar to jQuery's .done(f[, f, f, ...]), which notably returns the original promise.

.ndone(cb)

Was envisioned as .then(function (x) { cb(null, x); }, cb).end(), or as .end() alone if no cb is passed.

.nend(cb)

Check out the implementation

Things to note:

• If cb is falsy, it will return the original promise. This is useful as we'll see below.
• The use of nextTick escapes the handler-catchers, so that errors thrown in the callback jump to the top of the stack (Node-style).
• It's not very end-ey: without a callback, it acts nothing like end.

On nend or ndone enabling dual APIs

nend currently enables dual APIs like so:

function asyncFunc(a, b, c, cb) {
    return promise.nend(cb);
}

This is very nice.

If we were to move to ndone as part of a move to done a naive translation no longer works, since

function asyncFunc(a, b, c, cb) {
    promise.ndone(cb);
    return promise;
}

will throw-in-next-tick if promise ends up being rejected, denying any promise-using callers a chance to handle rejections. This makes me think ndone is pretty useless.

Recommendation

Whereas,

• nend as it exists is awesome, but it is named confusingly since it doesn't behave like end at all;
• done is still a good idea and a good replacement for end;
• ndone turns out to be useless;

I think we should replace end with done and do nothing else. Keep nend named as it is, since there will no longer be a end to confuse matters. And forget about ndone.

[kriskowal]

nend needs a different name. I’ll buy done as you propose.

[kriskowal]

nend stands for "if the user provides a nodeback, transfer control to Node-style continuation passing. otherwise, return the promise, so this function can be used by both promise consumers and nodeback providers". That’s a mouthful and might be hard to capture, but I’ll buy up to three words and no more than 80 characters.

[kriskowal]

Some potential seeds for the name:

• cb
• toNode
• toNodeback
• maybeToNodeback
• ambidextrous
• ambi

[tlrobinson]

• hermaphrodite

[kriskowal]

• honeyBadger
• nodeCompatible

[domenic]

• nodeify
• maybeNodeify

[kriskowal]

• supportNode

[ForbesLindesay]

+1 for supportNode +1 for nodeify +0.5 for toNodeback

In the use case described of "I wish to expose an api that's familiar to people used to node style callbacks" I'd go with supportNode as the hands-down winner.

For the alternative case of working with things like express.js where you must call a callback as a node.js callback, it seems a little counter-intuitive to call it supportNode, which makes me favour something like nodeify.

toNodeBack seems nice and un-offensive, but could be confused with Deffered's makeNodeResolver.

[rstacruz]

For those coming in via a Google search, let me save you some reading:

• This has been merged and is now called promise.nodeify().
• Calling .nodeify(done) is the the equivalent of .then(function(data) { done(null, data); }, done).
• There's also promise.done(), where .done(x,y,z) is the same as .then(x,y,z).done().

Express.js example:

function (req, res, next) {
  Q.when(Posts.findAll())
  .then(function() {
    // Do things
  })
  .nodeify(next);
});

Mocha.js example:

it("should work properly", function(done) {
  Q.when(Posts.findAll())
  .then(function(posts) {
    assert.equal posts.length, 40
  })
  .nodeify(done);
});