JavaScript Promises: The Definitive Guide, Part 2

Welcome back! If you missed Part 1, you can check it out here.

Common Mistakes With Promises

People always say that whatever you write in JS will be executed and will work. That’s almost true. At the end of the day, when you’re making quality products, it should be fast and bug-free. By using Promises, you can make some really bad mistakes really easily, just by forgetting to put in some part of the code or by using something incorrectly. Below is a list of the common mistakes with Promises that a lot of people make every day.

Mistake #1: Nested Promises

Check the code below:

loadSomething().then(something => {
  loadAnotherThing().then(another => {
    doSomething(something, another)
  }).catch(e => console.error(e))
}).catch(e => console.error(e))

Promises were invented to fix the “callback hell” and the above example is written in the “callback hell” style. To rewrite the code correctly, we need to understand why the original code was written the way that it was. In the above situation, the programmer needed to do something after results of both Promises are available, hence the nesting. We need to rewrite it using Promise.all() as:

Promise.all([loadSomething(), loadAnotherThing()])
  .then(([something, another]) => {
    doSomething(something, another)
  })
  .catch(e => console.error(e))

Check the error handling too. When you use Promises properly, only one catch() is needed.

A Promise chain also gives us a finally() handler. It’s always executed and it’s good for cleanup and some final tasks that will always be executed, whether the Promise was resolved or rejected. It was added in the ES2018 version of ECMAScript and implemented in Node.js 10. It is used like this:

Promise.all([loadSomething(), loadAnotherThing()])
  .then(([something, another]) => {
    doSomething(something, another)
  })
  .catch(e => console.error(e))
  .finally(() => console.log('Promise executed'))

Mistake #2: Broken Promise Chain

One of the main reasons Promises are convenient to use is “promise-chaining” – an ability to pass the result of a Promise down the chain and call catch at the end of the chain to catch an error in one place. Let’s look at the below example:

function anAsyncCall() {
  const promise = doSomethingAsync()

  promise.then(() => {
    somethingElse()
  })

  return promise
}

The problem with the code above is handling the somethingElse() method. An error that occurred inside that segment will be lost. That’s because we didn’t return a Promise from thesomethingElse() method. By default, .then() always returns a Promise, so, in our case, somethingElse() will be executed, the Promise returned won’t be used, and .then() will return a new Promise. We just lost the results from the somethingElse() method. We could easily rewrite this as:

function anAsyncCall() {
  return doSomethingAsync()
    .then(somethingElse)
    .catch(e => console.error(e))
}

Mistake #3: Mixing Sync and Async Code in a Promise Chain

This is one of the most common mistakes with Promises. People tend to use Promises for everything and then to chain them, even for async code. It’s probably easier to have error handling on one place, to easily chain your code, but using Promise chains for that is not the right way to go. You’ll just be filling your memory and giving the garbage collector more work to do. Check the example below:

const fetch = require('node-fetch') // only when running in Node.js

const getUsers = fetch('https://api.github.com/users')
const extractUsersData = users =>
  users.map(({ id, login }) => ({ id, login }))
const getRepos = users => Promise.all(
  users.map(({ login }) => fetch(`https://api.github.com/users/${login}/repos`))
)
const getFullName = repos => repos.map(repo => ({ fullName: repo.full_name }))

const getDataAndFormatIt = () => {
  return getUsers()
    .then(extractUsersData)
    .then(getRepos)
    .then(getFullName)
    .then(repos => console.log(repos))
    .catch(error => console.error(error))
}

getDataAndFormatIt()

In this example, we have mixed sync and async code in the Promise chain. Two of those methods are getting the data from GitHub, the other two are just mapping through arrays and extracting some data, and the last one is just logging the data in the console (all three are sync). This is the usual mistake people make, especially when you have to fetch some data, then to fetch some more data per every element in the fetched array, but that’s wrong. By transforming thegetDataAndFormatIt() method into async/await, you can easily see where the mistake is:

const getDataAndFormatIt = async () => {
  try {
    const users = await getUsers()
    const userData = await extractUsersData(users)
    const repos = await getRepos(userData)
    const fullName = await getFullName(repos)
    await logRepos(fullName)
  } catch (error) {
    console.error(error)
  }
}

As you see, we’re treating every method as async (that’s what will happen in the example with the Promise chain). But we don’t need Promises for every method, only for two async methods, as the other three are sync. By rewriting the code a bit, we’ll finally fix the memory issue:

const getDataAndFormatIt = async () => {
  try {
    const users = await getUsers()
    const userData = extractUsersData(users)
    const repos = await getRepos(userData)
    const fullName = getFullName(repos)
    logRepos(fullName)
  } catch (error) {
    console.error(error)
  }
}

That’s it! It’s now written properly, only methods that are async will be executed as Promises, the other will be executed as sync methods. We don’t have a memory leak anymore. You should avoid chaining async and sync methods that will make a lot of problems in the future (especially when you have a lot of data to process). If it’s easier for you, go for async/await, it will help you understand what the Promise should be and what should stay in the sync method.

Mistake #4: Missing Catch

JavaScript does not enforce error handling. Whenever programmers forget to catch an error, JavaScript code will raise a runtime exception. The callback syntax, however, makes error handling more intuitive. Every callback function receives two arguments, error and result. By writing the code, you’ll always see that unused error variable and you’ll need to handle it at some point. Check the code below:

fs.readFile('foo.txt', (error, result) => {
  if (error) {
    return console.error(error)
  }

  console.log(result)
})

Since the callback function signature has an error, handling it becomes more intuitive and a missing error handler is easier to spot. Promises make it easy to forget to catch errors since .catch() is optional while.then() is perfectly happy with a single success handler. This will emit the UnhandledPromiseRejectionWarning in Node.js and might cause a memory or file descriptor leak. The code in the Promise callback takes some memory and the cleaning of the used memory after the Promise is resolved or rejected should be done by the garbage collector. But that might not happen if we don’t handle rejected promises properly. If we access some I/O source or create variables in the Promise callback, a file descriptor will be created and memory will be used. By not handling the Promise rejection properly, memory won’t be cleaned and file descriptor won’t be closed. Do this several hundred times and you’ll make big memory leak and some other functionality might fail. To avoid process crashes and memory leaks always finish Promise chains with a .catch().

If you try to run the code below, it will fail with the UnhandledPromiseRejectionWarning:

const fs = require('fs').promises

fs.stat('non-existing-file.txt')
  .then(stat => console.log(stat))

We’re trying to read stats for a file that doesn’t exist and we’re getting the following error:

(node:34753) UnhandledPromiseRejectionWarning: Error: ENOENT: no such file or directory, stat ‘non-existing-file.txt’

(node:34753) UnhandledPromiseRejectionWarning: Unhandled promise rejection. This error originated either by throwing inside of an async function without a catch block or by rejecting a promise which was not handled with .catch(). (rejection id: 1)

(node:34753) [DEP0018] DeprecationWarning: Unhandled promise rejections are deprecated. In the future, promise rejections that are not handled will terminate the Node.js process with a non-zero exit code.

If you don’t handle your errors properly, you’ll leak a file descriptor or get into some other denial of service situation. That’s why you need to clean up everything properly upon a Promise's rejection. To properly handle it, we should add a .catch() statement here:

const fs = require('fs').promises

fs.stat('non-existing-file.txt')
  .then(stat => console.log(stat))
  .catch(error => console.error(error))

With a .catch() statement, when the error happens, we should log it in the console:

{ [Error: ENOENT: no such file or directory, stat 'non-existing-file.txt']
  errno: -2,
  code: 'ENOENT',
  syscall: 'stat',
  path: 'non-existing-file.txt' }

We recommend checking out this blog post from Matteo Collina that will help you understand this issue and learn about the operational impact of unhandledRejection.

Mistake #5: Forget to Return a Promise

If you are making a Promise, do not forget to return it. In the below code, we forget to return the Promise in our getUserData() success handler.

getUser()
  .then(user => {
    getUserData(user)
  })
  .then(userData => {
    // userData is not defined
  })
  .catch(e => console.error(e))

As a result, userData is undefined. Further, such code could cause an unhandledRejection error. The proper code should look like:

getUser()
  .then(user => {
    return getUserData(user)
  })
  .then(userData => {
    // userData is defined
  })
  .catch(e => console.error(e))

Mistake #6: Promisified Synchronous Code

