Middy: Promise support

[Industrial]

Hi!

I see that there is support for promises if you use another library with Middy but I don't think it's hard to add "native" promise support to middy.

Just check if a callback parameter is passed. If it is not, return a promise with the error or result instead of calling a callback.

If you want I can create a Pull Request.

Edit: I want this functionality because otherwise all unit tests have to be written using the old callback stryle.

gr,

Tom

[lmammino]

Hello @Industrial, thanks for your comment here.

I am not sure I fully understand your point. Middy supports already native Node.js promises. Is it native Node.js promises that you wanted to add or is it something else?

[lmammino]

Can you also provide an example of unit test (what you are doing and what you would like to be doing). Thanks :)

[Industrial]

api.js:

const middy = require('middy');

const upperCase = async (event, context) => {
  const things = await doStuff();
  const output = await doMoreStuff(things);

  return event.abc.toUpperCase();
}

const handler = middy(upperCase);

module.exports = { handler };

api.test.js:

const { expect } = require('chai');

const api = require('./api');

describe('middy callback', () => {
  it('uses callback pattern', (cb) => {
    api.handler({ abc: 'def' }, { shoop: 'deewoop' }, (error, result) => {
      if (error) return cb(error);
      expect(result).to.equal('DEF');
      cb();
    });
  });
});

describe('middy promise', () => {
  it('does not work', async () => {
    const result = api.handler({ abc: 'def' }, { shoop: 'deewoop' });
    expect(result).to.equal('DEF');
  });
});

[Industrial]

Oops wrong button

[Industrial]

@lmammino ^

[chrisandrews7]

https://github.com/middyjs/middy#promise-support

[Industrial]

@chrisandrews7 Did you not understand my test code above?

Thisis currently not possible inside a test:

const result = api.handler({ abc: 'def' }, { shoop: 'deewoop' })
expect(result).to.equal('DEF')

Middy can take an async function but it creates a callback function:

api.handler({ abc: 'def' }, { shoop: 'deewoop' }, (error, result) => {
  if (error) return cb(error)
  expect(result).to.equal('DEF')
  cb()
})

My argument is: Since AWS can take a async function, why not let middy create one? Let's end this time of callbacks.

[lmammino]

Hello @Industrial and sorry for the very late reply (been very busy lately...).

I think I got now what you mean and it's a very good point indeed.

At the moment I don't feel like I want to change anything to keep supporting node 6.

Anyway since node 6 has been deprecated, one of the next major versions (maybe v.1.0.0 or the version after it), we should stop supporting it. At that point, we might go fully async/await + promises and also simplify a lot the core codebase of middy.

This requires some planning though and I am not sure yet about the timeline.

Any advice is most welcome.

[ChristophP]

Also ran into this and spent some time searching, trying to understand why my handler returns undefined:

const handler: middy(event => Promise.resolve({ test: "hello" }).use(cors())

Calling in a test

handler(event) // returns Promise<undefined> :-/

[eoinsha]

I've come to this issue a few times. As @Industrial summarizes, middy supports promises but it, in wrapping them, it creates a callback-based Lambda handler. This can be problematic when invoking your Lambda in other contexts. Examples are:

• in unit tests (as already outlined)
• in other wrappers. For example, I have a separate DLQ Lambda handler which can be invoked to pull failed messages from a DLQ SQS queue and replay them to the original handler. Because my promise-based handler is converted to a callback by Middy, I have to be aware of this and use a callback.

I would like to see Middy replicating the behavior of the handler as follows.

1. If the handler returns a Promise, the middy'd handler should return a Promise (for explicit Promise returns or async functions)
2. If the handler returns a non-Promise (synchronous return), that value should be returned by the middy'd handler
3. If the handler invokes a callback, the middy'd handler should call the callback with the same results
4. If the handler throws synchronously, the middy'd handler should throw synchronously
5. If a handler's returned Promise rejects (can be a throw in an async function), the middy'd handler's Promise should reject with the same arguments.

I'm looking at creating a dedicated Node module to provide this general-purpose wrapping behavior. Does this behavior look right for Middy, @lmammino?

[lmammino]

Hello @eoinsha and happy to see you here! 👋

I agree that the current behavior is not the most friendly and I am happy to consider changing it to something more suitable for the use cases here described.

Only a few things I would like to consider:

• Any change in 0.x should be backward compatible and keep compatibility with Node 6. This is mostly why right now the baseline becomes the callback driven approach.
• I am thinking the same for 1.0.x as well since it has been out for a while and many people are relying on it even if alpha, but I am more flexible on changing that if we can provide value to the users.

And some questions on the proposed technical implementation:

• How do you intend to verify if the handler is callback or promise driven? Do you plan to do that based on the number of arguments in the function definition (handler.length === 2 or 3)? Another alternative could be to switch dynamically based on the type of usage: at call time we check if a callback is passed, if not we can assume the user, in the specific context, prefers to use the promise-based API. Between the two options, at the moment, I would prefer the latter as it seems to give more flexibility to the user. But I reckon it might be more complicated to implement and there might be non-obvious implications on how do we propagate the call chain across the different phases of execution.

One last comment: if you are planning to create your own middleware library this could be a good source of inspiration fastify/middie (unsurprisingly, it has a very similar name to middy 🤣)

[lmammino]

I forgot to mention that, at some point I was also thinking that we could get totally rid of the callback driven api and go only with the promise based one. That would simplify a lot the codebase and make the various use cases less ambiguous.

There's currently a placeholder issue for 2.0.0: https://github.com/middyjs/middy/issues/356

[eoinsha]

Hi @lmammino 👋 !

• Agree that it makes sense to keep Node 6 support. I don't think this is an issue since async/await is an alternative syntax for Promises and Promises have been around since Node 4
• To be clear, my comments were around Promise/callback use in the Lambda itself, not any middleware. To me, Promise vs. callback support in the middleware library is a separate issue (good to know about #356 !)
• I would hope for no impact on callback users since callbacks will still be honored
• As far as I know, the only way to know if a handler is callback driven is to check its return value. If it returns a Promise, we assume it's Promise based. Otherwise, we have to check if it invokes a callback we pass it. Bear in mind that AWS Lambda will always provide a callback function as it supports both callbacks and Promises. The five rules I put in https://github.com/middyjs/middy/issues/322#issuecomment-521233855 hopefully cover all scenarios we have to detect but this might change when we go to implement.

I'm happy to try out an implementation for this and open a PR. Just wanted to gauge the mood first!

[lmammino]

I am absolutely happy to have a PR covering this. The only thing that I would say is that if the PR breaks any of the current tests, we should target 1.0.0, but it's still very desirable to handle this better.

Thanks a lot for putting all the effort in investigating this and proposing ideas

[duro]

Hey there @lmammino

I am on a pretty recent version of the 1.0 alpha, and this is making it really hard to try and test middy handlers using unit tests. I can't seem to figure out how to get any value back from my handler other than undefined when testing from the outside. I've even tried passing in a callback, but I still get undefined, since my inner handler is async.

[duro]

Forgive me @lmammino, this was actually an error on my end.

My main problem right now in the 1.0 alpha is that the typescript type declarations are wrong, and spit back an error if I don't provide a callback, even though that is a valid option.

So is it safe to say that 1.0 alpha already supports this, it just needs the types fixed?

[lmammino]

Hey @duro, thanks a lot for the update.
Unfortunately, I am not the strongest Typescript person. Would you like to submit a fix for the types you think are wrong?

[Industrial]

Thanks for continuing this work. I'm currently not in the position to use AWS so I can't be of much help due to other responsibilities, but I'm keeping tabs on the ticket =)

[MasroorHamdani]

Do we have this issue resolved?
I am facing same issue with my test cases.

[musayev-io]

+1
Has anyone figured out a not so pleasant hack in the meantime?

[Chuckytuh]

I've just stumbled upon this as well and after digging on the current type definition I ended up on this issue opened by @duro https://github.com/DefinitelyTyped/DefinitelyTyped/issues/38342 that sheds additional light to the problem at hands.

So for now I've just accepted it and I'm doing this

const handler = middy(async () => ({ statusCode: 200 }));
const result = await handler({}, {} as Context, null as unknown as Callback);

Hard casting null to Callback to silence tsc :/

[jan-wilhelm]

Is there any hacky workaround for this that we can just use for now?

[HassanMahmud]

I got here because the result of await was stuck as a pending promise.

The mistake I was making was that I was passing an empty function for a callback when invoking the handler.
When I passed null to the callback then the result of await was as I expected.

[willfarrell]

@HassanMahmud Have you tried v2? We deprecated the callback pattern to make promises way easier to use.