-
Notifications
You must be signed in to change notification settings - Fork 2k
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
Proposal: Replace integrations with an HTTP transport. #3184
Comments
* Re-land "Expose composed middleware via getMiddleware()" This reverts commit d05aceb which was originally #2435, but was reverted in 52ab22e (#3046) as a precaution, prior to releasing 2.7.0. This attempts to reland it. * Use an `express.Router` rather than `compose-middleware`. Aside from avoiding unnecessary dependencies, by using this Router, we avoid the `apollo-server` base package from behaving different than it has in the past. Specifically, `apollo-server` uses `/` as the default path, but it does so in a wild-card way which serves GraphQL requests on _any_ path. That means that `/graphqlllll` and `/graphql` both also served the GraphQL Playground interface, and also responded to GraphQL execution requests. Since some may be leveraging that behavior, we should preserve it, if we can. * Add CHANGELOG.md link to #3184, for awareness.
I know you've mentioned subscriptions over HTTP without web sockets, but is there any other document about this? This is super important in a serverless architecture like AWS APIGateway web sockets. We did our own implementation hacking quite a lot of Apollo to have the desire results and while it works quite well, I'm really sad and not proud of how it looks like. It really is difficult to work around Apollo opinionated transport like expecting a long running server to always be available. ): |
It was going to come to this sooner or later, since Node.js v6 is no longer supported by the Node.js Foundation. In this case, I'm adding this exception because after bringing #3229 (2dd0592), the `koa-bodyparser` package was updated to a new major version which, itself, dropped Node.js 6 support. That update to `koa-bodyparser`, which fixes an incorrect/malformed `Content-length` header calculation is — I think — important enough that we should make sure it's included in Apollo Server, which currently drives the underlying version of Koa for all users because of its close coupling with Koa itself (via the `apollo-server-koa` package). This micro-framework-management will no longer be a concern with Apollo Server, particularly because of the introduction of a transport abstraction, which I've proposed in #3184. Ref: #3184
It was going to come to this sooner or later, since Node.js v6 is no longer supported by the Node.js Foundation. In this case, I'm adding this exception because after bringing #3229 (2dd0592), the `koa-bodyparser` package was updated to a new major version which, itself, dropped Node.js 6 support. That update to `koa-bodyparser`, which fixes an incorrect/malformed `Content-length` header calculation is — I think — important enough that we should make sure it's included in Apollo Server, which currently drives the underlying version of Koa for all users because of its close coupling with Koa itself (via the `apollo-server-koa` package). This micro-framework-management will no longer be a concern with Apollo Server, particularly because of the introduction of a transport abstraction, which I've proposed in #3184. Ref: #3184
It was going to come to this sooner or later, since Node.js v6 is no longer supported by the Node.js Foundation. In this case, I'm adding this exception because after bringing #3229 (2dd0592), the `koa-bodyparser` package was updated to a new major version which, itself, dropped Node.js 6 support. That update to `koa-bodyparser`, which fixes an incorrect/malformed `Content-length` header calculation is — I think — important enough that we should make sure it's included in Apollo Server, which currently drives the underlying version of Koa for all users because of its close coupling with Koa itself (via the `apollo-server-koa` package). This micro-framework-management will no longer be a concern with Apollo Server, particularly because of the introduction of a transport abstraction, which I've proposed in #3184. Ref: #3184
It was going to come to this sooner or later, since Node.js v6 is no longer supported by the Node.js Foundation. In this case, I'm adding this exception because after bringing #3229 (2dd0592), the `koa-bodyparser` package was updated to a new major version which, itself, dropped Node.js 6 support. That update to `koa-bodyparser`, which fixes an incorrect/malformed `Content-length` header calculation is — I think — important enough that we should make sure it's included in Apollo Server, which currently drives the underlying version of Koa for all users because of its close coupling with Koa itself (via the `apollo-server-koa` package). This micro-framework-management will no longer be a concern with Apollo Server, particularly because of the introduction of a transport abstraction, which I've proposed in #3184. Ref: #3184
Since Node.js v6 is no longer supported by the Node.js Foundation, it was going to come to this sooner or later since transitive packages are inching their ECMAScript compilation targets to more and more recent versions of the language. While Apollo Server itself will drop support for Node.js v6 in 3.x, the current Koa integration necessitates a more immediate exception since, after bringing #3229 (2dd0592), the `koa-bodyparser` package was updated to a new major version which, itself, dropped Node.js 6 support. That update to `koa-bodyparser`, which fixes an incorrect/malformed `Content-length` header calculation is important enough on its own, but there's also a [CVE][1] for the [`qs`][2] dependency, which makes it even more pressing. We should make sure both of those are included in Apollo Server, which currently drives the underlying version of Koa for all users because of its close coupling with Koa itself (via the `apollo-server-koa` package). This doesn't necessarily mean that those who are still on Node.js v6 are completely out of luck, since they could probably modify their `package-lock.json` files to use an older copy of `koa-bodyparser`, but anyone still using Node.js v6 should certainly make considerations - sooner rather than later — about upgrading to more recent and more supported versions of Node.js! Luckily, this micro-framework-management will soon no longer be a concern with Apollo Server, particularly because of the introduction of a transport abstraction, which I've proposed in #3184. Ref: #3184 [1]: https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2017-1000048 [2]: https://npm.im/qs Fixes: #3050
Since Node.js v6 is no longer supported by the Node.js Foundation, it was going to come to this sooner or later since transitive packages are inching their ECMAScript compilation targets to more and more recent versions of the language. While Apollo Server itself will drop support for Node.js v6 in 3.x, the current Koa integration necessitates a more immediate exception since, after bringing #3229 (2dd0592), the `koa-bodyparser` package was updated to a new major version which, itself, dropped Node.js 6 support. That update to `koa-bodyparser`, which fixes an incorrect/malformed `Content-length` header calculation is important enough on its own, but there's also a [CVE][1] for the [`qs`][2] dependency, which makes it even more pressing. We should make sure both of those are included in Apollo Server, which currently drives the underlying version of Koa for all users because of its close coupling with Koa itself (via the `apollo-server-koa` package). This doesn't necessarily mean that those who are still on Node.js v6 are completely out of luck, since they could probably modify their `package-lock.json` files to use an older copy of `koa-bodyparser`, but anyone still using Node.js v6 should certainly make considerations - sooner rather than later — about upgrading to more recent and more supported versions of Node.js! Luckily, this micro-framework-management will soon no longer be a concern with Apollo Server, particularly because of the introduction of a transport abstraction, which I've proposed in #3184. Ref: #3184 [1]: https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2017-1000048 [2]: https://npm.im/qs Fixes: #3050
☝️ Same here. Great work putting that together @abernix 👏. Current API design feels unnecessary restrictive and robs users from the freedom of applying their own custom solutions and patterns.
@harlandduman My two cents here? This needn't be so. There could always be an extra export/package that may serve as a canned/just-add-water solution - similarly to what's being exported now, without restricting everyone's experience. |
@alfaproject here is the IncrementalDelivery proposal, which is still a working draft in the transport spec. @abernix this initiative is really exciting! |
We work 100% with lambda even for websockets using a Serverless approach and while I'm happy to see progress with traditional long-lived HTTP servers, usually the implementations don't help us that much in that regard. I believe the solution for @defer and @stream with an AWS serverless architecture will have to serve the partial responses via websockets. Happy to see progress and thank you for the heads up but I guess we'll have to keep waiting for the Apollo server protocol abstraction to start working on something useful for us. |
@alfaproject here is a demo of the @robrichard and I took a look, and either netlify's proxy or the aws api gateway just waits til the last chunk to send all the payloads. hoping removing streams are meant to be short lived and finite, so IMHO websockets don't make sense for this case, especially on lambda. however, there is talk of another, non-competing websockets transport spec being submitted for note: if you have comments on the spec please open them as issues or PRs for the transport spec body, so we can iterate and submit an RFC to the IETF this year (hopefully) |
We are still excited about the idea of replacing integrations with an HTTP transport in a future major version bump of Apollo Server. Our current plan for Apollo Server 3 is mostly focused on the story of "fewer hard-coded dependencies on specific versions of third-party software", and some of the things in this issue like decoupling from |
This is very much resolved in Apollo Server 4, currently a Release Candidate. Thanks @abernix for setting out an excellent proposal, which served as a great design for Apollo Server 4. |
Introduction
This document outlines the intent of some pattern changes to the way that Apollo Server is initialized which aims to:
applyMiddleware
] (e.g.apollo-server-express
,apollo-server-koa
).IncomingMessage
) into the shape that the HTTP transport expects (SeeHTTPGraphQLRequest
, below). In the outbound direction, these adapters convert the body, status code and headers from the object that the HTTP transport receives (seeHTTPGraphQLResponse
, below) into that which the framework expects (frequently instances of Node.js'ServerResponse
).(req, res, next)
request handler signature and hides away the details of the aforementioned HTTP adapters and HTTP transport.http
.At a high-level, this suggests implementors to:
Move away from using
applyMiddleware
, opting instead for patterns appropriate for with their HTTP framework (e.g. Express, Koa, Hapi).These HTTP frameworks should be able to evolve at their own pace without the close coupling of their major versions with Apollo Server's own. Currently, because of our close coupling, Apollo Server is responsible for (too) many updates to those frameworks and their typings because they're baked into our integrations.
Provide file-upload middleware, if necessary.
This has the advantage of allowing different GraphQL upload integrations and, more importantly, the versions of those integrations (e.g.
graphql-upload
) which has been previously baked into Apollo Server and out of the user's control.Provide their own "health check" implementation, if necessary.
Health checks are more complicated and application-specific than the limited implementation which we've previously provided (which was near nothing).
Provide/choose GraphQL user interface (i.e. GraphQL Playground, GraphiQL, etc.), if necessary.
It should be easier to remove the interface entirely, change it to a custom version, or replace it with one which supports offline support. Many organizations have decided that a particular interface is better for them, and it should be easy to switch these out, or leave them out entirely (if using a third-party tool, like Postman or Insomnia).
This work speaks directly to items that we've noted in the Apollo Server 3.0 Roadmap. Specifically, it helps with two of the roadmap's bullet-points:
Please read on for additional details!
Background
GraphQL requests and responses are most often exchanged over HTTP or WebSockets. As part of Apollo Server's processing of a GraphQL request and generating a GraphQL response, it must transform an incoming request — be it presented as HTTP, a WebSocket stream, or otherwise — into a shape that it can understand. For example, extracting the
query
andvariables
from a WebSocket stream or parsing them out of the HTTP request body. Additionally, it is in this phase that it has the opportunity to extract protocol-specific properties, like HTTP headers.After processing the request and executing the operation, Apollo Server must turn the resulting GraphQL response into a format that confines with the desires of the transport the request came in on. For example, when transporting over HTTP, adding the correct
Content-type
headers, generating an appropriate status code (e.g. 200, 401, 500), etc.Currently, Apollo Server has a number of so-called integrations that allow it to do these various data-structure mappings to corresponding expectations of HTTP server frameworks (e.g. Express, Koa, etc.) and FaaS implementations (e.g. AWS Lambda, Google Cloud Functions, Azure Functions, etc.) .
Notably, the following eight integrations within the Apollo Server repository alone:
apollo-server-azure-functions
apollo-server-cloud-functions
apollo-server-express
apollo-server-fastify
apollo-server-hapi
apollo-server-koa
apollo-server-lambda
apollo-server-micro
Each one of these packages exists to define the aforementioned protocol mapping in a similar, though slightly different way.
Consider this incoming request scenario which leaves out some steps while still, I think, highlighting unnecessary complexity in our current approach:
apollo-server-express
package uses thecors
package to prepare the response with the appropriate CORSAccess-Control-*
headers based on the settings provided on thecors
option of the theapplyMiddleware
method. Theapollo-server-lambda
package uses a similar, albeit different configuration interface, but sets the same headers directly onto the response object.apollo-server-express
package checks to see ifreq.method
is equal toGET
, whereas theapollo-server-lambda
checks ifevent.httpMethod
isGET
.apollo-server-express
package then checks to see if it's a user making the request via a browser by checking if theAccepts
header containstext/html
by using theaccepts
package to checkreq
(IncomingMessage
). Theapollo-server-lamdba
package makes a similar check, but merely usesString.prototype.includes
on theevent.headers.Accept
property to check for the presence oftext/html
.apollo-server-express
andapollo-server-lambda
package both return the result ofrenderPlaygroundPage
, which returns the HTML for rendering GraphQL Playground. Prior to returning the (HTML) content though, the Express integration first sets theContent-type
response header totext/html
usingres.setHeader
, while the Lambda integration sets thestatusCode
property on the result context directly.If you see where this is going, you'll note that with a more abstracted HTTP implementation, much of this duplication could be consolidated. While the above simply contrasts the behavior in
apollo-server-lambda
with that ofapollo-server-express
, further examples of this duplication are prevalent in the integrations for other "integrations" — for example, our Micro integration, Koa integration, and Fastify integration all do mostly identical, though (frustratingly/) subtly different steps.HTTP Transport (and Interfaces)
The Apollo Server repository would be the home of a new HTTP transport, and potentially others — to be determined as we build it out and demonstrate the need. WebSockets is certainly a strong contender.
HTTP framework-specific handlers (when not naturally compatible with these interfaces) would use the result of the transport to map to their own data structures An HTTP transport would be responsible for translating an HTTP request's data structures into the data structures necessary for GraphQL execution within Apollo Server. Most notably, that means feeding the request pipeline with the
query
, theoperationName
, thevariables
and theextensions
. Additionally, the GraphQL request context should have access to strongly-typed transport-specific properties (e.g.method
,url
), which would be definitively undefined on non-HTTP requests.To demonstrate this data structure mapping, some interfaces serve as good examples:
Sending the right messages
While the above should handle the core bits of mapping to HTTP, we will certainly need to expand
GraphQLResponse
so it can provide the correct hints to the transport when it needs to make more complicated decisions.For example, rather than directly setting a
cacheControl
header inside of Apollo Server, we should provide the transport with amaxAge
calculation that it can use as necessary. For the HTTP transport, this still means setting aCache-Control
header, but for other transports (e.g. WebSockets) it might need to react differently or not at all.API pattern changes to
ApolloServer
The following sections demonstrate some bits that we should change in order to facilitate this decoupling.
While these changes might seem to move away from simpler patterns, I feel they enable GraphQL server users to have a more clear understanding of what their server is doing. Armed with that understanding — which we will provide whenever possible in the form of documentation and useful error messages — users will be better equipped to scale their server and the server will be easier to grow with.
"Getting Started" (Previously
apollo-server
)The current getting started experience with Apollo Server runs an
express
server internally. Even though it has implications for the future that server, this hidden usage doesn't really make it clear to the user that we've made that choice for them. For example, it may not align with organizational requirements to run a particular framework flavor (e.g. Koa or Hapi). It's also just a larger dependency than the built-inhttp.createServer
server that comes with Node.js!This pattern should make it much more clear what server you're using and make it easier to scale out of a Getting Started experience as an application grows (e.g. when non-GraphQL middleware needs to come into the picture):
Before
After
With this particularly simple usage, there are some default restrictions which might require adopting a more full-fledged HTTP framework in order to change:
An HTTP framework is still not mandatory though, even with all of these options, since packages like
compose-middleware
work well withhttp.createServer
too, allowing the composition/chaining of various middleware into a single handler. For example, we might consider providing suggestions like this one, passing in various middleware likecors()
,graphqlPlayground
,json
body-parsing, etc.).Express (previously
apollo-server-express
)The widely used
apollo-server-express
integration bakes in a number of other pieces of batteries-included functionality, though Apollo Server doesn't build on top of that functionality and instead merely passes it through to underlying dependencies (which again, fall out of date if we fail to update them regularly — which we have tried not to, but it takes a lot of work and extra versioning noise!):Before
After
In my experience, this after experience ends up being a lot more what you'd find in a typical server which has literally any other purpose other than serving GraphQL. You'll note that we're merely passing options through to the underlying dependencies (i.e.
cors
,body-parser
):Even if there are more imports here, this After scenario isn't actually introducing more dependencies since all of those are used internally already. Furthermore, when someone wants to "eject" from our behavior, they generally need to move to a pattern more similar to the above anyhow.
Batteries previously included?
Apollo Server 2.x adopted several “batteries included”-like functionalities which complicate integrations and cloud up HTTP transports including:
This section intends to shed light on what those are and what they (often, did not) do.
The first two bullet points above (body parsing and CORS) are implemented by utilizing popular third party packages (in Express, at least; and often manually in other frameworks).
The third (health checks) is merely a user-definable function which is just as well served passed directly to the framework itself.
The last is a third-party upload implementation which is well documented on its own and has previously needed to be updated independent of Apollo Server, but has been unfortunately at the mercy of our versioning.
Body Parser
In the
apollo-server-express
integration, Apollo Server automatically implements thebody-parser
package to parse the incoming HTTP request body. In the wild, most any server which does anything besides behave as a GraphQL server will likely have body-parsing functionality in place already. Though this can be implemented as:Other frameworks have their own approach here, like Koa's
koa-bodyparser
and Hapi's built-inroute.options.payload.parse
(default!).CORS
In order to properly restrict requests to specific domains, CORS should be a consideration of any web server. By automatically using the
cors
package in its default configuration, Apollo Server allows the user to completely forget about CORS and just allow their server to participate in cross-domain messaging. That's great for getting started, but the default configuration ofcors
could be less secure than not enabling CORS at all since the defaults (ofcors
) setsorigin: *
and the correspondingAccess-Control-*
headers to permit all common HTTP verbs (e.g.GET
,POST
,DELETE
, and so on.), allowing Apollo Server to serve any GraphQL request from any domain.It’s arguable whether or not we’re doing much in the way of a favor to the user here, particularly since the code necessary for a user to put such permissive CORS behavior in place on their own is as simple as:
Similarly, Koa offers
@koa/cors
and Hapi has itsroute.options.cors
. FaaS implementations often require you simply set the headers directly (which is all these other packages do).While some teams might be tuned into important configuration like CORS, educating users of the importance of this option by making it more explicit and providing them the correct documentation to make the right decisions for their application seems of the utmost importance.
Health check
Right now, Apollo Server supplies a health-check middleware. If
onHealthCheck
is not defined — which it isn’t by default — Apollo Server merely returns a healthy status code response with{status: 'pass'}
body — irregardless of whether Apollo Server is actually configured properly. If a user also defines theonHealthCheck
method on theirApolloServer
constructor, it will utilize that in order to quantify whether the server is healthy or not, though it doesn't receive the actual Apollo Server context, so it's abilities are limitedThis same health-check middleware could be implemented in user-code with:
There are other ways to do health checks, and even just providing a resolver which returns a similar value would be more accurate.
File uploads
Currently, Apollo Server automatically applies a third-party upload middleware called
graphql-upload
to provide support for file upload support via GraphQL fields that are typed as a specialUpload
scalar.In order to replicate this behavior, the user would need to manually add the
scalar Upload
type (currently done automatically) to their type definitions and also add thegraphqlUploadExpress
middleware before their Apollo Server middleware:As noted above, this implementation is well documented on the
graphql-upload
GitHub repository. Having uploads enabled by default isn't necessary for most users and, in practice, there are often other ways that users choose to handle uploads.Closing
HTTP isn't the only way that GraphQL execution can take place and even looking at just HTTP, the constantly growing number of frameworks has made it increasingly hard for Apollo Server to welcome those into the Apollo Server repository directly. I haven't personally seen it, but Amazon Lambdas, in addition to being triggered by API Gateway, can also be triggered by SQS, SMS, S3 uploads, and more. In practice, whether WebSockets or something more exotic, we've seen users needing to dig too deeply into Apollo Server's core in order to make this work, often sacrificing other Apollo Server features and functionality in the process.
Keeping HTTP-specific properties out of Apollo Server core will help other transports succeed but allow Apollo Server to focus on areas where it can provide greater value, such as caching, performance, observability, and different execution techniques. Additionally, the maintenance overhead and upkeep to inch all the integrations forward has been difficult, at best.
By defining this separation more concretely, I hope to make it easier for us to not need to selectively choose what frameworks belong in the core repository or not, since this data structure mapping should enable Apollo Server to co-exist with whatever framework flavor. In Apollo Server 3.x, we'll do our best to provide interfaces with maximum compatibility and minimal coupling to provide a better one-size-fits-all solution. This might mean providing an
async
handler that accepts a similar(req: IncomingMessage, res: ServerResponse)
that could be used more ubiquitously, but should stop short of overly definitive implementations don't give access to necessary escape hatches.I'm very excited about the opportunities this change will afford us, even if it does change the getting started experience. We're throwing around some other ideas to continue to keep that step easy (maybe easier than before!), but it's of the utmost importance that we provide flexibility to larger organizations as they grow out of that default experience.
I believe this is a step in that direction and feedback is very much appreciated.
The text was updated successfully, but these errors were encountered: