Deceptive Promises behavior
Tags: javascript, react, typescript
Published: 2021-11-17 15:19:52

The most part of applications written in JS nowadays uses at least few calls of **Promises API**, some of them uses es5 syntax, others `async/await`. But sometimes 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 interesting case with promises: promises have an API which can lead to incorrect interpreting of potential result.

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 user:

```javascript
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 case. 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 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:

```javascript
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 correct order, which feels harder to read.

```javascript
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:
```javascript
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 be dangerous because they could be incorrectly interpret 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:

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

this looks and reads easier, compared to the responsible:
```javascript
const createUserHandler = userData => {
  saveUser(userData)
    .then(user =>
      sendWelcomeMessage(user)
        .catch(sendErrorMessage)
    );
};
```

Lets go further, to see another way how the API can be dangerous: lets add additional logging for if the user can't be created:

```javascript
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 save fails, but if `sendWelcomeMessage` failed, we will need to send error message for user email.

However, because 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 a email for a non-existing user.

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

```javascript
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 developer is moving towards the way of readability.