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

The HTTP2 streams sometimes don't emit the end event #32978

Closed
szmarczak opened this issue Apr 21, 2020 · 9 comments
Closed

The HTTP2 streams sometimes don't emit the end event #32978

szmarczak opened this issue Apr 21, 2020 · 9 comments
Labels
http2 Issues or PRs related to the http2 subsystem.

Comments

@szmarczak
Copy link
Member

szmarczak commented Apr 21, 2020

  • Version: v13.13.0
  • Platform: Linux solus 5.5.11-151.current deps: update openssl to 1.0.1j #1 SMP PREEMPT Tue Mar 24 18:06:46 UTC 2020 x86_64 GNU/Linux
  • Subsystem: http2

What steps will reproduce the bug?

const {connect} = require('http2');
const session = connect('https://www.facebook.com');
session.once('remoteSettings', () => {
	console.log('got settings');
	session.request({
		'user-agent': 'Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/78.0.3904.70 Safari/537.36',
		'accept-encoding': 'gzip, deflate, br'
	}).once('response', headers => {
		console.log('got headers', headers);
	}).on('data', chunk => {
		console.log(chunk.toString().length);
	}).on('end', () => {
		console.log('got end');
	}).resume();
});

How often does it reproduce? Is there a required condition?

It reproduces 80% of the time. Sometimes it emits the end event as expected.

What is the expected behavior?

got settings
got headers [Object: null prototype] { ... }
[numbers here]
+got end

What do you see instead?

got settings
got headers [Object: null prototype] { ... }
[numbers here]

Additional information

curl works as expected:

curl --http2 -H 'accept-encoding: gzip, deflate, br' -H 'user-agent: Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/78.0.3904.70 Safari/537.36' https://www.facebook.com>/dev/null

First discovered by @kaatt szmarczak/http2-wrapper#38

@addaleax addaleax added the http2 Issues or PRs related to the http2 subsystem. label Apr 24, 2020
@rexagod
Copy link
Member

rexagod commented May 21, 2020

@szmarczak In events where 'end' is not emitted, is there an unusual delay before it's emitted, or is it not emitted at all (and the session closes abruptly with/without throwing)?

@szmarczak
Copy link
Member Author

Not emitted at all. It doesn't throw, just hangs.

@rexagod
Copy link
Member

rexagod commented May 21, 2020

Given the flaky nature of this issue, I can narrow this down to the fact that either Facebook sent you a "falsely" chunk (improbable) or the chunk size you received was over the highwatermark (which paused the stream and since you're waiting for the 'end' event before resuming, it hung up).

@szmarczak
Copy link
Member Author

But there is a data event handler, so it shouldn't be paused...

@rexagod
Copy link
Member

rexagod commented May 21, 2020

Refer this (L1023). ret can be a falsey value if either the stream has ended or the highwatermark is exceeded.

node/lib/_stream_readable.js

Lines 1009 to 1025 in a4e273b

stream.on('data', (chunk) => {
debug('wrapped data');
if (state.decoder)
chunk = state.decoder.write(chunk);
// Don't skip over falsy values in objectMode.
if (state.objectMode && (chunk === null || chunk === undefined))
return;
else if (!state.objectMode && (!chunk || !chunk.length))
return;
const ret = this.push(chunk);
if (!ret) {
paused = true;
stream.pause();
}
});

@kaatt
Copy link

kaatt commented May 22, 2020

A more consistent repro would be to create a http2 server that does what @rexagod mentioned or inspect the network traffic of facebook.com on wireshark

@duongvanba
Copy link

Hi everyone, any fix or solution for this ?

@duongvanba
Copy link

HI everyone
Any fix on this ?

@szmarczak
Copy link
Member Author

Fixed in #33875

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

5 participants