Fri, 31 Oct 2014
Henrik Joretag tweeted that he doesn't like
promises in javascript.
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:
- Load some user info from the local datastore, if possible; otherwise, from the network
- Load metadata about the models used in the app and initialize controllers
- 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
- Show the initial application screen
- Schedule a sync of metadata and data
- 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:
- Initialize the models
- Check whether the app is ready to start up offline
- If so, kick off any upgrade processing necessary
- If not, start up the remote initialization process
- Initialize the controllers
It looks like this could be done with async.auto,
but it would probably start getting messy.
tech | Permanent Link
