Fri, 31 Oct 2014

Promises are Useful

Henrik Joretag tweeted that he doesn't like promises in javascript.

I’m just gonna say it: I really don’t like promises in JS. It’s a clever pattern, but I just don’t see the upside, sorry.

— Henrik Joreteg (@HenrikJoreteg) October 26, 2014

I've been working on a large javascript application for the past two years, and I've found promises to be invaluable in expressing the dependencies between functions.

But Henrik's a smart guy, having authored Human Javascript and written lots of Ampersand.js, so I decided to look at a recent case where I found the use of promises helpful, and try to come up with the best alternative solution.

The Backstory

I recently discovered a bug in our app initialization. It's an offline-first mobile app. We have a function that looks like this:

App.init = function() {
   return Session.initialize()
      .then(App.initializeModelsAndControllers)
      .then(App.manageAppCache)
      .then(App.showHomeScreen)
      .then(App.scheduleSync)
      .then(_.noop, handleError);
};

When the app starts up, it needs to:

  1. Load some user info from the local datastore, if possible; otherwise, from the network
  2. Load metadata about the models used in the app and initialize controllers
  3. Set up polling for application cache updates and event handles to appcache events, to prompt the user to restart when a new version of the app is available, for example
  4. Show the initial application screen
  5. Schedule a sync of metadata and data
  6. and if an error occurs during any of these steps, handle it gracefully

The chaining of promise-returning functions in App.init nicely expresses the dependencies between these steps.
We need valid session info for the user before we can initialize the models, which may require fetching data over the network.
We can't show the initial home screen until the controllers have been initialized to bind to DOM events and display information from the models.
If there's a new version of the app available, we want to prompt the user to restart before they start using the app, and we don't want to schedule a sync until the app is being used.

Unfortunately, during a recent upgrade some users ran into a bug in some code I wrote to upgrade the local database, called from App.initializeModelsAndControllers. The result was an uncaught javascript error, Uncaught RangeError: Maximum call stack size exceeded, on the initialization screen which is displayed before App.init is called.

I fixed the bug which broke upgrading some databases, but I also realized that the dependency between App.initializeModelsAndControllers and App.manageAppCache should be reversed. If there's an upgrade available, we want users to be able to restart into it before initializing the app further (and running more code with a larger surface area for possible bugs).

The diff to make this change with our promise-returning functions is nice and simple:

