loading...

Common Node.js mistakes in Lambda

theburningmonk profile image Yan Cui Updated on ・3 min read

I have helped quite a few clients with their Node.js serverless projects. In doing so I have seen some recurring mistakes around async/await.

Still using callbacks

Many people are still using the callbacks in their async handler functions:

module.exports.handler = async (event, context, cb) => {
  const response = {
    statusCode: 200,
    body: JSON.stringify({ message: 'hello world' })
  }

  cb(null, response)
}

instead of the simpler alternative:

module.exports.handler = async (event, context) => {
  const response = {
    statusCode: 200,
    body: JSON.stringify({ message: 'hello world' })
  }

  return response
}

Not using promisify

Before Node8, bluebird filled a massive gap. It provided the utility to convert callback-based functions to promise-based. But Node8's built-in util module has filled that gap with the promisify function.

For example, we can now transform the readFile function from the fs module like this:

const fs = require('fs')
const { promisify } = require('util')
const readFile = promisify(fs.readFile)

No need to use bluebird anymore. That's one less dependency, which helps reduce the cold start time for our functions.

Too sequential

async/await lets you write asynchronous code as if they're synchronous, which is awesome. No more dealing with callback hell!

On the flip side, we can also miss a trick and not perform tasks concurrently where appropriate.

Take the following code as an example:

async function getFixturesAndTeam(teamId) {
  const fixtures = await fixtureModel.fetchAll()
  const team = await teamModel.fetch(teamId)
  return {
    team,
    fixtures: fixtures.filter(x => x.teamId === teamId)
  }
}

This function is easy to follow, but it's hardly optimal. teamModel.fetch doesn't depend on the result of fixtureModel.fetchAll, so they should run concurrently.

Here is how you can improve it:

async function getFixturesAndTeam(teamId) {
  const fixturesPromise = fixtureModel.fetchAll()
  const teamPromise = teamModel.fetch(teamId)

  const fixtures = await fixturesPromise
  const team = await teamPromise

  return {
    team,
    fixtures: fixtures.filter(x => x.teamId === teamId)
  }
}

In this version, both fixtureModel.fetchAll and teamModel.fetch are started concurrently.

You also need to watch out when using map with async/await. The following will call teamModel.fetch one after another:

async function getTeams(teamIds) {
  const teams = _.map(teamIds, id => await teamModel.fetch(id))
  return teams
}

Instead, you should write it as the following:

async function getTeams(teamIds) {
  const promises = _.map(teamIds, id => teamModel.fetch(id))
  const teams = await Promise.all(promises)
  return teams
}

In this version, we map teamIds to an array of Promise. We can then use Promise.all to turn this array into a single Promise that returns an array of teams.

In this case, teamModel.fetch is called concurrently and can significantly improve execution time.

async/await inside forEach()

This is a tricky one, and can sometimes catch out even experienced Node.js developers.

The problem is that code like this doesn't behave the way you'd expect it to:

[ 1, 2, 3 ].forEach(async (x) => {
  await sleep(x)
  console.log(x)
})

console.log('all done.')

When you run this you'll get the following output:

all done.

See this post for a longer explanation about why this doesn't work. For now, just remember to avoid using async/await inside a forEach!

Not using AWSSDK’s .promise()

Did you know that the AWS SDK clients support both callbacks and promises? To use async/await with the AWS SDK, add .promise() to client methods like this:

const AWS = require('aws-sdk')
const Lambda = new AWS.Lambda()

async function invokeLambda(functionName) {
  const req = {
    FunctionName: functionName,
    Payload: JSON.stringify({ message: 'hello world' })
  }
  await Lambda.invoke(req).promise()
}

No more callback functions, yay!

Wrap-up

That's it, 5 common mistakes to avoid when working with Node.js in Lambda. Follow me for more tips on building production-ready serverless applications and operational best practices.

Check out theburningmonk.com for more of my articles.

Discussion

pic
Editor guide
Collapse
dmh2000 profile image
david howard

const sleep = (ms) => new Promise(r => setTimeout(r, ms)); // from the linked post

[1, 2, 3].forEach(async (num) => {
await sleep(50);
console.log(num);
});
console.log('all done');

prints:
F:\projects>node x.js
Done
1
2
3

F:\projects>

I understand why 'all done' comes first, but the console.log's all print as expected. of course they come after 'all done', but the article and the link don't explain why the console.logs wouldn't work, because it appears they do. It might be because the linked article used a preproduction v 7 node where async/await/promises weren't quite working. or something.

node 10.x

Collapse
theburningmonk profile image
Yan Cui Author

Yeah, I see the same behaviour as you in Node 8, so it's possible the console.log issue is related to Node7.

But it wasn't the problem that I was hoping to highlight (my bad!), which is that it doesn't behave as you'd expect when you await sleep(50) inside an async function. It has stung me in the past where we had something along the lines of

xs.forEach(async (x) => {
  const result = await doSomething(x);
  // do something with results
})

doSomethingElseAfterAllResultsAreProcessed();

which fails because by the time doSomethingElseAfterAllResultsAreProcessed runs the results haven't actually been processed in the forEach. Once you understand the problem it's easy to see why it behaved that way, but it wasn't intuitive as the time of writing the code (which is why I actually made this mistake multiple times before I eventually "learnt the lesson").

Collapse
theburningmonk profile image
Yan Cui Author

haha, you're not alone in this - I have run into the same problem (it's not isolated to Lambda by the way) multiple times myself before it's finally carved in stone in my head. It's a systematic failure on the part of the language that it doesn't work intuitively, and unless it improves at the language level we have to (sadly) rely on memorization and building up muscle memory individually.

Collapse
pavelloz profile image
Paweł Kowalski

Great post! Im still guilty of some errors after all those years ;)

Collapse
theburningmonk profile image
Yan Cui Author

Thank you, I've made these mistakes before as well. That forEach thing has stung me multiple times and it was a pain to debug!

Collapse
fjanon profile image
Fred Janon

Why using a callback instead of an async function would be a mistake? Why wouldn't the callback work or be less efficient?

Collapse
theburningmonk profile image
Yan Cui Author

You can use callback instead of async, but probably shouldn't mix the two as it gives off the impression that you can return early before the async function's promise resolves. Which is not the case, the Lambda invocation will finish and return to the caller with a response when the async function's promise resolves at the end.