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

Add a test showing http/2 works #6992

Open
glasser opened this issue Oct 4, 2022 · 5 comments
Open

Add a test showing http/2 works #6992

glasser opened this issue Oct 4, 2022 · 5 comments
Labels
📚 good-first-issue Issues that are more approachable for first-time contributors. 🙏 help-wanted 🧪 testing

Comments

@glasser
Copy link
Member

glasser commented Oct 4, 2022

Apollo Server v3 used node-fetch Requests to represent incoming requests, and those objects aren't compatible with http/2 requests which have colons in header names. See #2333 and #1533.

We think that AS4 (which no longer uses that representation for requests) should support http/2. It would be great to get a PR adding a test!

@glasser glasser added 🙏 help-wanted 📚 good-first-issue Issues that are more approachable for first-time contributors. 🧪 testing labels Oct 4, 2022
@nedjulius
Copy link

hi @glasser, I am a new contributor and I would be happy to work on this issue.

would I add this test to the general apolloServerTests.ts suite?

@glasser
Copy link
Member Author

glasser commented Oct 13, 2022

Sure! Alternatively, add it to packages/server/src/__tests__/express4/expressSpecific.test.ts and do it just for express middleware, since not all integrations might support HTTP/2.

@kiran052
Copy link

kiran052 commented Jan 3, 2023

Provide an example to how to create apollo server with http2 and express.

@glasser
Copy link
Member Author

glasser commented Feb 13, 2023

AS4 ships only with a simple standalone server and an Express middleware, and Express doesn't support http2 out of the box without using one of various "bridge" packages. So putting tests for this in the AS core repo might not make much sense. Perhaps we should find an integration for a framework that does support http2 (like Koa) and put a test like this into its integration repository. Once we have evidence that it's possible to do that for some framework, perhaps we can close this issue.

@theJC
Copy link

theJC commented Mar 2, 2023

apollo-server-integrations/apollo-server-integration-koa#87 should suffice to close this out.

We just migrated to AS4 at my company in PROD today (using koa integration). I've also manually tested and have proven sending GraphQL requests via http2 cleartext works just fine, but we haven't transitioned to using it yet, need to get changes made to our service mesh and kubernetes to support h2c health checks/readiness checks/etc before we can do that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
📚 good-first-issue Issues that are more approachable for first-time contributors. 🙏 help-wanted 🧪 testing
Projects
None yet
Development

No branches or pull requests

4 participants