--- a/app.js
+++ b/app.js
@@ -1,7 +1,7 @@
 App.init = function() {
   return Session.initialize()
-     .then(App.initializeModelsAndControllers)
      .then(App.manageAppCache)
+     .then(App.initializeModelsAndControllers)
      .then(App.showHomeScreen)
      .then(App.scheduleSync)
      .then(_.identity, handleError);

(The careful reader will notice that stuck users need a way of upgrading the broken app stored in the app cache. The cache manifest gets fetched automatically to check for and trigger an app update each time the page is loaded, so even though we haven't set up an event handler to prompt the user to restart after the update has been downloaded, if the user waits a few minutes to ensure that the app update is complete, then kills and restarts the app, the new version will be loaded.
The careful reader who happens to be a coworker or affected customer will note that this didn't actually work because our cache manifest requires a valid session and our app starts up without refreshing the OAuth session to speed up start-up time. An upcoming release of GreatVines app to the iOS app store will contain a new setting to trigger the session refresh at app initialization. Affected customers have been given a work-around that requires tethering the device to a computer via USB. Developers using the Salesforce Mobile SDK are welcome to email me for more info.)

Recapping the Question

App.init = function() {
   return Session.initialize()
      .then(App.initializeModelsAndControllers)
      .then(App.manageAppCache)
      .then(App.showHomeScreen)
      .then(App.scheduleSync)
      .then(_.noop, handleError);
};

Now, back to the issue at hand. What would this function look like using an alternative to promises. Ignoring the possible use of a preprocessor that changes the semantics of javascript, the two likely candidates are callback passing and event emitter models.

The typical callback hell approach looks something like this:

App.init = function(success, error) {
   Session.initialize(function() {
      App.manageAppCache(function() {
         App.initializeModelsAndControllers(function() {
            App.showHomeScreen(function() {
               App.scheduleSync(success, error);
            }, error);
         }, error);
      }, error);
   }, error);
};

For simplicity, I've ignored the fact that the original version both had some error-handling and returns the error to the caller through a rejected promise, but I'm pretty sure this is not what Henrik had in mind. So what might an improved callback-passing approach look like?

App.init = function(success, error) {
   var scheduleSync = App.scheduleSync.bind(App, success, error);
   var showHomeScreen = App.showHomeScreen.bind(App, scheduleSync, error);
   var initializeModelsAndControllers = App.initializeModelsAndControllers.bind(App, showHomeScreen, error);
   var manageAppCache = App.manageAppCache.bind(App, initializeModelsAndControllers, error);
   Session.initialize(manageAppCache, error);
};

By pre-binding the callbacks, we avoid the nested indentations, but we've reversed the order of the dependencies, so that's not a great improvement.

I've failed to come up with a good callback-passing alternative to promises, so let's look at what event emitter approach would look like.

App.init = function() {
   BackboneEvents.mixin(App);
   App.once('sessionInitialized', App.manageAppCache);
   App.once('appCacheManaged', App.initializeModelsAndControllers);
   App.once('modelsAndControllersInitialized', App.showHomeScreen);
   App.once('homeScreenShown', App.scheduleSync);
   var initialized = App.trigger.bind(App, 'initialized');
   App.once('syncScheduled', initialied);
   App.once('sessionInitializationError appCacheManagementError modelsAndControllersInitializationError homeScreenError syncSchedulingError', function(error) {
      App.off('sessionInitialized', App.manageAppCache);
      App.off('appCacheManaged', App.initializeModelsAndControllers);
      App.off('modelsAndControllersInitialized', App.showHomeScreen);
      App.off('homeScreenShown', App.scheduleSync);
      App.off('syncScheduled', initialied);
      App.trigger('initializationError', error);
   });
   Session.initialize();
};

This approach does allow us to list dependencies from top to bottom, and doesn't take us into calllback hell, but it has some problems as well.
With the promise-returning and callback-passing approaches, each function was loosly coupled to the caller, returning a promise or accepting success and error callbacks. With this event emitter approach, we need to make sure both the caller and called agree upon the names of events. Admittedly, most functions are going to be more tightly coupled than these, regardless of the approach, having to agree upon parameters to the called function and parameters to the resolved promise or callback, but they don't need to share any special information like the event name to determine when the called function is done.

This event emitter example also requires more work to clean up when an error occurs.

Perhaps instead of using a single event bus, we could listen for events on multiple objects and standardize on event names. Events could be triggered on each function.

App.init = function() {
   var initialized = App.init.trigger.bind(App.init, 'initialized');
   var error = function(error) {
      App.init.trigger('error', error);
      Session.initialize.off('done', App.manageAppCache);
      Session.initialize.off('error', error);
      App.manageAppCache.off('done', App.initializeModelsAndControllers);
      App.manageAppCache.off('error', error);
      App.initializeModelsAndControllers.off('done', App.showHomeScreen);
      App.initializeModelsAndControllers.off('error', error);
      App.homeScreenShown.off('done', App.scheduleSync);
      App.homeScreenShown.off('error', error);
      App.scheduleSync.off('done', initialized);
      App.scheduleSync.off('error', error);
   };
   // Assume every function mixes in BackboneEvents
   Session.initialize.once('done', App.manageAppCache);
   Session.initialize.once('error', error);

   App.manageAppCache.once('done', App.initializeModelsAndControllers);
   App.manageAppCache.once('error', error);

   App.initializeModelsAndControllers.once('done', App.showHomeScreen);
   App.initializeModelsAndControllers.once('error', error);

   App.homeScreenShown.once('done', App.scheduleSync);
   App.homeScreenShown.once('error', error);

   App.scheduleSync.once('done', initialized);
   App.scheduleSync.once('error', error);

   Session.initialize();
};

This is very verbose, containing a lot of boilerplate. We could create a function to create these bindings, e.g.

var when = function(fn, next, error) {
   BackboneEvents.mixin(fn);
   fn.once('done', next);
   fn.once('error', error);
};
when(Session.initialize, App.manageAppCache, error);
...

But it doesn't feel like we're on the right path. We'd still have to handle unbinding all of the error handlers when an error occurs in one of the functions.

It's also worth pointing out that using an event emitter isn't really appropriate in a case like this where you want to get an async response to a specific function call. If the called function can be called multiple times, you don't know that the event that was triggered was in response to your call without passing additional information back with the event, and then filtering.

There's Got To Be A Better Way

Unsatisfied with any of the alternatives to promises I came up with, I replied to Henrik's tweet, and he said that he uses Async.js and Node's error-first convention. So let's look at what using async.series would look like.

App.init = function(callback) {
   async.series([
      Session.initialize,
      App.manageAppCache,
      App.initializeModelsAndControllers,
      App.showHomeScreen,
      App.scheduleSync,
   ], callback);

Finally, we've come to an alternative solution that feels equivalent to the promise one. The async.series solution has at least one benefit over the promise-chaining one: the Async library has a separate function, async.waterfall, to use when the return value from one function should be passed as a parameter to the next function in the series. In my original solution, it's not clear without looking at the function definitions whether the return value from each function is used by the next function.

Given the simple error handling in App.init, in which an error anywhere in the chain aborts the flow, and a single error handler function is responsible for handling the error from any of the functions, Async.js would be a decent replacement.

But for a more complicated flow, a promise-based approach would probably come out on top. For example, here's what App.initializeModelsAndControllers looks like:

App.initializeModelsAndControllers = function() {
   return Base.init()
      .then(Base.sufficientlyLoadedLocally)
      .then(App.handleUpgrades, App.remoteInit)
      .then(App.loadControllers);
};

This function introduces a fork in the chain of serialized functions:

  1. Initialize the models
  2. Check whether the app is ready to start up offline
  3. Initialize the controllers

It looks like this could be done with async.auto, but it would probably start getting messy.

tech | Permanent Link

The state is that great fiction by which everyone tries to live at the expense of everyone else. - Frederic Bastiat