The Dangers of False Interpretations When Using the Promises API

The majority of applications written in JS nowadays use at least a few calls of Promises API, some of them use es5 syntax, others async/await. But sometimes, an incomplete understanding of this technology(as in any other) can lead to unpredictable behavior, which can confuse uses, and take hours for you to understand the cause of the problem.

Spending too much time in writing JS code, I've found an interesting case with promises: promises have an API which can lead to incorrect interpreting of potential results.

This is mostly related to classic es5 promise realization, but, alas, also affects async/await promises realization.

Lets as an example check the process of saving users:

const handleSave = userData => {
  saveUser(rawUserData)
    .then(user => showNotification(`User ${getUserName(user)} has been created`))
    .catch(err => showNotification(`User was not created because of error`));
};

This code looks easy to read, but not easy to predict potential edge cases. While trying to be explicit, we have attached our catch not only for the saveUser request, but also for the onFulfilled block. Thus, if then throws the error (e.g. the getUserName function throws) then the user will be notified that user creation failed with error, even though it was.

Someone might think, that switching the order of the then/catch blocks, so that the catch is attached to the saveUser call directly. This paves the way for another issue.

Using the async/await approach will not necessarily help. It is agnostic to using the API correctly, and because of its block scoping it also makes it easier and prettier to write it dangerously as above:

const handleSave = async userData => {
  try {
    const user = await saveUser(userData);
    showNotification(`User ${getUserName(user)} has been created`);
  } catch(error) {
    showNotification(`User was not created because of error`));
  }
};

As you can see, this code has the same issue as above.

To avoid this behavior(when using native Promise API) we need to pass 2 callbacks(error callback, success callback) into then block in the correct order, which feels harder to read.

const handleSave = userData => {
  saveUser(userData)
    .then(
      user => showNotifications(`User ${getUserName(user)} has been created`),
      err => showNotifications(`User was not created because of error`));
    );
};

To be clear, this isn't a bad API in itself. But considering the rightful intention of being explicit as a developer, there is a temptation of using a named function for each, rather than one then with the two callbacks. The responsible code is less explicit and readable than dangerous code - it is temptingly dangerous to misuse the API - while feeling more explicit and readable!

The responsible refactor using async/await looks strange. Having to define variables in a higher scope feels like a bad control flow. It feels like we're working against the language:

const handleSave = async userData => {
  try {
    const user = await saveUser(rawUserData)
        .catch(() => showNotifications(`User could not be saved`))

    showNotifications(`User ${displayName(user)} has been created`);
  } catch(error) {
    console.error(`User could not be saved`));
  }
};

Whereas the examples above are dangerous because they could be incorrectly interpreted by developers the catch is meant to be attached to the "root" async call - there is also a danger with long chains of thinking the catch is associated with the most recent then.

For example:

const createUserHandler = userData => {
  saveUser(userData)
    .then(sendWelcomeMessage)
    .catch(sendErrorMessage)
};

This looks and reads easier, compared to the responsible:

const createUserHandler = userData => {
  saveUser(userData)
    .then(user =>
      sendWelcomeMessage(user)
        .catch(sendErrorMessage)
    );
};

Let’s go further, to see another way how the API can be dangerous: let’s add additional logging for if the user can't be created:

const createUserHandler = userData => {
  saveUser(userData)
    .catch(logUserCreationError)
    .then(sendWelcomeEmail)
    .catch(sendErrorMessageByEmail)
};

What we want is to write the issue to our logs if the user saves fails, but if sendWelcomeMessage failed, we will need to send an error message for the user email.

However, because the catch block doesn't re-throw or reject, it returns a resolved promise and so the next then block which calls sendWelcomeEmail will be triggered, and because there is no user, it will throw, and we will create an email for a non-existing user.

So, the fix looks ugly the same as for the example above:

const createUserHandler = userData => {
  saveUser(userData)
    .then(
      logIssues,
      user =>
          sendWelcomeEmail(user)
            .catch(sendErrorMessageByEmail)
      );
};

To summarize, we've seen how promise's API for handling errors while seemingly sleek, can be dangerous when a developer is moving towards the way of readability.