Promises + Sync Code = Disaster

How the wrong mix of synchronous code with Promises can make your code harder to maintain

Promises provide a very good structure to handle asynchronous operations by explicitly changing the interface of your component.

And that's it. It’s not a proxy object.

You might be tempted to start changing your synchronous code style just because you are using Promises. You might try to reason that since it makes the async code looking “pretty”, it can also make your sync code look “pretty”, and then you start abusing the Promises chain to run synchronous functions that don’t return a Promise. In fact, you unnecessarily increase the level of abstractions, which eventually makes your code harder to reason about.

Houston, we have a problem.

You shouldn't start working with Promises as if it was just another way to structure your code, it is not. Promises solve a specific problem, and that problem is to make asynchronous functions inherit synchronous properties (such as throw/catch) and not to simply change the way the synchronous code is written.

Let's take the following pseudo-code as an example:

Yes, it probably works. After the first high-order Promise fetchSoccerMatchesFromResource() is resolved, it will return a thenable that accepts either another Promise, which will be fulfilled later in the next .then, or a resolved value, that will be implicitly casted into a Promise so it doesn’t break the whole execution chain.

Just because it works, it doesn't mean it is correct.

Besides the obvious performance implications of abusing the Promises chain by running synchronous functions unnecessarily, you are also breaking the reader’s expectation by changing the intent of how the code is supposed to work. In the example above, it is expected that each of those functions preceded by a .then returns a thenable that runs a potentially long-running operation under the hood, instead of having synchronous functions that are there just for the sake of being there, with no reason at all.

One of the problems that this can cause is when the reader is trying to spot some major performance problems caused by a long running operation in the event loop. We know that Promises are supposed to be used to make asynchronous code inherit the properties of synchronous code, so when one sees multiple executions of functions like that, it is assumed that for each function preceding a .then call there will be a potentially expensive long running operation happening in the event loop, except for the last function which is supposed to just continue its synchronous flow with the resolved data.

Returning only a resolved value before the next “.then” call will make it difficult to determine if the code is being executed synchronously or asynchronously

The problem above does not make itself much clearer when working with a small number functions. In that case, it is not hard to understand what is happening because all functions are closer, and most importantly: they are small. The problem becomes evident when you have many internal abstractions. In that case, you don’t have a way to make it clear what the value of the returned function represents when reading the code, and you should always be looking up and down in order to understand the details of what the code does, even if the function has a very high cohesion.

Consider the example below:

Can you tell with 100% of confidence that addToCache is not making an additional asynchronous request? Is it adding to the memory cache in a module’s variable or is it persisting to redis? The code is not self-documented, you need domain knowledge to understand that.

This problem is not limited only to the wrong usage of Promises, it can happen with any abstraction where you cannot infer the variable type efficiently inside the context where it is being used.

But then why does the Promise spec allows returning a value? Wouldn’t be better to restrict the return to just a Promise and nothing else? Well, you can have a requirement where you should return a Promise that runs an expensive operation. For example, if the value is in some sort of external cache you can return the value directly, but then the rest of the chain will behave like if you returned a Promise. It is a legit use, because in this case you are not writing only for synchronous purposes.

The code below is inspired by one of the examples of the topic #5 in the famous article "We have a problem with promises":

To avoid the problems of the addToCache example, we should use synchronous code when the execution is only synchronous, and using Promises when the execution can be asynchronous:

This pattern also complies with the Robustness Principle:

• The .then function will be liberal in what it accepts as the return, it can be either a Promise or the resolved value.
• When you are the one returning, though, you should be conservative in what you send, doing what you have to do synchronously and only returning something when there is a valid reason to do so.

Promises are very useful, but using them to run synchronous code can cause unexpected results for other developers reading your code in the future, or even for yourself as the code base grows.

Have some feedback? Twitter | Facebook| Github