Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Validation error with async handler results in timeout #95

Closed
FranzBusch opened this issue Jan 19, 2018 · 13 comments
Closed

Validation error with async handler results in timeout #95

FranzBusch opened this issue Jan 19, 2018 · 13 comments

Comments

@FranzBusch
Copy link

When using an async handler and during the validation, an error occurs the function times out. The code below results in a timeout for me

const createHandler: Handler = async (event: any, context: Context, callback: Callback) => {
    await exampleAsyncMethod
    const { name } = event.body
    return okResponse
}

export const handler = middy(createHandler)
    .use(eventLoopMiddleware())
    .use(validator({ inputSchema }))
    .use(httpErrorHandler())

Changing it to a none async handler it works fine.


const createHandler: Handler = (event: any, context: Context, callback: Callback) => {
    exampleAsyncMethod.then(
          // code
    })
}

export const handler = middy(createHandler)
    .use(eventLoopMiddleware())
    .use(validator({ inputSchema }))
    .use(httpErrorHandler())

I am using my own middleware for the event loop because of this issue. Is there any reason why async handlers are not working with the middleware?

@FranzBusch
Copy link
Author

Quick update. The code does not result in a timeout anymore but in an internal server error. The validation error is logged in cloud watch but the httpError middleware is not catching it.

Code looks like this:

const inputSchema = {
    type: 'object',
    properties: {
        body: {
            properties: {
                name: { type: 'string' }
            }
        }
    }
}

const createHandler: Handler = async (event: any, context: Context, callback: Callback) => {
    const con = await connection
    const { name } = event.body
    const advertisementRepository = con.getRepository(Advertisement)
    const existingAdvertisement = await advertisementRepository.findOne({ name: name })

    if (existingAdvertisement !== undefined) {
        throw new httpError.Conflict()
    }

    let advertisement = new Advertisement()
    advertisement.name = name

    await advertisementRepository.save(advertisement)
    return okResponse
}

export const handler = middy(createHandler)
    .use(eventLoopMiddleware())
    .use(jsonBodyParser())
    .use(validator({ inputSchema }))
    .use(httpErrorHandler())

@lmammino
Copy link
Member

I will try to troubleshoot this ASAP, but at a first glance i would say that the error is not being catched because the error is not using the http-errors class

@lmammino
Copy link
Member

Your code is missing some important parts for me to be able to debug it. I stick with the original hypothesis that your code is generating a non http exception somewhere and thus it is not catched by the http error middleware.

I recreated the following example following most of your steps and it seems to work as expected (also converted to normal js for convenience, to be run in node 8+ for async support). feel free to use it as a starting point to recreate a similar example that we can use to troubleshoot your issue further:

const middy = require('middy')
const { doNotWaitForEmptyEventLoop, jsonBodyParser, validator, httpErrorHandler } = require('middy/middlewares')

const inputSchema = {
    type: 'object',
    properties: {
        body: {
            properties: {
                name: { type: 'string' }
            }
        }
    }
}

const createHandler = async (event, context, callback) => {
    return ({
      statusCode: 200,
      body: "ok"
    })
}

module.exports = middy(createHandler)
    .use(doNotWaitForEmptyEventLoop())
    .use(jsonBodyParser())
    .use(validator({ inputSchema }))
    .use(httpErrorHandler())

const handler = require('./handler')

const event = {
  body: {
    name: "Test"
  }
}

handler(event, {}, (error, response) => {
  console.log({ error, response })
})

@FranzBusch
Copy link
Author

Can't reproduce it right now. I will reopen if I do

@ffxsam
Copy link

ffxsam commented Feb 2, 2021

@lmammino

I'm having the same problem. @middy/validator just throws an error, where I would expect it to just return an error to the client neatly.

const test = async event => {
  return {
    statusCode: 200,
    body: JSON.stringify({ event }),
  };
};

export const main = middyfy(test, schema);
import middy from '@middy/core';
import jsonBodyParser from '@middy/http-json-body-parser';
import httpErrorHandler from '@middy/http-error-handler';
import validator from '@middy/validator';

