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

Consider replacing apollo-server-lambda with apollo-server-express + wrapper #5078

Closed
glasser opened this issue Apr 1, 2021 · 1 comment
Closed

Comments

@glasser
Copy link
Member

glasser commented Apr 1, 2021

I am getting more and more convinced that continuing to add more and more features to apollo-server-lambda (and other serverless frameworks) that just make up for their lack of middleware structure (eg, more and more complex cors options) is not a great use of this project's efforts. I'd like to see whether we can replace the entirety of apollo-server-lambda with something that wraps apollo-server-express with something like @vendia/serverless-lambda to convert an Express app into a Lambda app. Then we only have to implement features once in apollo-server-express, and folks can use standard Express libraries to implement other features.

This might mean "apollo-server-lambda is a small package that combines those two packages" or even "apollo-server-lambda is removed from AS3 and docs teach you how to use @vendia/serverless-lambda itself.

Note that despite the name, this package is not related to https://github.com/serverless/serverless

@glasser glasser added this to the Release 3.x milestone Apr 1, 2021
glasser added a commit that referenced this issue May 13, 2021
Before this change, apollo-server-lambda contained a lot of code that tried to
understand the formats of Lambda event (request) and result (response)
formats. It tried to work with APIGatewayProxy messages of payloadFormatVersion
1.0 and 2.0 as well as ALB.

But understanding the intricacies of the various Lambda message formats isn't
really what the Apollo Server project is about. A pretty large fraction of all
maintenance on Apollo Server in 2021 has gone into tweaking details of the
Lambda event parsing and making the Lambda handler support all AS features
without the help of a library supporting composable middleware.

This PR (targeted for AS3) throws away the laboriously constructed bespoke
Lambda parsing and faux-middleware implementation and replaces it with two
packages that solve these problems for us: `@vendia/serverless-express` which
understands a variety of Lambda input and output formats and converts them to
Express format, and `express`, the most popular Node library for defining HTTP
server behavior. (Note that `@vendia/serverless-express` is not related to the
`serverless` CLI/framework.)

Now `apollo-server-lambda` is just a convenience wrapper around
`apollo-server-express`. It has to deal with the difference in startup logic
between serverless and non-serverless environments, but it doesn't have to
reimplement all of the Lambda and HTTP logic from scratch.

As an added advantage, you can now optionally provide your own express app to
`apollo-server-lambda`. Previously, `apollo-server-lambda` gave no real way to
customize any of its behavior past the particular options we define, because
Lambda doesn't have a built-in middleware framework. Since we are removing some
built-in features like `graphql-upload` integration in AS3, it's important that
we continue a way to add custom behavior to your Lambda server. Letting you
define that custom behavior with a standard Express app seems reasonable.

We recognize that Lambda users generally care strongly about bundle size, so
adding two new dependencies may seem problematic. That said, we don't currently
have a principled way of evaluating Lambda bundle sizes when we make choices in
this project, and compared to other dependencies of Apollo Server, these new
dependencies are not very large. For now, the improvement in maintainability and
flexibility seems worth the bundle size increase. If `apollo-server-lambda`
users want to help out with a new project of focusing on Lambda bundle size
optimization, we can work together to define benchmarks based on realistic
build/bundler conditions, and find other ways to reduce bundle size (eg, there
is a fair amount of low hanging fruit inside `apollo-reporting-protobuf`).

Fixes #5078. Fixes #4951 (because that API is just "Express").
glasser added a commit that referenced this issue May 13, 2021
Before this change, apollo-server-lambda contained a lot of code that tried to
understand the formats of Lambda event (request) and result (response)
formats. It tried to work with APIGatewayProxy messages of payloadFormatVersion
1.0 and 2.0 as well as ALB.

But understanding the intricacies of the various Lambda message formats isn't
really what the Apollo Server project is about. A pretty large fraction of all
maintenance on Apollo Server in 2021 has gone into tweaking details of the
Lambda event parsing and making the Lambda handler support all AS features
without the help of a library supporting composable middleware.

This PR (targeted for AS3) throws away the laboriously constructed bespoke
Lambda parsing and faux-middleware implementation and replaces it with two
packages that solve these problems for us: `@vendia/serverless-express` which
understands a variety of Lambda input and output formats and converts them to
Express format, and `express`, the most popular Node library for defining HTTP
server behavior. (Note that `@vendia/serverless-express` is not related to the
`serverless` CLI/framework.)

Now `apollo-server-lambda` is just a convenience wrapper around
`apollo-server-express`. It has to deal with the difference in startup logic
between serverless and non-serverless environments, but it doesn't have to
reimplement all of the Lambda and HTTP logic from scratch.

As an added advantage, you can now optionally provide your own express app to
`apollo-server-lambda`. Previously, `apollo-server-lambda` gave no real way to
customize any of its behavior past the particular options we define, because
Lambda doesn't have a built-in middleware framework. Since we are removing some
built-in features like `graphql-upload` integration in AS3, it's important that
we continue a way to add custom behavior to your Lambda server. Letting you
define that custom behavior with a standard Express app seems reasonable.

