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

bug: fetch causes http2 server to hang #2311

Closed
vegerot opened this issue Oct 5, 2023 · 7 comments · Fixed by #2314
Closed

bug: fetch causes http2 server to hang #2311

vegerot opened this issue Oct 5, 2023 · 7 comments · Fixed by #2314
Labels
bug Something isn't working

Comments

@vegerot
Copy link

vegerot commented Oct 5, 2023

Bug Description

Reading the body of an http2 POST call causes node:http2 to hang when called from this fetch, but not from Deno's fetch or curl.

Reproducible By

import http2 from "node:http2";
import assert from "node:assert";
import { exec } from "node:child_process";

import { Client, fetch } from "undici";
import devcert from "devcert";

const ssl = await devcert.certificateFor("localhost");
const server = http2.createSecureServer(ssl, async (req, res) => {
  assert.equal(req.method, "POST");
  console.log("beginning to read body...");
  let body = [];
  for await (const chunk of req) {
    console.log({ chunk });
    body.push(chunk);
  }
  let buffer = Buffer.concat(body);
  console.log("end of reading body");
  res.end(buffer);
}).listen(3000);

const METHOD = "fetch";
const h2Client = new Client("https://localhost:3000", {
  connect: {
    rejectUnauthorized: false,
  },
  allowH2: true,
});

server.on("listening", async () => {
  console.log("listening on 3000");
  if (METHOD === "fetch") {
    const response = await fetch("https://localhost:3000", {
      dispatcher: h2Client,
      method: "POST",
      body: "12",
      headers: {
        "content-type": "text/plain",
        "content-length": "2",
      },
    });
    console.log("DONE", { response });
  } else if (METHOD === "curl") {
    const curl =
      `curl --no-progress-meter -k -X POST -H "content-type: text/plain" -H "content-length: 2" -d "12" https://localhost:3000`;
    exec(curl, (err, stdout, stderr) => {
      if (err) {
        console.error("DONE", err);
        return;
      }
      console.log("DONE", stdout);
    });
  } else {
    throw new Error();
  }
});

Play around with the value of METHOD to see what I'm talking about

Expected Behavior

both curl and fetch should receive a response quickly.

Instead, curl gets a response immediately but fetch never returns.

Logs & Screenshots

// with curl
beginning to read body...
{ chunk: <Buffer 31 32> }
end of reading body
DONE 12
// with fetch
beginning to read body...
{ chunk: <Buffer 31 32> }
// never gets passed here

Environment

node v20.3.1
Linux maxs-pc 5.19.0-43-generic #44~22.04.1-Ubuntu SMP PREEMPT_DYNAMIC Mon May 22 13:39:36 UTC 2 x86_64 x86_64 x86_64 GNU/Linux

Additional context

I have tested this code with

  • node's builtin fetch ❌
  • undici fetch ❌
  • undici Client#request
  • curl ✅
  • Deno fetch ✅
@vegerot vegerot added the bug Something isn't working label Oct 5, 2023
@vegerot
Copy link
Author

vegerot commented Oct 5, 2023

fetch will work if you read the body like

const server = http2.createSecureServer(ssl, async (req, res) => {
  assert.equal(req.method, "POST");
  console.log("beginning to read body...");
  let body = [];
  req.on("data", (chunk) => {
    body.push(chunk);
  });
  let buffer = Buffer.concat(body);
  console.log("end of reading body");
  res.end(buffer);
}).listen(3000);

Why is for await...of different?

@vegerot
Copy link
Author

vegerot commented Oct 5, 2023

Of course, that's just because I'm only responding with the first chunk. The bug is back if I do

const server = http2.createSecureServer(ssl, async (req, res) => {
  assert.equal(req.method, "POST");
  console.log("beginning to read body...");
  let body = [];
  req.on("data", (chunk) => {
    body.push(chunk);
  });
  req.on("end", () => {
    let buffer = Buffer.concat(body);
    console.log("end of reading body");
    res.end(buffer);
  })
}).listen(3000);

@vegerot
Copy link
Author

vegerot commented Oct 5, 2023

Failing test vegerot@f2519dc

@vegerot
Copy link
Author

vegerot commented Oct 5, 2023

@strager found that Node fetch is sending a "partial entity body" while curl and Deno's fetch do not
image

image

I don't know what "partial entity body" means, but our hypothesis is that Node fetch isn't sending the "whole body", so the server waits forever to get the rest of the body.

@metcoder95
Copy link
Member

metcoder95 commented Oct 5, 2023

Does it fail if you only use Client#request instead?
Can you also share what version of undici are you using?

@vegerot
Copy link
Author

vegerot commented Oct 5, 2023

thanks @metcoder95

client.request also hangs.

I'm using latest, aka 5.24.4.

I added a test to undici to reproduce the bug (I think)

@metcoder95
Copy link
Member

Found the issue, basically fetch returns an iterable, and we didn't handle its closeup after the full body was written

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants