Make always() more sane

[briancavalier]

Right now, promise.always() is weird for a few reasons, several of which were discussed in this google group thread. For instance:

1. It receives either a fulfilled value, or a rejection reason and can't easily distinguish, except by inspecting the value/reason itself. This makes the function passed to always() trickier to implement and potentially more error prone.
2. It is easy to accidentally "handle" a rejection simply by returning. For example, passing the identity function or a function that returns undefined, like console.log to always() will always squelch errors.

One idea would be to try to make always() work more like synchronous finally. Consider the following code:

try {
  return doSomething(x);
} catch(e) {
    return handleError(e);
} finally {
    cleanup();
}

Notice that this finally clause does not have access to the return value unless we do some extra work above and capture it in a variable. So, the "default" is for finally NOT to have access to the return value or to the exception. While cleanup can throw an exception, thus transforming an originally-successful return into a failure, it cannot turn a failure into a successful return by not throwing.

Now consider how the current always() implementation works in this code:

var fn = require('when/function');
fn.call(doSomething, x)
  .otherwise(handleError)
  .always(cleanup);

There are two obvious differences:

1. cleanup has access to the result or the error.
2. cleanup can modify the outcome of the operation in a way that it can't above: it can transform a failure into success simply by returning (even if it simply returns its input, i.e. the identity function!).

Here is a proposed version of always that I think behaves more like (but still not exactly like) finally:

Promise.prototype.always = function(callback) {
  return this.then(
    function(value) {
      return when(callback(), function() {
        return value;
      });
    },
    function(error) {
      return when(callback(), function() {
        return when.reject(error)
      });
    });
  )
}

Notice that callback is not given access to either the fulfillment value or the rejection reason. It can still turn a success into a failure by throwing, but it cannot turn a failure into a success simply by returning. It also cannot change the ultimate fulfillment value.

I'm becoming a fan of this, but want to see what other folks think.

One potential variant of this would be to pass the value/reason to callback, but still to disallow the case of turning a failure into success. That would look like:

Promise.prototype.always = function(callback) {
  return this.then(
    function(value) {
      return when(callback(value), function() {
        return value;
      });
    },
    function(error) {
      return when(callback(error), function() {
        return when.reject(error)
      });
    });
  )
}

Thoughts? Alternatives?

[unscriptable]

After some thought, I really like the idea that "clean up" is the top use case for this functionality. It seems that other use cases are easily served by .then() and .otherwise() -- even if we have to do .then(func, func).

Explicitly passing the same function for onFulfilled and onRejected ( .then(func, func)) makes it obvious that you want the function to be polymorphic.

Now I'm wondering if "always" is the right word. While it doesn't force an end to the chain, it certainly feels like the end of a unit of work when it's framed as an analog to "finally".

doSomething().otherwise(handleError).always(cleanup).then(doSomethingElse);

Heh. When I see it like that, "always" seems like the right word. Nm, I guess.

[unscriptable]

+1 for not passing anything to .always()

[Twisol]

I'm not sure "always" is a good name for this kind of functionality. It suggests semantics similar to that of "then" and "otherwise", when this is actually more like Ruby's Object#tap than anything. Maybe "tap" or "guarantee" would be better?

[briancavalier]

@unscriptable Wow, so much good stuff in your response. tl;dr you've completely convinced me that this new approach is the way to go :)

Explicitly passing the same function for onFulfilled and onRejected ( .then(func, func)) makes it obvious that you want the function to be polymorphic.

You could have stopped here, and I would have been convinced :) The synchronous analog to .then(f, f) is a great way to visualize why this makes sense:

try {
  return f(doSomething(x));
} catch(e) {
    return f(handleError(e));
} finally {
    cleanup();
}

You must add two calls to f(), which exactly parallels .then(f, f). Note that while you could assign the result of doSomething or handle Error to a value and call f inside or after the finally, that's not parallel to returning promise.then().otherwise().always().

Now I'm wondering if "always" is the right word. While it doesn't force an end to the chain, it certainly feels like the end of a unit of work when it's framed as an analog to "finally".

Right, and the thing to remember about finally is that it doesn't have to be the last thing in a function. You can (obviously!) still execute code after the closing brace of a finally. So, we shouldn't think of always as the absolute end of a promise chain, which leads me to:

doSomething().otherwise(handleError).always(cleanup).then(doSomethingElse);
Heh. When I see it like that, "always" seems like the right word. Nm, I guess.

I agree, it seems perfectly reasonable. In this case .then() is simply acting to stitch together the previous doSomething/otherwise/always triad (which is an analog to a try/catch/finally triad!), with code that follows the triad (i.e. code after a finally). So, with this newly proposed .always() behavior, it will be exactly parallel to the synchronous:

var result;
try {
  result = doSomething(x);
} catch(e) {
    result = handleError(e);
} finally {
    cleanup();
}

doSomethingElse(result);

I'm convinced!

[briancavalier]

