Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

http2 - http/1 compat errors #15292

Closed
ronag opened this issue Sep 9, 2017 · 3 comments
Closed

http2 - http/1 compat errors #15292

ronag opened this issue Sep 9, 2017 · 3 comments
Labels
http2 Issues or PRs related to the http2 subsystem.

Comments

@ronag
Copy link
Member

ronag commented Sep 9, 2017

Consider the following:

const server = http2.createServer({
  allowHTTP1: true
})

server.on('stream', stream => {
  stream.on('error', err => console.error(err))
})

server.on('request', req => {
  if (req.httpVersionMajor >= 2) {
    // XXX workaround
    // req.on('error', () => { /* Do nothing ... */ })
    // res.on('error', () => { /* Do nothing ... */ })
    // NOTE http2 request are handled through HTTP2Server's 'stream' handler.
    return
  }
})

If stream now emit an error we will get an unhandled exception even though we have stream.on('error', ...) because we didn't add an error listener to the compat request which will throw like this:

function onStreamError(error) {
  const request = this[kRequest];
  request.emit('error', error);
}

Causing a crash like this:

"type": "NodeError",
      "message": "Stream closed with error code 8",
      "stack": "Error [ERR_HTTP2_STREAM_ERROR]: Stream closed with error code 8\n    at process.nextTick (internal/http2/core.js:1530:21)\n    at _combinedTickCallback (internal/process/next_tick.js:131:7)\n    at process._tickCallback (internal/process/next_tick.js:180:9)"
@ronag ronag changed the title http1 compat errors http2 - http1 compat errors Sep 9, 2017
@ronag ronag changed the title http2 - http1 compat errors http2 - http/1 compat errors Sep 9, 2017
@mscdex mscdex added the http2 Issues or PRs related to the http2 subsystem. label Sep 9, 2017
@apapirovski
Copy link
Contributor

apapirovski commented Sep 9, 2017

Thanks for the report @ronag. Are you using 8.4 or a build from master? This was patched recently to no longer forward the errors to request and response.

In general, you might find it a more pleasant experience working with a build from master which has a lot of bug fixes for http2. Let me know if you need any help or have any questions for that.

@ronag
Copy link
Member Author

ronag commented Sep 9, 2017

I'm running 8.4.

Yea, I'm using nvm for handling my mode versions. I wish they had a build from master option. I'll see if I can go over to just building it manually.

@dougwilson
Copy link
Member

If nvm had just a nightly option, that would be prefect. Here is the issue for nvm to add nightly support: nvm-sh/nvm#1053

@ronag ronag closed this as completed Sep 11, 2017
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
http2 Issues or PRs related to the http2 subsystem.
Projects
None yet
Development

No branches or pull requests

4 participants