Promises are designed to help you manage asynchronous code. Therefore, there are no advantages to using Promises for synchronous processing. As per the JavaScript documentation: “The Promise object is used for deferred and asynchronous computations. A Promise represents an operation that hasn’t yet completed, but is expected to in the future.” What happens if we wrap synchronous operation in a Promise, as below?

const syncPromise = new Promise((resolve, reject) => {
  console.log('inside sync promise')
  resolve()
})

The function passed to the Promise will be invoked immediately but the resolution will be scheduled on the microtask queue, like any other asynchronous task. It will just be blocking the event loop without a special reason. In the above example, we created an additional context, which we are not using. This will make our code slower and consume additional resources, without any benefits. Furthermore, since our function is a Promise, the JavaScript engine will skip one of the most important code optimizations meant to reduce our function call overhead – automatic function inlining.

Mistake #7: Mixing Promise and Async/Await

const mainMethod = () => {
  return new Promise(async function (resolve, reject) {
    try {
      const data1 = await someMethod()
      const data2 = await someOtherMethod()

      someCallbackMethod(data1, data2, (err, finalData) => {
        if (err) {
          return reject(err)
        }

        resolve(finalData)
      })
    } catch (e) {
      reject(e)
    }
  })
}

This code is long and complicated, it uses Promises, Async/Await, and callbacks. We have an async function inside a Promise, a place where it is not expected and not a good thing to do. This is not expected and can lead to a number of hidden bugs. It will allocate additional Promise objects that will be unnecessarily wasting memory and your garbage collector will spend more time cleaning it.

If you want to use async behavior inside a Promise, you should resolve them in the outer method or use Async/Await to chain the methods inside. We can refactor the code like this:

const somePromiseMethod = (data1, data2) => {
  return new Promise((resolve, reject) => {
    someCallbackMethod(data1, data2, (err, finalData) => {
      if (err) {
        return reject(err)
      }

      resolve(finalData)
    })
  })
}

async function mainMethod() {
  try {
    const data1 = await someMethod()
    const data2 = await someOtherMethod()
    return somePromiseMethod(data1, data2)
  } catch (e) {
    console.error(e)
  }
}

Mistake #8: Async Functions That Return a Promise

async function method() {
  return new Promise((resolve, reject) => { ... })
}

This is unnecessary, a function that returns a Promise doesn’t need an async keyword, and the opposite, when the function is async, you don’t need to write return new Promise inside it. Use the async keyword only if you’re going to use await in the function, and that function will return a Promise when invoked.

function method() {
  return new Promise((resolve, reject) => { ... })
}

or

async function method() {
  const data = await someAsyncMethod()
  return data
}

Mistake #9: Defining a Callback as an Async Function

People often use async functions in places where you wouldn't expect them, like a callback. In the example below, we’re using an async function as a callback on the event from the server:

server.on('connection', async stream => {
  const user = await saveInDatabase()

  console.log(`Someone with ID ${user.id} connected`)
})

This is an anti-pattern. It is not possible to await the result in the EventEmitter callback, the result will be lost. Also, any error that’s thrown (can’t connect to the DB for example) won’t be handled, rather, you’ll get unhandledRejection, and there’s no way to handle it.

Working With Non-Promise Node.js APIs

Promises are a new implementation in ECMAScript and not all Node.js APIs are made to work in that way. That’s why we have a Promisify method that can help us generate a function that returns a Promise from a function that works with a callback:

const util = require('util')

const sleep = util.promisify(setTimeout)

sleep(1000)
  .then(() => console.log('This was executed after one second'))

It was added in Node.js version 8, and you can find out more about it here.

Conclusion

Every new feature in a programming language makes people excited but anxious to try it. But no one wants to spend time trying to understand the functionality; we just want to use it. That’s where the problem arises. The same goes with Promises; we were waiting for them for a long time and now that they are here we’re constantly making mistakes and failing to understand the point of using them.

In my opinion, they’re a cool feature but also quite complicated. In this post, we wanted to show you what’s happening under the hood when you write return new Promise() and some common mistakes that everyone makes. Write your code carefully or one day it may come back to you with some unhandled promise rejections or memory leaks.

If you get into bigger problems with Promises and need some assistance with profiling, we recommend using Node Clinic – a set of tools that can help you diagnose performance issues in your Node.js app.

Safe coding!