export const middyfy = (handler, inputSchema) => {
  return middy(handler)
    .use(jsonBodyParser())
    .use(validator({ inputSchema }))
    .use(httpErrorHandler());
};

And the schema:

export default {
  type: 'object',
  properties: {
    name: { type: 'string' },
  },
  required: ['name'],
} as const;

The client hangs, and on the server side (running locally using serverless-offline) I see:

BadRequestError: Event object failed validation
    at before (projv2-api/node_modules/@middy/validator/index.js:62:23)
    at runNext (projv2-api/node_modules/@middy/core/index.js:78:24)
    at before (projv2-api/node_modules/@middy/http-json-body-parser/index.js:24:5)
    at runNext (projv2-api/node_modules/@middy/core/index.js:78:24)
    at runMiddlewares (projv2-api/node_modules/@middy/core/index.js:99:3)
    at projv2-api/node_modules/@middy/core/index.js:173:7
    at new Promise (<anonymous>)
    at instance (projv2-api/node_modules/@middy/core/index.js:159:26)
    at InProcessRunner.run (projv2-api/node_modules/serverless-offline/dist/lambda/handler-runner/in-process-runner/InProcessRunner.js:178:16)
    at processTicksAndRejections (internal/process/task_queues.js:97:5) {
  details: [
    {
      keyword: 'required',
      dataPath: '',
      schemaPath: '#/required',
      params: [Object],
      message: 'should have required property name'
    }
  ]
}
{
  headers: { 'Content-Type': 'plain/text' },
  statusCode: 400,
  body: 'Event object failed validation'
}
{
  headers: { 'Content-Type': 'plain/text' },
  statusCode: 400,
  body: 'Event object failed validation'
}
(node:84720) UnhandledPromiseRejectionWarning: #<Object>
(node:84720) 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(). To terminate the node process on unhandled promise rejection, use the CLI flag `--unhandled-rejections=strict` (see https://nodejs.org/api/cli.html#cli_unhandled_rejections_mode). (rejection id: 4)
(node:84720) [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.

When deployed to AWS, the client doesn't hang, but I get this immediately:

{
  "message": "Internal Server Error"
}

@willfarrell
Copy link
Member

@ffxsam What version are you running? #609 (comment)

@ffxsam
Copy link

ffxsam commented Feb 2, 2021

1.5.0

@willfarrell
Copy link
Member

AWS Lambda only supports up to node v12 (without esm support). CloudWatch should tell you what the issues it.

@ffxsam
Copy link

ffxsam commented Feb 2, 2021

I don't think that's the issue here.

I've gotten down to the bottom of it, I think: @middy/validator is throwing an error. I was under the impression that if we use the validator middleware, it would simply return something like this (without throwing an error):

{
  statusCode: 400,
  body: JSON.stringify(validationErrors)
}

Maybe I'm misunderstanding how it's intended to be used. But throwing an error inside a Lambda behind an API Gateway route causes the server to simply return "Internal server error" which isn't very useful.

@ffxsam
Copy link

ffxsam commented Feb 2, 2021

Nvm, got it. Accidentally mixed versions again (http-error-handler was at 2.x).

This should also maybe take care of @FranzBusch 's issue too. Adding the @middy/http-error-handler middleware will catch the error thrown by the validator and return a nice & neat response body.

@willfarrell
Copy link
Member

Yeah, only the message is returned to ensure secure by default and the error response from the middleware is not in any specific error format. Thus needs to be handled custom to meet specific needs, like you just posted.

Yeah, mixing version will cause unexpected issues. v2 is a big breaking change.

@ffxsam
Copy link

ffxsam commented Feb 2, 2021

Thanks! I wound up using my own middleware instead of http-error-handler, because it didn't return anything more than "Event object failed validation". I'm sending the whole error payload to the client.

@willfarrell
Copy link
Member

Yes, Event object failed validation is expected. That's the error message.

Yeah, only the message is returned to ensure secure by default and the error response from the middleware is not in any specific error format. Thus needs to be handled custom to meet specific needs, like you just posted.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

4 participants