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

apollo-server-lambda: Root ("/") path breaks server #5462

Closed
Alec2435 opened this issue Jul 12, 2021 · 10 comments · Fixed by #5497
Closed

apollo-server-lambda: Root ("/") path breaks server #5462

Alec2435 opened this issue Jul 12, 2021 · 10 comments · Fixed by #5497
Labels
size/small Estimated to take LESS THAN A DAY

Comments

@Alec2435
Copy link

Package/Version

This bug relates to apollo-server-lambda 3.0 (this issue did not appear in any 2.x releases)

Expected Behavior

apollo-server-lambda runs regardless of the specific http path used for it within the serverless.yml configuration.

Actual Behavior

I built an example based exactly on the steps found on the tutorial page with the only change being that I used an http path of / instead of graphql within serverless.yml. This is so that the GraphQL server runs at the root of the domain instead of at /graphql. When run, this results in the server responding to all requests with the following message:

<!DOCTYPE html>
<html lang="en">

<head>
	<meta charset="utf-8">
	<title>Error</title>
</head>

<body>
	<pre>Cannot POST /</pre>
</body>

</html>

This does not happen if I revert back to a path of graphql.

In our use case, we simply had defaulted to using the root url for GraphQL and had done so with all previous releases, so we were hoping to keep the paradigm to avoid unnecessary changes to our client applications. It's my understanding that a path of / is the correct way to reference the root url within serverless configurations as it's worked before.

Reproduction Steps

An example for reproduction can be found at pearljobs/apollo-serverless-bug

@glasser
Copy link
Member

glasser commented Jul 12, 2021

Thanks, I'll take a look. To be clear: are you seeing this when running your serverless app locally or when deployed to Lambda or both?

@glasser glasser added this to the MM-2021-07 milestone Jul 12, 2021
@glasser
Copy link
Member

glasser commented Jul 12, 2021

I'll try to reproduce soon (or maybe tomorrow, I'm nearing the end of my day), but I think this is working as intended. By default, all versions of ApolloServer other than apollo-server only handle requests on /graphql, whereas the apollo-server one handles requests at /. If you want it to handle requests at /, you should run server.createHandler({ expressGetMiddlewareOptions: { path: '/' } }).

