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

Using express with the new http2 module #14672

Closed
PiniH opened this issue Aug 7, 2017 · 18 comments
Closed

Using express with the new http2 module #14672

PiniH opened this issue Aug 7, 2017 · 18 comments
Labels
http2 Issues or PRs related to the http2 subsystem.

Comments

@PiniH
Copy link

PiniH commented Aug 7, 2017

  • Version: v9.0.0-pre
  • Platform: MacOS
  • Subsystem: http2

Testing node's new http2 native module I couldn't get express to serve requests over http2,
Using node master build with --expose-http2 (v9.0.0-pre macOS) flag:

const express = require('express');

const app = express();

app.get('/express', (req, res) => {
  res.send('Hello from express');
});

const server = http2.createSecureServer({
  key,
  cert
}, app);

When requesting /express the server crashed with the following error:

(node:80731) ExperimentalWarning: The http2 module is an experimental API.
_http_incoming.js:104
  if (this.socket.readable)
                 ^

TypeError: Cannot read property 'readable' of undefined
    at IncomingMessage._read (_http_incoming.js:104:18)
    at IncomingMessage.Readable.read (_stream_readable.js:431:10)
    at IncomingMessage.read (_http_incoming.js:96:15)
    at resume_ (_stream_readable.js:811:12)
    at _combinedTickCallback (internal/process/next_tick.js:138:11)
    at process._tickCallback (internal/process/next_tick.js:180:9)

Opened an issue for express as well: expressjs/express#3388
@benjamingr

@benjamingr
Copy link
Member

Ping @jasnell - this is a first time contributor trying http2.

@benjamingr benjamingr added the http2 Issues or PRs related to the http2 subsystem. label Aug 7, 2017
@PiniH
Copy link
Author

PiniH commented Aug 8, 2017

Playing with it a little more - I got it to work, but I had to expose Http2ServerRequest/Response, I think it might be a good idea to expose it in general in the new http2 module.

phouri added a commit to phouri/node that referenced this issue Aug 10, 2017
In order for express (and possibly other libraries) to get and use
the Http2ServerRequest/Response - expose them in the http2 exports.
Same as is done in http module.

This is only a suggestion - not sure if this is the way to go, but
with this it allows for express to run with http2 server immediately.

ref: expressjs/express#3390
fixes: nodejs#14672

Fix Lint
MSLaguana pushed a commit to nodejs/node-chakracore that referenced this issue Aug 21, 2017
In order for express (and possibly other libraries) to get and use
the Http2ServerRequest/Response - expose them in the http2 exports.
Same as is done in http module.

PR-URL: nodejs/node#14690
Ref: expressjs/express#3390
Fixes: nodejs/node#14672
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Matteo Collina <[email protected]>
@PiniH
Copy link
Author

PiniH commented Aug 31, 2017 via email

@benjamingr
Copy link
Member

benjamingr commented Aug 31, 2017

@PiniH any ETA on when we can expect Express to work with http2?

@PiniH
Copy link
Author

PiniH commented Aug 31, 2017 via email

@benjamingr
Copy link
Member

benjamingr commented Aug 31, 2017

@dougwilson

(unrelated: I have removed excess spacing created by replying via email without altering the content in PiniH's post)

@MongoExpUser
Copy link

@PiniH,
Thanks for the updates.

@dougwilson
Copy link
Member

Has the change to expose the prototype been released yet?

@jasnell
Copy link
Member

jasnell commented Aug 31, 2017

Not yet. It has landed tho so it should be soon

@dougwilson
Copy link
Member

Nice :) also did anything ever happen with the two items at #14690 (comment) ?

@jasnell
Copy link
Member

jasnell commented Aug 31, 2017

I'll look at the statusMessage setter. The path one is likely better for express to handle but if you have an idea on how we can handle it on our side then ++

@dougwilson
Copy link
Member

Thanks, James :) I believe @PiniH is going to look into how / if Express.js can handle the req.path on it's side, so it may be possible.

@jasnell
Copy link
Member

jasnell commented Aug 31, 2017

Awesome. I'm excited. Let me know if there's anything else needed for this.

MylesBorins pushed a commit that referenced this issue Sep 10, 2017
In order for express (and possibly other libraries) to get and use
the Http2ServerRequest/Response - expose them in the http2 exports.
Same as is done in http module.

PR-URL: #14690
Ref: expressjs/express#3390
Fixes: #14672
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Matteo Collina <[email protected]>
MylesBorins pushed a commit that referenced this issue Sep 12, 2017
In order for express (and possibly other libraries) to get and use
the Http2ServerRequest/Response - expose them in the http2 exports.
Same as is done in http module.

PR-URL: #14690
Ref: expressjs/express#3390
Fixes: #14672
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Matteo Collina <[email protected]>
@jrgleason
Copy link

jrgleason commented Jun 4, 2019

Any chance of getting an example of how you got it working?

I tired...

const {
           Http2ServerRequest,
           Http2ServerResponse,
} = http2;
this.express.use((req, res, next)=>{
            Object.setPrototypeOf(req, Http2ServerRequest.prototype);
            Object.setPrototypeOf(res, Http2ServerResponse.prototype);
            next();
});
this.server = http2.createSecureServer({
            key: fs.readFileSync(process.env.KEY_FILE),
            cert: fs.readFileSync(process.env.CERT_FILE),
            allowHTTP1: true
}, this.express);

@calaway
Copy link

calaway commented Oct 30, 2019

Playing with it a little more - I got it to work, but I had to expose Http2ServerRequest/Response, I think it might be a good idea to expose it in general in the new http2 module.

@PiniH (or anyone else), can you please explain your solution? I'm getting the same error now and I don't see how you were able to get it working.

  • Node 10.16
  • Express ~4.16.1.

My simple server:

// index.js

const port = 3000
const express = require('express')
const http2 = require('http2')

const app = express();

app.get('*', (req, res) => {
  res.status(200).json({message: 'ok'})
})

const server = http2.createServer(app)
server.listen(port)
server.on('listening', () => console.log(`Listening on port: ${port}.`))

I'm using http2.createServer instead of http2.createSecureServer because I'm using a reverse proxy and the TLS encryption happens there, so it must not be encrypted here.

Here's what I get when I run the server locally and hit it with cURL:

$ curl --http2-prior-knowledge http://localhost:3000
curl: (56) Recv failure: Connection reset by peer

and on the server I see the error reported at the top of this issue:

_http_incoming.js:95
  if (this.socket.readable)
                  ^

TypeError: Cannot read property 'readable' of undefined
    at IncomingMessage._read (_http_incoming.js:95:19)
    at IncomingMessage.Readable.read (_stream_readable.js:453:10)
    at resume_ (_stream_readable.js:929:12)
    at process._tickCallback (internal/process/next_tick.js:63:19)

I get the same error when I deploy it. It successfully navigates the TLS handshake on the reverse proxy, then it hits my server and crashes with the same error.

If anyone can help me get unstuck here I would be very grateful. Thanks for all you do.

@trollkarlen
Copy link

trollkarlen commented Dec 6, 2019

Whats up with this, pushed to express then whats the corresponding issue there ?
Its still a issue.

/T

@gaxolio
Copy link

gaxolio commented Dec 10, 2020

Hi @jrgleason , any news about this problem?
Is your example working?
Please, let me know.
Many thanks

Any chance of getting an example of how you got it working?

@crystalfp
Copy link

Crossing fingers, with node 14.15.1 and koa 2.13.1 my application seems to work with native http2 without problems.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
http2 Issues or PRs related to the http2 subsystem.
Projects
None yet
Development

No branches or pull requests

10 participants