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

Error when using HTTP2 #1533

Closed
abrenneke opened this issue Aug 15, 2018 · 11 comments
Closed

Error when using HTTP2 #1533

abrenneke opened this issue Aug 15, 2018 · 11 comments
Labels
⛲️ feature New addition or enhancement to existing solutions

Comments

@abrenneke
Copy link
Contributor

abrenneke commented Aug 15, 2018

apollo-server doesn't seem to like when Node's http2 module is used. When an HTTP2 client (i.e. the browser) makes a request, this error happens:

TypeError: :method is not a legal HTTP header name

  - index.js:670 validateName
    [venom-api]/[apollo-server-env]/[node-fetch]/lib/index.js:670:9

  - index.js:830 Headers.append
    [venom-api]/[apollo-server-env]/[node-fetch]/lib/index.js:830:3

  - nodeHttpToRequest.ts:11 Object.keys.forEach.key
    [venom-api]/[apollo-server-core]/src/nodeHttpToRequest.ts:11:15

  - Array.forEach

  - nodeHttpToRequest.ts:6 Object.convertNodeHttpToRequest
    [venom-api]/[apollo-server-core]/src/nodeHttpToRequest.ts:6:28

...more stuff

Since apollo-server-core uses Request and Headers objects from node-fetch, I'm not sure whether this is an upstream problem or not. The problem is that node-fetch doesn't seem to support the "special" header names documented here.

In HTTP/2, the request path, hostname, protocol, and method are represented as special headers prefixed with the : character (e.g. ':path'). These special headers will be included in the request.headers object.

Since nodeHttpToRequest isn't actually using node-fetch to make an HTTP request, this seems to me like a problem internal to Apollo Server. node-fetch has their own issue here for HTTP2, but that seems to be concerned with making HTTP2 requests.

Reproduction:

(I use Koa)

const Koa = require('koa');
const { ApolloServer } = require('apollo-server-koa');
const http2 = require('http2');

const app = new Koa();
const apollo = new ApolloServer({ etc });
apollo.applyMiddleware(app);
const server = http2.createSecureServer({ cert: 'cert here', key: 'key here' }, app.callback());
server.listen(8000);

Then make a GraphQL request, with your browser, to https://localhost:8000/ (browsers do not allow insecure HTTP2, so you'll have to ignore the security warning)

Potential Fix

In nodeHttpToRequest, ignore headers starting with :

@selipso
Copy link

selipso commented Oct 17, 2018

Also interested in having HTTP2 work with Apollo. Since Express doesn't support HTTP2 natively yet, it may be worth creating a wrapper for spdy with an HTTP2 boolean flag that can be passed in when calling / creating a new ApolloServer

@mzygmunt
Copy link

mzygmunt commented May 9, 2019

I'm interested in using HTTP2 in fastify.

@jsegaran jsegaran added 🚧👷‍♀️👷‍♂️🚧 in triage Issue currently being triaged ⛲️ feature New addition or enhancement to existing solutions labels Jul 8, 2019
@abernix abernix removed the 🚧👷‍♀️👷‍♂️🚧 in triage Issue currently being triaged label Jul 16, 2019
@Dbuggerx
Copy link

I'm interested in using it in koa

@sensha4u
Copy link

sensha4u commented Apr 4, 2020

Please extend the HTTP 2 support for Restdatasource also

@devdudeio
Copy link

any updates on this?

@theJC
Copy link

theJC commented Jan 28, 2021

Has any progress been made on this front, or any additional plans formed?

@andreas4all
Copy link

Problem is in node-fetch library, where is test for header token name.

original `node_modules/apollo-server-env/node_modules/node-fetch/lib/index.js

...
const invalidTokenRegex = /[^\^_`a-zA-Z\-0-9!#$%&'*+.|~]/;
...

when change to

...
const invalidTokenRegex = /[^\^_`:a-zA-Z\-0-9!#$%&'*+.|~]/;
...

It start working.

@glasser
Copy link
Member

glasser commented Jan 25, 2022

While I know this isn't the most exciting thing to hear, I think this will get naturally fixed as part of Apollo Server 4 which we expect to ship in the first half of this year. It looks like the problem is that we normalize incoming requests to the Request type from node-fetch, which doesn't allow for the special HTTP/2 headers. We'll be defining our own request type in AS4 (see eg the HTTPGraphQLRequest proposal in #3184) and we'll avoid using node-fetch's Request for that, which should solve the issue.

@annitya
Copy link

annitya commented Sep 27, 2022

Apollo 4 is still in Alpha. Also it's been four years. The only thing missing is literally a colon. Will you accept a merge-request where I replace node-fetch with a fixed version?

@glasser
Copy link
Member

glasser commented Sep 27, 2022

Apollo Server 4 is going into release candidate today. As mentioned above, this changes the type used to represent requests from node-fetch's (which doesn't allow for special HTTP/2 headers) to our own simpler type.

I'm going to close this issue as fixed on the version-4 branch; it would be great if HTTP/2 users could verify this.

@kiran052
Copy link

kiran052 commented Jan 3, 2023

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

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Apr 19, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
⛲️ feature New addition or enhancement to existing solutions
Projects
None yet
Development

No branches or pull requests