index.md 30KB
title: Promises: All The Wrong Ways
url: https://blog.getify.com/promises-wrong-ways/
hash_url: 2a774113a3
If you’re not well-versed on JavaScript Promises yet, I urge you to read up on them before diving into this article.
Promises are the promise of a vastly improved asynchrony over our previous primitives (notably: callbacks). By abstracting a potentially future value so that it can be treated as time-independent — we don’t have to care if the value is still pending or already finalized — we remove time from our concerns, making our programs far easier to reason about.
But I’m not writing to convince you to use promises. Nor am I writing to convince you not to use them. I simply want to call out some ways promises are being used in the wild that I think miss the point, muddy up your code, and dilute the benefits of using them in the first place.
False Start
You use some lib that has a function called foo(), and supposedly when you call it, you get a promise back. But… exactly what flavor of promise is it? Your app uses Bluebird promises because you love those extra API methods, but do you know what kind of promise this other foo() function is going to give you? Are you even sure it’s a reliably Promises/A+ compliant promise?
How much can you really trust that the promise you’re getting is the promise you think you’re getting? And what if you are using different libraries that each have their own flavor of promises returned? How are you managing the mixing of all these promises in your code?
The plain fact, which is inconvenient and troubling, is this: this code is a bad anti-pattern:
foo() .then( nextStep ) ..
You need to normalize the promise you’re receiving back to make sure it’s a legitimate promise and of the flavor you assume.
Fortunately, this is relatively easy, if not a teeny bit tedious. Use Promise.resolve(..) — or whatever the equivalent is for your promise library of choice — to normalize the returned value into a promise you recognize and trust:
Promise.resolve( foo() ) .then( nextStep ) ..
If foo() returns a promise that is the same as you are expecting, it’ll pass through Promise.resolve(..) untouched. Otherwise, it’ll be adapted so that the return of Promise.resolve(..) is a promise you recognize and trust.
But wait… does that mean we have to wrap every single promise we get? Eh, not exactly.
The then(..) method automatically does this same sort of normalizing on any return value it receives back from either the fulfillment or rejection handlers.
Promise.resolve( foo() ) .then( bar ) .then( lastStep );
Even if bar() produces a different kind of promise, that promise is subsumed by then(..) — the then(..) that comes from the kind of promise we want, via Promise.resolve(..) — meaning that the normalization is automatically implied as we’d want.
So it’s really only the beginning of the promise chain that we need to be more careful with. We need to make sure to normalize all first-of-chain promises, just in case.
Start Delays
Speaking of the start of a promise chain, another mistake that’s often made is adding unnecessary promise resolution steps. For example:
Promise.resolve() .then( firstStep );
Why would we do this? The first reason is that it automatically normalizes firstStep()‘s returned value/promise, as just described. The second we’ll cover in the next section.
But instead of calling firstStep() right away, here it’ll be delayed until the end of the event-loop tick. Is that really a big deal? Usually not, but when you do this over and over again, those delays can add up.
Don’t put an extra empty tick at the beginning of your promise chain. Just start the chain right away.
And while we’re discussing it, don’t do this either:
new Promise( function(resolve,reject){
firstStep()
.then( resolve, reject );
} )
..
You may think that’d be obnoxious and obviously bad, but it’s more common than you’d realize and really shouldn’t be done.
Failing To Start
What if there’s any case where firstStep() being called could result directly in a synchronous exception — not a rejected promise?
Most people think such method behavior shouldn’t ever be designed — indeed, many would call such a function an anti-pattern in and of itself — but this post isn’t about bad patterns with API design, it’s about bad practices with using the promises you get.
The fact is, some functions of this sort do exist. If you have to deal with one, you’re probably going to want trap such an exception and turn it into a promise rejection that can be dealt with in the normalized promise chain.
You might consider doing something manual like this:
var p;
try {
p = Promise.resolve( firstStep() );
}
catch (err) {
p = Promise.reject( err );
}
p
.then( .. )
..
We not only normalize the return value to the desired promise flavor with Promise.resolve(..), but the try..catch also normalizes any caught exception into the rejection. That’s effective, but ugly, and this code’s verbosity doesn’t actually contribute to easier understanding, IMO.
This concern is where two earlier-mentioned patterns probably originated from:
Promise.resolve()
.then( firstStep )
..
// and:
new Promise( function(resolve,reject){
firstStep()
.then( resolve, reject );
} )
..
In both cases, if firstStep() results in an exception, it will automatically be caught by the Promise machinery, and turned into a promise rejection that the rest of the chain can articulate handling for.
But I think a cleaner way of doing this is to use an async function, specifically in its => arrow-function IIFE form:
async _=>firstStep()() .then( .. ) ..
Note: async functions are a proposed addition to the JS standard, but have not officially landed. They are experimentally implemented in several major browsers, but as with all recent JS changes, you’ll likely be transpiling them, which is fully supported by Babel and others.
The async function normalizes whatever comes back from the firstStep() function call into a promise, including catching any exception and turning it into a promise rejection.
The small caveat here, different from using Promise.resolve(..) manually, is that the promise coming back from an async function is going to be a built-in native Promise, not some custom extended flavor. Use an additional Promise.resolve(..) or equivalent to cast it into a promise you’re happy with.
Deconstruction
When you construct a promise manually, the initialization function you pass to the constructor receives the capabilities for resolving that promise:
var pr = new Promise( function init(resolve,reject){
// call `resolve()` to fulfill the promise
// or call `reject()` to reject it
} );
By convention, we usually call those functions resolve() and reject().
Back before the standard for promises was finalized, there was a notion of these two capabilities — referred to collectively at the time as the deferred — being separately created from the promise they controlled.
Imagine creating a promise more like this:
var [ pr, def ] = new Promise(); // call `def.resolve()` to fulfill the promise // or call `def.reject()` to reject it
Note: FYI: [pr,def] = .. is utilizing the new ES6 destructuring syntax, specifically array destructuring.
Having the promise and the deferred separated can seem quite useful in certain cases if the code that is doing this construction needs to transfer the resolution capabilities (deferred) to one part of the code and the observation capabilities (the promise) to another.
For example:
var [ pr, def ] = new Promise(); // later: setupEvent( def.resolve, def.reject ); // even later: handleEvent( pr );
Essentially, this approach treates a promise like a single-value event conduit. One part of the app holds the “write end” of the pipe, the deferred. Another part of the app holds the “read end”, the promise.
Though the Promise API doesn’t work like this, it is possible to emulate, by extracting the resolution capabilities:
function makePromise() {
var def = {};
var pr = new Promise( function init(resolve,reject){
def.resolve = resolve;
def.reject = reject;
} );
return [ pr, def ];
}
var [ pr, def ] = makePromise();
As you can see, we extract the resolve() and reject() capabilities from the promise initialization function storing them on def, then return both pr and def to the calling code.
Undeniably, there are cases where this is quite convenient. I’ve done this many times myself.
But I will assert that it’s an anti-pattern and that you should try to avoid it. At a minimum, if you have to do so, you should be hiding that kind of trick inside the plumbing of a library, not exposing the deferred and promise separately at the top level of logic in your application.
If you agree with me that promise resolution extraction smells a little hackish, it’s because IMO we’re abusing a promise to act like a different kind of abstraction (that is already more well understood).
Abstractions For Value Communication
One way to envision this alternate, more well-suited abstraction is as an Event Emitter:
var evt = new EventEmitter(); // later: setupEvent( evt ); // even later: handleEvent( evt );
Inside setupEvent(..), we use evt to call evt.emit(..) to set an event of some name, and optionally pass along any data with that event. Inside handleEvent(..), we call something like evt.once(..) to set up a single-occurring event listener for that pre-agreed event name.
Either with promises (as shown earlier) or these Event Emitter events, we use a push model for communicating the value at the appropriate future time.
But another way to abstract this problem is with a pull model. In other words, instead of the producer pushing the value once it’s ready, the consumer pulls the value when it’s ready to handle it.
In my asynquence library, I have an abstraction called “iterable sequences”, which is basically like an queue. You can add new handlers to the queue by calling then(..), and then iterate through the queue using the standard next() iterator interface.
For example:
var isq = ASQ.iterable();
isq
.then( function(val){
return val * 2;
} )
.then( function(val){
return !!val;
} )
.then( function(val){
return val.toUpperCase();
} );
isq.next( 21 ); // { value: 42, done: false }
isq.next( false ); // { value: true, done: false }
isq.next( "hello" ); // { value: "HELLO", done: false }
isq.next(); // { done: true }
An iterable sequence is a different kind of abstraction that encapsulates both the resolution and observation capabilities into a single interface. We pass around that single abstraction wrapper instead of splitting up the promise and the deferred.
Note: By the way, that’s similar in nature to Observable Subjects.
Aside from custom library abstractions, we can use already built-in parts of JavaScript to create a pull abstraction directly. A generator can create a producer iterator:
function *setupProducer() {
var pr = new Promise(function(resolve,reject){
setupEvent( resolve, reject );
});
yield pr;
}
var producer = setupProducer();
What’s going on here? We want an iterator that when we call next() on it, we’ll get back a value. But when we pull it, that value might not be ready yet. So instead, we yield a promise for that single next value that the producer will eventually produce. So the consumer is pulling (iterating) out each push instance (promise).
Why is that different from the earlier promise capability extraction I called an anti-pattern? Because we’re not tracking the promise and its paired deferred separately at the top level of logic; we’ve hidden those as implementation details inside the setupProducer() generator.
What we instead pass around is the wrapper around this capability: producer, which is an iterator. That’s a better abstraction that makes code easier to reason about. Tracking promises and their deferred capabilities separately is much harder to reason about in complex applications.
Note: The other advantage, though hard to see in this snippet, is that our generator/iterator could produce a new promise for each pull request, so we can use this conduit for communicating multiple values, instead of just once when there’s only a single promise involved. We get a new promise for each value.
Here’s how we later pull a value:
var pr = producer.next();
pr.then( function(value){
// ..
} );
We call next() on our producer iterator, and we get out a promise, that we can then listen to.
By the way, this notion of a generator that produces promises is not my own invention. There’s currently a proposal to add a new primitive to JavaScript called an “async generator”. It basically does what I just sketched out: it gives an iterator that returns a new promise each time it is next() iterated.
These various abstractions are far better suited for modeling the transmission of a value asynchronously from one part of the application to another. Promises have to be twisted and contorted to serve that purpose.
Don’t do it just because you can do it. Promises shouldn’t be used that way, at least not visibly to your main application code.
Promise Side-Effects
Promises are, at their most core, a model for values. The fact that we use functions with the Promise API as the vehicle for unwrapping those values is an unfortunate temptation trap we can too easily fall into.
Functional programming practices implore us to rely on value immutability for easier to reason about code, as well as using functions without side-effects (aka, pure). But it’s all too common that promises are used in very non-FP-friendly ways. Specifically, it’s far too common to either create or rely on side-effects with the functions passed to promise methods.
The first kind of common side-effect is relative sequencing between multiple promises.
Consider:
firstStep()
.then( secondStep )
.then( function(){
console.log( "two" );
} );
firstStep()
.then( function(){
console.log( "one" );
} );
What order do you think those two console messages are printed? Almost certainly in “one”, “two” order, right? Almost. It’s actually possible that the second invocation of the firstStep() function will produce a promise that isn’t resolved until after the secondStep() promise is finished, resulting in “two”, “one” order.
If you author code like this, what you’re saying is, “I believe I can predict the relative ordering of asynchronous activity, and rely on that for my application.” Unfortunately, you’re the only one; noone else who reads your code will be able to do so to the same effective extent. ð
Another example:
Promise.resolve().then( function(){
console.log( "two" );
} );
console.log( "one" );
The console here will always print “one”, “two”, because we’re taking advantage of our knowledge that a then(..) handler will always be run asynchronously.
By “run asynchronously”, we mean strictly after the current synchronous execution context, but we don’t actually mean on the next tick of the event loop, like with a setTimeout(..0) type of asynchrony:
setTimeout( function(){
console.log( "three" );
}, 0 );
Promise.resolve().then( function(){
console.log( "two" );
} );
console.log( "one" );
In a conforming/native implementation, this snippet will always print “one”, “two”, “three”, because the “two” console message function is on the microtask queue to be run at the end of the snippet (after the “one” console message), but the setTimeout(..0) schedules the “three” message on the next event loop tick.
As intrictate as this situation is, I’ve seen plenty of code that relies on the relative sequencing of these types of actions, even with multiple promises:
Promise.resolve().then( function(){
console.log( "one" );
} );
Promise.resolve().then( function(){
console.log( "two" );
} );
The specification doesn’t explicitly require “one”, “two” sequencing here. However, in practice, it’ll probably happen that way, because there will probably only be one internal microtask queue that those two separate then(..) actions are scheduled onto, in first-come-first-served order.
While you might be able to rely on this as an implementation detail, I think these kinds of tricks are an absolutely terrible idea.
If there’s some dependency between the actions, then express it directly — do not rely on perceived behaviors or async microtask processing. Even if the code “works”, you’ve written entirely un-reason-able code, as future readers will almost certainly not be able to intuit those same implied sequencing relationships.
Inter-promise sequencing may or may not be guaranteed in any given scenario, but one thing that is guaranteed is that relying on this behavior makes your code worse. Don’t do it!
Construction Sequencing
Another place where this sequencing side-effects thing is often seen is with the Promise(..) constructor.
The idea is to take advantage of the knowledge that the initialization function you pass in is synchronously executed, even though the other parts of the Promise API (like the handlers you pass to then(..)) execute those functions asynchronously.
We already saw this illustrated earlier when we discussed the capability extraction.
For example:
var fn;
var pr = new Promise( function init(resolve){
fn = resolve;
} );
fn( 42 );
We know we can call fn() right away because we know the init(..) function is executed synchronously by the Promise constructor.
I’m asserting that this is a bad idea to exploit this sort of semantic in your normal application code alongside other then(..) function calls, since those are always asynchronous. Having both sync and async behaviors mixed together makes the code harder to understand.
The above snippet is equivalent to just:
var pr = Promise.resolve( 42 );
I’m not saying to avoid the Promise constructor. But don’t use it as a synchronous side-effect producing function — specifically, relying on the sequencing of the side effects — if you can at all avoid it.
Scopes
Another pattern that seems to crop up is the sharing of values across scopes via side-effects, to get around the fact that promises only model/fulfill single values.
Consider:
function getOrderDetails(orderID) {
var _order;
return db.find( "orders", orderID )
.then( function(order){
_order = order;
return db.find( "customers", order.customerID )
} )
.then( function(customer){
_order.customer = customer;
return _order;
} );
}
What’s going on here? We first make a db.find("orders" ..) call, which gives us the order. We not only need that to get order.customerID for the db.find("customers" ..) call, but we’ll also need order for the final result of our getOrderDetails(..) operation.
But the second then(..) will receive the result of the db.find("customers" ..) call, which will not pass along the order value as part of its result. So, it seems we have to save it somewhere, which we choose to do with an outer _order variable that’s shared across scopes. When the second then(..) handler runs, _order will still be available for us to use.
Don’t kid yourself: this is side-effect programming. _order is a side-effect of the first then(..). We are crippled by the limitations of the Promise API mechanics and can’t share the state with the next step, so we just stick that state in an outer scope to share it.
This is a bad idea. I don’t care how many times you’ve done this or how acceptable you think it is. It’s an anti-pattern code smell. Stop programming side-effects with your promises. It makes your code harder to reason about.
I know how allergic many developers seem to be with respect to nesting, but the more appropriate way to handle this case is to nest the second then(..) in the scope where order still exists, as shown here:
function getOrderDetails(orderID) {
return db.find( "orders", orderID )
.then( function(order){
return db.find( "customers", order.customerID )
// nested `then()` instead of flattened to outer chain,
// so that we still have lexical scope access to `order`
.then( function(customer){
order.customer = customer;
return order;
} );
} );
}
Normally, nesting promise then(..)s is a bad idea, and even goes by the name “promise hell”.
But in these cases where multiple values need to be available (in scope), a flattened vertical promise chain necessitates the worser evil of side-effects. Promise nesting is the better choice.
Promise Chain’itis
A promise models a value that may still be pending. The purpose is to factor time out of the picture so that you only have to think about the value and what you need to do with it — not the time you have to wait until it’s ready.
The problem is that the mechanics of how promises work in JS means that the finalizing of a value is observable — you can tell when it happens. Though you shouldn’t be thinking about time, promises let you still think about time if you want to, and many people do it too much.
The most obvious way we use our knowledge of when a promise resolves is to use this observation as a step in a flow control. For example:
firstStep() .then( secondStep ) .then( thirdStep );
Here, what we’re saying with the promise chain is that we want to delay the execution of the secondStep() function until after firstStep() has finished. You’ll notice that nothing here makes it obvious that we’re using a promise as a placeholder for a future value. Instead, we’re using promises kind of like event handlers to let us know when each step finishes.
I used to be enormously excited about promise chains as flow control. But the longer I’ve programmed with promises, the more I’ve realized that long promise chains are actually a bad smell. I’ve almost come to the point where I’d say there should only ever be one then(..) in the chain. Using promise chains as a way to express flow control is a poor idea.
Yes, yes, I know that literally almost everybody is doing this. Doesn’t matter, they’re all wrong.
So, what should we do to express our multi-step flow control? Go back to nested callbacks? No.
Use the synchronous-async pattern. If you’re not familiar, I’ve talked about this pattern in detail in several places, including:
To boil this down, using either generators (with a runner), or async functions, we get the ability to express fundamentally asynchronous code in a synchronous fashion. This sync semantic works not only for success flow control but also error handling (ie, try..catch).
Here’s how the previous promise-chain flow control code should be written:
async function main() {
await firstStep();
await secondStep();
await thirdStep();
}
// or:
function *main() {
yield firstStep();
yield secondStep();
yield thirdStep();
}
We’re still relying on the fact that each step returns a promise, and we’re still waiting on each promise to resolve before moving onto the next step.
Unfortunately, with these examples we’re missing not only articulation of the result values but also of any error handling. A bit more of a realistic example:
async function main() {
try {
var val1 = await firstStep();
var val2 = await secondStep( val1 );
var val3 = await thirdStep( val1, val2 );
console.log( "Final: ", val3 );
}
catch (err) {
console.error( err );
}
}
This example clearly illustrates how we are using synchronous code semantics to express the async flow control (success and exception).
Under the covers, something similar to the earlier chain, but not the same, is happening. firstStep() produces a promise, which is then awaited. That means, hidden from view, the engine puts a then(..) on it and waits for it to finish before resuming the paused main() function. It also unwraps the promise’s fulfillment value and gives it back so we can assign it to val1.
When the await is called on the promise from the secondStep(..) call, it’s performing a then(..) against that promise, not the one that came from the then(..) called on the first. And so on.
It may seem like pointless nuance, but that’s not the same thing as chaining one then(..) after another. It’s treating, explicitly, each step as a separate promise/then(..) pair.
Conceptually that’s a much cleaner and easier to understand process than what happens when promises are chained multiple times. If you’ve ever tried to understand just how a promise chain is wired up, or explain it to someone else, you’ll know it’s not so simple. I won’t belabor those details here, but I’ve previously explained it in more detail if you care to dig into it.
The sync-async pattern above wipes away any of that unnecessary complexity. It also conveniently hides the then(..) calling altogether, because truthfully, that part is a wart. The then(..) on a promise is an unfortunate but necessary component of the Promise API design that was settled on for JS.
I assert that, for the most part, we should try not to ever call then(..) explicitly. As much as possible, that should stay hidden as an implementation detail.
Calling then(..) on a promise is a code smell and anti-pattern.
I’m sure you’ve heard the admonition of “use the right tool for the job.” To put a slight twist on that, here the problem is more about using a tool inappropriately. It’s like if you held a hammer in your teeth and tried to drive in a nail. A hammer is better used if held in your hand.
Instead of using the promise API for expressing async flow control, we should only use promises for what they’re good for: modeling future values in a time independent way. Sure, promises can be chained together. No, you shouldn’t be doing that. Let libraries and engines do the plumbing for you.
There’s a better tool for the job of expressing flow control: the sync-async pattern of generators+promises or async functions.
Summary
Summing up, using promises outside of the main notion of modeling a future value independent of time is probably wandering towards anti-patterns.
Promises are powerful and transformative, but I urge you to take a step back and re-examine how you’re relying on them. They are best as a value/composition mechanism. Use them as such and stay away from all the wrong ways.