@Twisol you've raised a great point! We have some options:

1. Change the behavior of always(). Since this breaks backward compat, we must wait for 2.0 (which is not really far off at all)
2. Introduce a new API for this, and play the name game. We can introduce this anytime we want, e.g. 1.8.1, and then we have sub-options:
   1. Deprecate always() because we feel it is useless or hazardous, and remove it in 2.0
   2. Leave it because we feel it is useful and worth the hazards.

We can have the name discussion after we decide which way we want to go :)

[renato-zannon]

Until now I've used always only for convenience - as a shorthand for then(func, func) - and I agree that the error-eating behavior can be a little confusing. So, +1 for a change on that!

About the API, I think that modifying the existing always might be confusing if people are aware of how it works now (and want to use it like that). So, I would vote for 2.1 on @briancavalier's options :)

[briancavalier]

@Twisol I just read about ruby's tap(), very interesting! Without getting into the naming discussion just yet, it led me to realize that this new functionality could be implemented as:

Promise.prototype.always = function(callback) {
  var self = this;

  return this.then(injectHandler, injectHandler);

  function injectHandler() {
    return when(callback()).yield(self); // <- whoah!
  });
}

Interestingly, that is likely to be more efficient considering how when.js's internal machinery works.

<3 promise.yield()

[unscriptable]

head asplodes

On Fri, Feb 15, 2013 at 10:07 AM, Brian Cavalier
notifications@github.comwrote:

@Twisol https://github.com/Twisol I just read about ruby's tap()http://blog.rubybestpractices.com/posts/gregory/011-tap-that-hash.html,
very interesting! Without getting into the naming discussion just yet, it
led me to realize that this new functionality could be implemented as:

Promise.prototype.always = function(callback) {
var self = this;

return this.then(injectHandler, injectHandler);

function injectHandler() {
return when(callback()).yield(self); // <- whoah!
});}

Interestingly, that is likely to be more efficient considering how
when.js's internal machinery works.

<3 promise.yield()

—
Reply to this email directly or view it on GitHubhttps://github.com//issues/103#issuecomment-13610568.

[briancavalier]

Ok, I guess we play the name game a bit to see where it leads us :) Some contenders, in no particular order:

tap - Quite a nice word on its own, imho. However coming from a non-ruby perspective, it doesn't seem intuitive to me. I get the feeling from reading that tap article and its comments that it confused rubyists as well.

guarantee - also a nice word; on the longish side (but hey, we also have "otherwise", so who cares). What are we guaranteeing? promise.guarantee(myFunc) ... although, for certain function names, e.g. promise.guarantee(cleanup), it may be quite intuitive.

finally - why not? :) Are there still environments we care about that would have to quote this? Consider this: alias the new when/function's call to try, alias otherwise to catch and implement this new functionality as finally:

var fn = require('when/function');
return fn.try(
    doSomething, x, y
).catch(
    handleError
).finally(
    cleanup
);

Looks surprisingly like its synchronous counterpart. The oddly placed return is the only ugliness, although perhaps a good reminder that this isn't actually synchronous at all.

That raises the question of whether we should try to look so similar to sync code or not. Would it confuse people?

[unscriptable]

Looking back at your example for always as tap, it seems that the callback is necessarily a side-effect of the promise pipeline. In that sense, the word "tap" kinda makes sense to me. However, it's more like "also do this other thing and then continue". What is the word for that?

[briancavalier]

Ah, right, good point about thinking of it as side-effect. Although, you're right: it's not a completely detached side effect ... we're not spawning a new thread to do something "over there". It's still temporally attached: it can return a promise to delay the next link in the chain.

"do this other thing then continue" thoughts: aside, pause, wait, meanwhile, interrupt, intervene, soliloquy

"meanwhile" is a great word, but since it implies "at the same time", it's not the right meaning here :( We should save it for some other purpose, tho!

[renato-zannon]

To add to the pool of options, there's also ensure, which is basically ruby's finally (if we forget about some ruby-only edge cases). I think it has a semantic very similar to guarantee, except for the direct parallel it brings for rubyists (is that desirable?). Using the previous example:

doSomething().otherwise(handleError).ensure(cleanup).then(doSomethingElse);

I like the similarity that finally and the aliases bring! As for the confusion, I think that using the usual formatting it is diminished:

fn.try(doSomething, x, y).catch(handleError).finally(cleanup);

[briancavalier]

A thought on "tap": Putting aside what I know about ruby's tap now, to me tap implies a way to "tap into" the promise chain, i.e. to observe the current state, but not change it, a la "wire tap". That's not necessarily what we're doing here, since we aren't granting access to the value/reason.

That said, "wire tap"-like functionality seems like something that could be an interesting addition, although I haven't thought through what it would actually do :)

[briancavalier]

"ensure" - good one! Like you said, it's "guarantee" with fewer keystrokes.

[briancavalier]

Oh, I know! promise ... hmmm, wait, nm ;)