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

ERR_HTTP2_INVALID_STREAM #24470

Closed
StephenLynx opened this issue Nov 19, 2018 · 23 comments
Closed

ERR_HTTP2_INVALID_STREAM #24470

StephenLynx opened this issue Nov 19, 2018 · 23 comments

Comments

@StephenLynx
Copy link

Node 10.13
CentOS 7
HTTP2

When trying to use writeHead on the compatibility layer, I get a ERR_HTTP2_INVALID_STREAM sometimes that crashes the worker process.

Error [ERR_HTTP2_INVALID_STREAM]: The stream has been destroyed
at Http2ServerResponse.writeHead (internal/http2/compat.js:569:13)

The header is in {key:[value,value],anotherkey:[value]} format.

@Trott
Copy link
Member

Trott commented Nov 19, 2018

@nodejs/http2 (Should this be transferred to the help repo?)

@StephenLynx
Copy link
Author

StephenLynx commented Nov 19, 2018

Trott, keep in mind that this isn't the first time someone has issues with this.
#22135
Now, it should have been fixed on 10.12, so it was either a regression or I am doing something very sketchy with the request object that is causing it to happen.

Edit: the issue happened on ubuntu 16, actually. I am trying to reproduce on CentOS 7 now.
Edit2: it just happened again on CentOS 7 too.

@StephenLynx
Copy link
Author

I would also like to point out that I am using the cluster module to spawn one worker process for each CPU core.

@StephenLynx
Copy link
Author

StephenLynx commented Nov 19, 2018

And after installing 10.12 and 10.13 again I can't seem to reproduce it anymore.
Edit: nevermind, just happened again.

@antsmartian
Copy link
Contributor

@StephenLynx Could you please share a reproducible code?

@StephenLynx
Copy link
Author

I can not, aside from my software gitgud.io/LynxChan/LynxChan

@Trott
Copy link
Member

Trott commented Nov 25, 2018

@nodejs/http2 Is it likely that the source of the problem can be determined with the information given?

@apapirovski
Copy link
Member

If writeHead is crashing the process then in all likelihood your stream is getting closed in advance of you calling the function. Given that you're using compatibility mode, it's very much possible that some other code is causing an issue that leads to the stream prematurely closing. Note that this was also the case with the issue you linked above.

I'm going to close this out as there's no easily usable reproduction and there's not much likelihood this is a Node core issue.

@StephenLynx
Copy link
Author

How could one close it? I can look around my code, I use the request and response exclusively as I have used them for http1 for years.

@StephenLynx
Copy link
Author

StephenLynx commented Nov 29, 2018

So while I am not discarding the possibility to be with my code, I am not so sure it isn't with node. I remember finding some similar intermittent crash around version 5 that I also couldn't consistently reproduce but ended up being a DoS on node itself.

@mcollina
Copy link
Member

I think this is a bug in our compat API.

As you can see in

node/lib/_http_outgoing.js

Lines 240 to 247 in 9920dbc

OutgoingMessage.prototype._writeRaw = _writeRaw;
function _writeRaw(data, encoding, callback) {
const conn = this.connection;
if (conn && conn.destroyed) {
// The socket was destroyed. If we're still trying to write to it,
// then we haven't gotten the 'close' event yet.
return false;
}
, http1 writeHead does not throw if the stream is destroyed, it just returns false.

I think this is a proper bug.

@mcollina mcollina reopened this Nov 29, 2018
@mcollina
Copy link
Member

@StephenLynx can you please upload a script to reproduce the crash?

@StephenLynx
Copy link
Author

I tried, but it wouldn't crash with a simple wait to hold concurrent requests and another script hammering it.
The crashes seem to happen a bit at random with my software running. It works, then suddenly crash. It also seemed to be related with time. It would hardly crash right away, but once a worker crashed, it seems all the others became much more prone to crashing too.

@mcollina
Copy link
Member

Can you please include a full stacktrace?

@StephenLynx
Copy link
Author

I'll try, but I remember it being most of the time here https://gitgud.io/LynxChan/LynxChan/blob/master/src/be/form/captcha.js#L11

@StephenLynx
Copy link
Author

StephenLynx commented Nov 29, 2018

GOT IT
@mcollina
https://pastebin.com/vCbVS1LT
It spent 30 minutes working fine, then crashed when outputting from gridfs, then crashed again 14 seconds later when outputting some error 500 page.

@StephenLynx
Copy link
Author

StephenLynx commented Nov 29, 2018

Also, notice how the error it threw originated from trying to output a response on the cache handler. But that one is inside a try catch, so it didn't crash there.
I'd also like to point that I had about 3 tabs open, no sure if that influenced anything.

@StephenLynx
Copy link
Author

So, the issue here is that is expected for the browser to sometimes interrupt the connection before the response and is up to what handles the connection to deal with it?

@mcollina
Copy link
Member

The issue is just a mismatch between writeHead in http1 and http2. See #24723

@StephenLynx
Copy link
Author

Btw, did you check if the write and end also don't crash?

@mcollina
Copy link
Member

@StephenLynx please check the test in my PR.

@StephenLynx
Copy link
Author

Ah yeah, you do a write head, write and end on the test.
The name of the pr made me think if you weren't looking just at writeHead.

Trott pushed a commit to Trott/io.js that referenced this issue Dec 3, 2018
Fixes: nodejs#24470

PR-URL: nodejs#24723
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
@Trott
Copy link
Member

Trott commented Dec 3, 2018

Fixed in 32fed93

@Trott Trott closed this as completed Dec 3, 2018
BridgeAR pushed a commit that referenced this issue Dec 5, 2018
Fixes: #24470

PR-URL: #24723
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
refack pushed a commit to refack/node that referenced this issue Jan 14, 2019
Fixes: nodejs#24470

PR-URL: nodejs#24723
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
BethGriggs pushed a commit that referenced this issue Feb 12, 2019
Fixes: #24470

PR-URL: #24723
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
rvagg pushed a commit that referenced this issue Feb 28, 2019
Fixes: #24470

PR-URL: #24723
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants