Middy: Timeout caused when used with `knex`

[willfarrell]

I've bee trying to debug this over the past day. Basically when knex and middy are used together the lambda function times out. Note that when this time out happens console.log prints out the expected value, and runs all middleware as expected. Also worth noting, works perfectly when run locally.

node8.10

const middy = require('middy')
const knex = require('knex')({
  client: 'pg',
  connection: process.env.PGURL
})

/*
Permutations:
1) knex + middy => timeout
2) !knex + middy => success
3) knex + !middy => success
4) !knex + !middy => success
 */
const app = async (event, context) => {
  const sql = knex.select(['id', 'name']).from('datastream.programs').limit(1)

  // knex options
  const rows = await sql  // knex
  //const rows = await Promise.resolve([{"id":"d6683390-f10c-433f-b0fd-b21fa66cc456","name":"test"}])  // !knex
  console.log(rows)

  return {
    statusCode: 200,
    body: JSON.stringify({
      data: rows
    })
  }
}

// middy options
const handler = middy(app)  // middy
//const handler = app // !middy

module.exports = { handler }

Edit:
I've run out of idea on what it could be. I've even confirmed middy is calling the callback with the expected response. (Really nice and simple code btw)

I have a support ticket open with AWS, hopefully they can shed some light on this. I have a feeling they modified node in some way.

[lmammino]

I used knex myself only to write migrations script, so I am not sure what happens when you use it as an ORM and how it manages the connection to Postgres. With that in mind, I suspect that knex will keep the connection open if you don't close it explicitely.

In such case you would have two solutions:

1. Use the doNotWaitForEmptyEventLoop middleware. This will make sure that, once you invoke the callback, the lambda is terminated even if the empty loop is still not empty (which happens when your DB connection is kept open)
2. Force the connection to be closed (check knex docs for reference). Keep in mind that this last approach might cause some relevant performance issues as the connection will be recreated per every function execution, so if you are using this lambda for an API (as it seems) I'd recommend the first approach.

Let me know if this helps! 😉

[willfarrell]

Oh my god - doNotWaitForEmptyEventLoop solved it. Thank you so much. If you're ever in Calgary or Banff Canada, let me know drinks on me. Seriously.

[lmammino]

ahaha thanks @willfarrell :)
Same for you if you come in Dublin!

[morficus]

oooooff. this should 100% be in the documentation.

I just spent north of 4 hours pealing through stuff just to conclude that knex + middy don't play well together. was about to open a bug when I found this issue.

[lmammino]

Hello @morficus, sorry to hear that. Would you like to submit a PR to update the docs? :)

[morficus]

@lmammino sure! just point me to the right repo.
I see there is a "doc" directory in this repo but that seems to be full documentation is available on https://middy.js.org

[lmammino]

Sorry for not being more specific @morficus.

So the docs on the main website are generated from the README file. We have two options in my opinion, either we add a dedicated generic section on the README (something maybe titled "Avoid lambda timeouts") where we explain the generic issue and maybe we can make the Knex as an example, or we can do that in a middleware. In the latter case, I believe the doNotWaitForEmptyEventLoop might be the best place.

At the moment I am more for having that in the README, because it might be easier to find even before having the problem in the first place.

What do you think?

PS: also take into account that if we do the change in the current branch (master = 0.x), we should ideally update also the docs in branch 1.0.0-alpha.

[stavros-zavrakas]

@lmammino I had the same issue today with @morficus with mysql and sequelizejs. I don't know if finally it is part of the docs but I almost gave up until I found that issue. I think that this will be the case for most of the people (having a middleware or a function that will provide the database connection) and I think that it will help a lot of people if it will be in a bit more obvious place (even a main section in the homepage)