We recognize that Lambda users generally care strongly about bundle size, so
adding two new dependencies may seem problematic. That said, we don't currently
have a principled way of evaluating Lambda bundle sizes when we make choices in
this project, and compared to other dependencies of Apollo Server, these new
dependencies are not very large. For now, the improvement in maintainability and
flexibility seems worth the bundle size increase. If `apollo-server-lambda`
users want to help out with a new project of focusing on Lambda bundle size
optimization, we can work together to define benchmarks based on realistic
build/bundler conditions, and find other ways to reduce bundle size (eg, there
is a fair amount of low hanging fruit inside `apollo-reporting-protobuf`).

Fix inconsistency in the content-type returned from health checks (the old
Lambda used `application/json` instead of `application/health+json` like most
other integrations).

This new version passes all of the existing integration tests (plus the
`testApolloServer` suite from
`apollo-server-integration-testsuite/src/ApolloServer.ts` which wasn't being run
previously!) essentially out of the box. (I fleshed out the "mock server"
implementation which converts from Node http requests to Lambda events a bit
more, and changed, but no "core" code or test changes were needed other than
fixing the health check `content-type`.)

Fixes #5078. Fixes #4951 (because that API is just "Express").
glasser added a commit that referenced this issue May 13, 2021
Before this change, apollo-server-lambda contained a lot of code that tried to
understand the formats of Lambda event (request) and result (response)
formats. It tried to work with APIGatewayProxy messages of payloadFormatVersion
1.0 and 2.0 as well as ALB.

But understanding the intricacies of the various Lambda message formats isn't
really what the Apollo Server project is about. A pretty large fraction of all
maintenance on Apollo Server in 2021 has gone into tweaking details of the
Lambda event parsing and making the Lambda handler support all AS features
without the help of a library supporting composable middleware.

This PR (targeted for AS3) throws away the laboriously constructed bespoke
Lambda parsing and faux-middleware implementation and replaces it with two
packages that solve these problems for us: `@vendia/serverless-express` which
understands a variety of Lambda input and output formats and converts them to
Express format, and `express`, the most popular Node library for defining HTTP
server behavior. (Note that `@vendia/serverless-express` is not related to the
`serverless` CLI/framework.)

Now `apollo-server-lambda` is just a convenience wrapper around
`apollo-server-express`. It has to deal with the difference in startup logic
between serverless and non-serverless environments, but it doesn't have to
reimplement all of the Lambda and HTTP logic from scratch.

As an added advantage, you can now optionally provide your own express app to
`apollo-server-lambda`. Previously, `apollo-server-lambda` gave no real way to
customize any of its behavior past the particular options we define, because
Lambda doesn't have a built-in middleware framework. Since we are removing some
built-in features like `graphql-upload` integration in AS3, it's important that
we continue a way to add custom behavior to your Lambda server. Letting you
define that custom behavior with a standard Express app seems reasonable.

We recognize that Lambda users generally care strongly about bundle size, so
adding two new dependencies may seem problematic. That said, we don't currently
have a principled way of evaluating Lambda bundle sizes when we make choices in
this project, and compared to other dependencies of Apollo Server, these new
dependencies are not very large. For now, the improvement in maintainability and
flexibility seems worth the bundle size increase. If `apollo-server-lambda`
users want to help out with a new project of focusing on Lambda bundle size
optimization, we can work together to define benchmarks based on realistic
build/bundler conditions, and find other ways to reduce bundle size (eg, there
is a fair amount of low hanging fruit inside `apollo-reporting-protobuf`).

Fix inconsistency in the content-type returned from health checks (the old
Lambda used `application/json` instead of `application/health+json` like most
other integrations).

This new version passes all of the existing integration tests (plus the
`testApolloServer` suite from
`apollo-server-integration-testsuite/src/ApolloServer.ts` which wasn't being run
previously!) essentially out of the box. (I fleshed out the "mock server"
implementation which converts from Node http requests to Lambda events a bit
more, and changed, but no "core" code or test changes were needed other than
fixing the health check `content-type`.)

Fixes #5078. Fixes #4951 (because that API is just "Express").
glasser added a commit that referenced this issue May 21, 2021
Before this change, apollo-server-lambda contained a lot of code that tried to
understand the formats of Lambda event (request) and result (response)
formats. It tried to work with APIGatewayProxy messages of payloadFormatVersion
1.0 and 2.0 as well as ALB.

But understanding the intricacies of the various Lambda message formats isn't
really what the Apollo Server project is about. A pretty large fraction of all
maintenance on Apollo Server in 2021 has gone into tweaking details of the
Lambda event parsing and making the Lambda handler support all AS features
without the help of a library supporting composable middleware.

This PR (targeted for AS3) throws away the laboriously constructed bespoke
Lambda parsing and faux-middleware implementation and replaces it with two
packages that solve these problems for us: `@vendia/serverless-express` which
understands a variety of Lambda input and output formats and converts them to
Express format, and `express`, the most popular Node library for defining HTTP
server behavior. (Note that `@vendia/serverless-express` is not related to the
`serverless` CLI/framework.)

