Issues with async/await in handlers

[richardgarnier]

When playing with the library, I found two issues when using asynchronous code in my handlers. I'm using it with koa, but I think the problem would be the same for every framework.

First, the calls to the handlers are not awaited, meaning I can't do async work in the handler.
There is a possible fix here.

Second, its a really small one but since we only have one instance of the validation function, the validation errors can be lost in the case a async call is made in the error handler.
For example, is the following handler is registered as validationFail, c.validation.errors in the body can contains a different error.

module.exports = async (c, req) => {
  await logToElasticsearch(c.validation.errors);
  req.response.status = 400;
  req.response.body = c.validation.errors;
};

This is obviously easy to fix like this:

module.exports = async (c, req) => {
  const errors = c.validation.errors;
  await logToElasticsearch(errors);
  req.response.status = 400;
  req.response.body = errors;
};

That being said I made a small patch for this aswell.

Feel free to reuse any of this, and/or close the issue.

[anttiviljami]

Great work!

As far as I know, when returning an async function (i.e. a function that retuns a promise) I'm pretty sure you shouldn't need to await. Could you give me a working example where this is not the case? I haven't run into this issue with my async handlers yet.

For the second problem, I think this upcoming change will actually fix the issue since we are no longer returning the validator function in the context object, but a similar validation result object with just a shallow copy of the errors (I think). Can you verify?

https://github.com/anttiviljami/openapi-backend/pull/8/files#diff-7d4dec20edd9d5dea9fe6a05f70cf757R158

[anttiviljami]

I mean I don't really see any problem with adding the awaits, but I'm just really curious to see why async work can't be performed.

Not adding the await just seems better in my eyes so users have an option to do non-async handlers as well.

export type Handler = (context?: Context, ...args: any[]) => any | Promise<any>;

[richardgarnier]

You're actually right for the first one, I feel kind of dumb now :'(.
I added the await at the same time on api.handleRequest and inside handleRequest not realizing the latter was actually useless...
I'm going to try the PR for the second issue now.

[richardgarnier]

Yes I confirm that the second issue is fixed when running the code inside the PR.
Thanks for checking.

[anttiviljami]

Great! Closing this issue now and publishing changes from #6. :)