apollo-server-integrations / apollo-server-integration-aws-lambda

An integration to use AWS Lambda as a hosting service with Apollo Server
MIT License
46 stars 9 forks source link

Lambda calls time out after upgrading from 1.0.1 to 1.1.0 #63

Closed joekelley closed 1 year ago

joekelley commented 1 year ago

Hello! I'm building a GraphQL API on AWS Lambda with API Gateway (REST, v1) using this integration. Everything works as expected on 1.0.1, but after upgrading to 1.1.0 all my calls time out even with a 15 second lambda timeout and I get a 502 response with:

{
    "message": "Internal server error"
}

Everything works again when I downgrade and redeploy.

Please let me know if there's any more information I can provide to help debug or if there are any changes in 1.1.0 that I may need to accommodate in my code.

Maybe related to #60?

BlenderDude commented 1 year ago

60 was just an error in the documentation, upon adding the required keys to the example request it properly returned.

I am working on some e2e integration tests that will use CDK to deploy to AWS and confirm validity. I assume I'll be able to reproduce your issue, but if not I'll reach out so we can work on this further!

During initial development I only had a V2 request environment to test against, so its likely some V1 issue slipped through the cracks.

BlenderDude commented 1 year ago

@joekelley can you try this again on V2.0.0? Let me know if you run into any issues. I believe any parsing bugs should now be easier to dissect with the new request handler context so I'm going to close this issue for now, but am good to reopen if the issue persists.

joekelley commented 1 year ago

@BlenderDude unfortunately I'm having the same issue on 2.0.0. I added a logging plugin to my Apollo server and don't see a log for "serverWillStart" so it seems like there's still something up with the event parsing. Let me know if there's anything I can do on my end to get you some more debug info.

BlenderDude commented 1 year ago

Would it be possible for you to send the snippet you are using to create the handler? If you are using infrastructure as code, could you share the setup as well?

One thing I want you to double check is that the V1 gateway configuration is a Proxy integration, as that is the event type we are expecting.

joekelley commented 1 year ago

Sure! Here's our handler definition:

import { startServerAndCreateLambdaHandler, handlers } from '@as-integrations/aws-lambda';
import { server } from './server';
import { contextFunction } from './context';
import { Handler } from 'aws-lambda';
import { injectLambdaContext } from '@aws-lambda-powertools/logger';
import middy from '@middy/core';
import { logger } from '../utils/logger';
import { isDevelopmentEnvironment } from '../utils/env';

logger.appendKeys({ function: 'graphql' });

const lambdaHandler: Handler = startServerAndCreateLambdaHandler(
  server,
  handlers.createAPIGatewayProxyEventRequestHandler(),
  { context: contextFunction }
);

export const handler = middy(lambdaHandler).use(injectLambdaContext(logger, { logEvent: isDevelopmentEnvironment() }));

and our CDK:

    const graphqlFunction = new NodejsFunction(this, 'function', {
      environment: {
        ENV: env,
      },
      architecture: Architecture.ARM_64,
      runtime: Runtime.NODEJS_16_X,
      timeout: Duration.seconds(30),
      tracing: Tracing.ACTIVE,
    });

    const lambdaRestApi = new LambdaRestApi(this, 'my-graphql', {
      handler: graphqlFunction,
      restApiName: 'My API',
      deployOptions: {
        stageName: env,
        metricsEnabled: true,
        loggingLevel: MethodLoggingLevel.INFO,
        dataTraceEnabled: true,
        tracingEnabled: true,
      },
    });
BlenderDude commented 1 year ago

The : Handler in your code snippet infers to me that you are using a Typescript file. (On an unrelated note, you shouldn't have to manually type the handler, the startServerAndCreateLambdaHandler actually emits a fully typed Handler, with the Event and Result populated properly based on your handlers.createAPI... call).

How are you pulling the Lambda code into your new NodeJsFunction construct in the CDK? It seems like there is nothing in the properties. By default it will pull based on your stack file name. Can you confirm that the function filename is [[STACK_FILENAME]].function.ts to comply with the default?

Have you pulled the built asset getting pushed to lambda and confirmed the result looks as you expect? I always end up using the docker-based build method as the default esbuild can sometimes produce some unexpected results.

joekelley commented 1 year ago

How are you pulling the Lambda code into your new NodeJsFunction construct in the CDK? It seems like there is nothing in the properties. By default it will pull based on your stack file name. Can you confirm that the function filename is [[STACK_FILENAME]].function.ts to comply with the default?

Confirmed, we accommodate the default.

Have you pulled the built asset getting pushed to lambda and confirmed the result looks as you expect? I always end up using the docker-based build method as the default esbuild can sometimes produce some unexpected results.

I just tried building with docker instead. The bundle size is a bit different but the behavior is the same, the lambda times out. I haven't compared the built assets yet.

I also had to make changes to the typing of my context function to satisfy typescript but I didn't make any code changes, maybe there's a breaking change there that I missed between 1.0.1 and 2.0.0? My context function looks like this (note that the log doesn't fire):

export type GraphQLContext = Record<string, never>;

export const contextFunction: ContextFunction<
  [LambdaContextFunctionArgument<handlers.RequestHandler<APIGatewayProxyEvent, APIGatewayProxyResult>>],
  GraphQLContext
> = async (integrationContext) => {
    logger.info('context_function_entered', { integrationContext: integrationContext });
...
  return {};
}
joekelley commented 1 year ago

I found the problem between my keyboard and chair; turns I didn't give my lambda function enough memory. After bumping to 256 all is well. Thanks for working with me!

BlenderDude commented 1 year ago

@joekelley so glad to hear this!! I'll make a note that mentions this in the README. Is the graph or app particularly large? 128MB of ram is still quite a bit and I'm surprised it would've halted with an out of memory error if it's just Apollo server.

robpark commented 1 year ago

No, in fact it's pretty small. Though we do have xray and some middy logging in there if that might impact the footprint.

@rob

On Wed, Apr 19, 2023 at 7:01 PM Daniel Abdelsamed @.***> wrote:

@joekelley https://github.com/joekelley so glad to hear this!! I'll make a note that mentions this in the README. Is the graph or app particularly large? 128MB of ram is still quite a bit and I'm surprised it would've halted with an out of memory error if it's just Apollo server.

— Reply to this email directly, view it on GitHub https://github.com/apollo-server-integrations/apollo-server-integration-aws-lambda/issues/63#issuecomment-1515487938, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAAZZBDLNRXE335ODESQJQTXCBVERANCNFSM6AAAAAAS63QBMQ . You are receiving this because you are subscribed to this thread.Message ID: <apollo-server-integrations/apollo-server-integration-aws-lambda/issues/63/1515487938 @github.com>