Now `apollo-server-lambda` is just a convenience wrapper around
`apollo-server-express`. It has to deal with the difference in startup logic
between serverless and non-serverless environments, but it doesn't have to
reimplement all of the Lambda and HTTP logic from scratch.

As an added advantage, you can now optionally provide your own express app to
`apollo-server-lambda`. Previously, `apollo-server-lambda` gave no real way to
customize any of its behavior past the particular options we define, because
Lambda doesn't have a built-in middleware framework. Since we are removing some
built-in features like `graphql-upload` integration in AS3, it's important that
we continue a way to add custom behavior to your Lambda server. Letting you
define that custom behavior with a standard Express app seems reasonable.

We recognize that Lambda users generally care strongly about bundle size, so
adding two new dependencies may seem problematic. That said, we don't currently
have a principled way of evaluating Lambda bundle sizes when we make choices in
this project, and compared to other dependencies of Apollo Server, these new
dependencies are not very large. For now, the improvement in maintainability and
flexibility seems worth the bundle size increase. If `apollo-server-lambda`
users want to help out with a new project of focusing on Lambda bundle size
optimization, we can work together to define benchmarks based on realistic
build/bundler conditions, and find other ways to reduce bundle size (eg, there
is a fair amount of low hanging fruit inside `apollo-reporting-protobuf`).

Fix inconsistency in the content-type returned from health checks (the old
Lambda used `application/json` instead of `application/health+json` like most
other integrations).

This new version passes all of the existing integration tests (plus the
`testApolloServer` suite from
`apollo-server-integration-testsuite/src/ApolloServer.ts` which wasn't being run
previously!) essentially out of the box. (I fleshed out the "mock server"
implementation which converts from Node http requests to Lambda events a bit
more, and changed, but no "core" code or test changes were needed other than
fixing the health check `content-type`.)

Fixes #5078. Fixes #4951 (because that API is just "Express").
glasser added a commit that referenced this issue May 21, 2021
Before this change, apollo-server-lambda contained a lot of code that tried to
understand the formats of Lambda event (request) and result (response)
formats. It tried to work with APIGatewayProxy messages of payloadFormatVersion
1.0 and 2.0 as well as ALB.

But understanding the intricacies of the various Lambda message formats isn't
really what the Apollo Server project is about. A pretty large fraction of all
maintenance on Apollo Server in 2021 has gone into tweaking details of the
Lambda event parsing and making the Lambda handler support all AS features
without the help of a library supporting composable middleware.

This PR (targeted for AS3) throws away the laboriously constructed bespoke
Lambda parsing and faux-middleware implementation and replaces it with two
packages that solve these problems for us: `@vendia/serverless-express` which
understands a variety of Lambda input and output formats and converts them to
Express format, and `express`, the most popular Node library for defining HTTP
server behavior. (Note that `@vendia/serverless-express` is not related to the
`serverless` CLI/framework.)

Now `apollo-server-lambda` is just a convenience wrapper around
`apollo-server-express`. It has to deal with the difference in startup logic
between serverless and non-serverless environments, but it doesn't have to
reimplement all of the Lambda and HTTP logic from scratch.

As an added advantage, you can now optionally provide your own express app to
`apollo-server-lambda`. Previously, `apollo-server-lambda` gave no real way to
customize any of its behavior past the particular options we define, because
Lambda doesn't have a built-in middleware framework. Since we are removing some
built-in features like `graphql-upload` integration in AS3, it's important that
we continue a way to add custom behavior to your Lambda server. Letting you
define that custom behavior with a standard Express app seems reasonable.

We recognize that Lambda users generally care strongly about bundle size, so
adding two new dependencies may seem problematic. That said, we don't currently
have a principled way of evaluating Lambda bundle sizes when we make choices in
this project, and compared to other dependencies of Apollo Server, these new
dependencies are not very large. For now, the improvement in maintainability and
flexibility seems worth the bundle size increase. If `apollo-server-lambda`
users want to help out with a new project of focusing on Lambda bundle size
optimization, we can work together to define benchmarks based on realistic
build/bundler conditions, and find other ways to reduce bundle size (eg, there
is a fair amount of low hanging fruit inside `apollo-reporting-protobuf`).

Fix inconsistency in the content-type returned from health checks (the old
Lambda used `application/json` instead of `application/health+json` like most
other integrations).

This new version passes all of the existing integration tests (plus the
`testApolloServer` suite from
`apollo-server-integration-testsuite/src/ApolloServer.ts` which wasn't being run
previously!) essentially out of the box. (I fleshed out the "mock server"
implementation which converts from Node http requests to Lambda events a bit
more, and changed, but no "core" code or test changes were needed other than
fixing the health check `content-type`.)

Fixes #5078. Fixes #4951 (because that API is just "Express").
@glasser glasser closed this as completed May 25, 2021
@glasser glasser removed this from the Release 3.x milestone Jun 10, 2021
@coyoteecd
Copy link

Note this reverses the changes made in #5018, because createHandler now returns a Handler type exported by aws-lambda (whose author intentionally doesn't want to remove the callback from the handler signature).

@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
None yet
Projects
None yet
Development

No branches or pull requests

2 participants