This is not particularly well-documented right now though (at https://www.apollographql.com/docs/apollo-server/deployment/lambda/ or https://www.apollographql.com/docs/apollo-server/integrations/middleware/#apollo-server-lambda or https://www.apollographql.com/docs/apollo-server/api/apollo-server/#framework-specific-middleware-function).

@glasser
Copy link
Member

glasser commented Jul 14, 2021

We should probably at least get this better documented in the migration guide (and generally document expressGetMiddlewareOptions).

In AS2, apollo-server-lambda only cared about its path to check for health checks. In AS3 it works just like the non-serverless framework integrations: there's a path passed to createHandler and we only watch for GraphQL operations on that path.

Frankly I think the path in the integrations is pretty poor design and the exported middlewares shouldn't be doing any path parsing (you can mount the middleware at a path yourself...) but I thought it would be a pretty jarring change in AS3 to change this for apollo-server-express (there were enough jarring changes already). But I guess it's somewhat jarring to change it for apollo-server-lambda.

While AS3 is officially released, I think we can make some quick changes over the next week that align the serverless integrations back with AS2 behavior. I think we could change the serverless integrations so that (like apollo-server, the batteries-included one) it doesn't have a default path of /graphql. ie, the default expressGetMiddlewareOptions would be {path: '/'}.

@Alec2435
Copy link
Author

@glasser In response to your first question, it was happening both locally and in Lambda.

If the requirement to get it working is just adding something like the { expressGetMiddlewareOptions: { path: '/' } } that's fine by me. Given the breadth of breaking changes with AS3, I don't think it's too big an issue to require the setting, as long as it's more clearly documented. I'm not sure how other lambda users have their config set up, so it's definitely possible I'm in the minority using the / path.

@glasser glasser added 🖇️ lambda size/small Estimated to take LESS THAN A DAY labels Jul 16, 2021
glasser referenced this issue Jul 16, 2021
In Apollo Server 2, none of the serverless integrations paid much
attention to the HTTP request path (other than for health checks).  In
AS3 we rewrote the Lambda and GCF integrations on top of
`apollo-server-express` so they inherited ASE's behavior of only
responding under `/graphql`. This doesn't make that much sense for
serverless integrations, where paths are typically specified in the API
Gateway (etc) configuration, and where functions typically do one thing.
(Frankly, it doesn't make much sense for a default path to be built in
to the framework integrations either rather than letting you specify the
path when you attach the middleware to your app, but changing that in
AS3 would have been one backwards-incompatible change too many).

So we're going to change how AS3 works by saying that serverless
integrations serve on `/`. This is technically incompatible with 3.0.0
but hey, it just came out, and this is more compatible with AS2.

Improve some other docs while we're at it.

Fixes #5472.
@glasser
Copy link
Member

glasser commented Jul 16, 2021

Fix released in 3.0.1 (by #5497).

@glasser glasser closed this as completed Jul 16, 2021
@tiagocpeixoto
Copy link

When I use the example posted by @Alec2435, I got the following error:

{
    "body": "POST body missing, invalid Content-Type, or JSON object has no keys.", 
    "isBase64Encoded": false, 
    "multiValueHeaders":  {
        "access-control-allow-origin": ["*"], 
        "content-length": ["68"], 
        "content-type": ["text/html; charset=utf-8"], 
        "etag": ["W/\"44-LstwwwiSrcEeuZb+mu4TKVPIfxQ\""], 
        "x-powered-by": ["Express"]
    },
    "statusCode": 400
}

I'm executing this code:

   const event  = generateEvent({
            query: `
              query {
                hello
              }
            `,
        });
    const result = await graphqlHandler(event, context);
    expect(JSON.parse(result.body).data.hello).toEqual("Hello world!");

Where the event is generated by:

exports.generateEvent = body => ({
    body: body ? JSON.stringify(body) : null,
    headers: {
        "Accept": "application/json",
        "Content-Type": "application/json",
    },
    multiValueHeaders: {},
    httpMethod: "POST",
    isBase64Encoded: false,
    path: "/",
    pathParameters: null,
    queryStringParameters: null,
    multiValueQueryStringParameters: null,
    stageVariables: null,
    requestContext: {
        accountId: "1234567890",
        apiId: "appid",
        httpMethod: "POST",
        identity: {},
        authorizer: {},
        protocol: "HTTP/1.1",
        path: "/",
        stage: "dev",
        requestId: "test",
        requestTimeEpoch: 0,
        resourceId: "none",
        resourcePath: "/",
    },
    resource: "/",
});

exports.context = {
    callbackWaitsForEmptyEventLoop: false,
    functionName: "",
    functionVersion: "latest",
    invokedFunctionArn: "aws:arn:",
    memoryLimitInMB: "128",
    awsRequestId: "test",
    logGroupName: "test",
    logStreamName: "test",
    getRemainingTimeInMillis() {
        return 0;
    },
    done() {
        // do nothing.
    },
    fail() {
        // do nothing.
    },
    succeed() {
        // do nothing.
    },
}

What am I doing wrong? In v2 it works perfectly!

@glasser
Copy link
Member

glasser commented Jul 20, 2021

(followed up with @tiagocpeixoto in #5504)

@hwillson hwillson removed this from the MM-2021-07 milestone Jul 29, 2021
@maitriyogin
Copy link

Hi,
I just can't seem to get this to work!
In my case I'm creating a federated graph and the / path is for the gateway ..

My setup looks like :

functions:
  gateway:
    handler: src/services/gateway/handler.graphql
    events:
      - http:
          path: / 
          method: post
          cors: true
      - http:
          path: / 
          method: get
          cors: true
const gateway = new ApolloGateway({
  supergraphSdl: new IntrospectAndCompose({
    subgraphs: urls,
  }),
});

export const server = new ApolloServer({
  gateway,
  csrfPrevention: true,
  cache: "bounded",
  plugins: [ApolloServerPluginLandingPageLocalDefault({ embed: true })],
});
export const apolloServerHandler = server.createHandler({
  expressGetMiddlewareOptions: { path: "/" },
});

in the server less offline logging I can see that an endpoint has been setup:

 POST | http://localhost:4001/dev                                         │
   │   POST | http://localhost:4001/2015-03-31/functions/gateway/invocations    │
   │   GET  | http://localhost:4001/dev            

when I try calling the endpoint from the browser I get Cannot GET null

Works fine if I give the path a route ..

deps :

    "@apollo/gateway": "^2.0.5",
    "@apollo/subgraph": "^2.0.5",
    "apollo-server-lambda": "^3.10.1",
    "dataloader": "^2.1.0",
    "graphql": "^16.5.0",

any ideas?
/Stephen.

@apatelvero
Copy link

Hi, I just can't seem to get this to work! In my case I'm creating a federated graph and the / path is for the gateway ..

My setup looks like :

functions:
  gateway:
    handler: src/services/gateway/handler.graphql
    events:
      - http:
          path: / 
          method: post
          cors: true
      - http:
          path: / 
          method: get
          cors: true
const gateway = new ApolloGateway({
  supergraphSdl: new IntrospectAndCompose({
    subgraphs: urls,
  }),
});

export const server = new ApolloServer({
  gateway,
  csrfPrevention: true,
  cache: "bounded",
  plugins: [ApolloServerPluginLandingPageLocalDefault({ embed: true })],
});
export const apolloServerHandler = server.createHandler({
  expressGetMiddlewareOptions: { path: "/" },
});

in the server less offline logging I can see that an endpoint has been setup:

 POST | http://localhost:4001/dev                                         │
   │   POST | http://localhost:4001/2015-03-31/functions/gateway/invocations    │
   │   GET  | http://localhost:4001/dev            

when I try calling the endpoint from the browser I get Cannot GET null

Works fine if I give the path a route ..

deps :

    "@apollo/gateway": "^2.0.5",
    "@apollo/subgraph": "^2.0.5",
    "apollo-server-lambda": "^3.10.1",
    "dataloader": "^2.1.0",
    "graphql": "^16.5.0",

any ideas? /Stephen.

Do you know if there is anyway to remove the environment from end of the url http://localhost:4001/dev. I would just prefer to have localhost:4001/graphql instead of localhost:4001/dev/graphql

@glasser
Copy link
Member

glasser commented Oct 7, 2022

@apatelvero This seems like a question about the serverless tool or AWS Lambda rather than anything Apollo Server specific. As of v3.0.1 apollo-server-lambda shouldn't care much about URL paths, and in the almost-released v4 nothing in Apollo Server will care about URL paths.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Apr 20, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
size/small Estimated to take LESS THAN A DAY
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants