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

http2: Only first headers cause an event in http2 stream, all following headers are muted. #38258

Closed
elmerbulthuis opened this issue Apr 16, 2021 · 3 comments · Fixed by #41739
Labels
confirmed-bug Issues with confirmed bugs. http2 Issues or PRs related to the http2 subsystem.

Comments

@elmerbulthuis
Copy link

elmerbulthuis commented Apr 16, 2021

What steps will reproduce the bug?

When making a http2 request, it seems that events are only fired for the first headers in the stream and not the following!

I encountered this problem when waiting for a response after a continue response. The first headers are the continue response so I do get an event for them. But the next headers (the actual response) do not fire an event.

The following code exposes the bug

const http = require("http");
const http2 = require("http2");

main();

async function main(){
    await testHttp();
    await testHttp2();
}

async function testHttp () {
    console.log("http test");

    const server = http.createServer((request, response) => response.end());
    await new Promise(resolve => server.listen(9090, resolve));

    const stream = http.request(
        "http://localhost:9090/",
        {
            headers: {
                "expect": "100-continue",
            },
        });

    const errorPromise = new Promise((resolve, reject) => stream.addListener("error", reject));
    const continuePromise = new Promise(resolve => stream.addListener("continue", resolve));
    const responsePromise = new Promise(resolve => stream.addListener("response", resolve));

    const continueValue = await Promise.race([
        errorPromise,
        continuePromise,
    ]);

    console.log("continue");

    const responseValue = await Promise.race([
        errorPromise,
        responsePromise,
    ]);

    console.log("response");

    await new Promise(
        (resolve, reject) => server.close(error => error ?
            reject(error) :
            resolve()),
    );

    console.log("done");
}

async function testHttp2() {
    console.log("http2 test");

    const server = http2.createServer((request, response) => response.end());
    await new Promise(resolve => server.listen(9090, resolve));

    const session = http2.connect("http://localhost:9090");
    const stream = session.request({
        ":path": "/",
        "expect": "100-continue",
    });

    const errorPromise = new Promise((resolve, reject) => stream.addListener("error", reject));
    const continuePromise = new Promise(resolve => stream.addListener("continue", resolve));
    const responsePromise = new Promise(resolve => stream.addListener("response", resolve));

    const continueValue = await Promise.race([
        errorPromise,
        continuePromise,
    ]);

    console.log("continue");

    const responseValue = await Promise.race([
        errorPromise,
        responsePromise,
    ]);

    // unfortunately we never get to here in node v15.4.0
    console.log("response");

    await new Promise(
        resolve => session.close(resolve),
    );

    await new Promise(
        (resolve, reject) => server.close(error => error ?
            reject(error) :
            resolve()),
    );

    console.log("done");
}

Also tried listening to the headers event of the http2 stream, this event fires only the first time (on the continue response), the next time (actuel response) there is no event.

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

reproduces every time!

What is the expected behavior?

When receiving a http continue response i expect the continue event to be fired, when receiveing a http response (other than continue) i expect the response event to be fired!

What do you see instead?

only the first headers frame causes an event to be fired. The event is correct, but the second event never happens!

Additional information

Please help! This bug makes the nodejs http2 client useless for our use case!

@elmerbulthuis elmerbulthuis changed the title Only first headers cause an event in http2 stream, all following headers are muted. http2: Only first headers cause an event in http2 stream, all following headers are muted. Apr 16, 2021
@Ayase-252 Ayase-252 added the http2 Issues or PRs related to the http2 subsystem. label Apr 16, 2021
@Ayase-252
Copy link
Member

Also reproducible in master

version v16.0.0-pre
http test
continue
response
done
http2 test
continue
// hang here

@pd4d10
Copy link
Contributor

pd4d10 commented May 5, 2021

It seems to be a bug caused by calling response.end() with no data. The temporary solution would be pass a string as the first argument, for example, response.end('')

@elmerbulthuis
Copy link
Author

It seems to be a bug caused by calling response.end() with no data. The temporary solution would be pass a string as the first argument, for example, response.end('')

Confirmed that that makes the test work!

@marsonya marsonya added the confirmed-bug Issues with confirmed bugs. label May 10, 2021
ofirbarak pushed a commit to ofirbarak/node that referenced this issue Jan 29, 2022
When sending a continue request, server response with null,
it does not fires the response event type

Fixes: nodejs#38258
ofirbarak pushed a commit to ofirbarak/node that referenced this issue Jan 30, 2022
When sending a continue request, server response with null,
it does not fires the response event type

Fixes: nodejs#38258
nodejs-github-bot pushed a commit that referenced this issue Feb 3, 2022
When sending a continue request, server response with null,
it does not fires the response event type

Fixes: #38258

PR-URL: #41739
Refs: #38561
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: James M Snell <[email protected]>
VoltrexKeyva pushed a commit to VoltrexKeyva/node that referenced this issue Feb 3, 2022
When sending a continue request, server response with null,
it does not fires the response event type

Fixes: nodejs#38258

PR-URL: nodejs#41739
Refs: nodejs#38561
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: James M Snell <[email protected]>
ruyadorno pushed a commit that referenced this issue Feb 8, 2022
When sending a continue request, server response with null,
it does not fires the response event type

Fixes: #38258

PR-URL: #41739
Refs: #38561
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: James M Snell <[email protected]>
danielleadams pushed a commit that referenced this issue Mar 2, 2022
When sending a continue request, server response with null,
it does not fires the response event type

Fixes: #38258

PR-URL: #41739
Refs: #38561
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: James M Snell <[email protected]>
danielleadams pushed a commit that referenced this issue Mar 3, 2022
When sending a continue request, server response with null,
it does not fires the response event type

Fixes: #38258

PR-URL: #41739
Refs: #38561
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: James M Snell <[email protected]>
danielleadams pushed a commit to danielleadams/node that referenced this issue Mar 4, 2022
When sending a continue request, server response with null,
it does not fires the response event type

Fixes: nodejs#38258

PR-URL: nodejs#41739
Refs: nodejs#38561
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: James M Snell <[email protected]>
danielleadams pushed a commit to danielleadams/node that referenced this issue Mar 4, 2022
When sending a continue request, server response with null,
it does not fires the response event type

Fixes: nodejs#38258

PR-URL: nodejs#41739
Refs: nodejs#38561
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: James M Snell <[email protected]>
danielleadams pushed a commit that referenced this issue Mar 8, 2022
When sending a continue request, server response with null,
it does not fires the response event type

Fixes: #38258

PR-URL: #41739
Refs: #38561
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: James M Snell <[email protected]>
danielleadams pushed a commit that referenced this issue Mar 14, 2022
When sending a continue request, server response with null,
it does not fires the response event type

Fixes: #38258

PR-URL: #41739
Refs: #38561
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
confirmed-bug Issues with confirmed bugs. http2 Issues or PRs related to the http2 subsystem.
Projects
None yet
4 participants