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

Cannot reliably use stream's pipeline function with an undici response's body in node v18.16.0 #2173

Closed
fvictorio opened this issue Jun 27, 2023 · 5 comments
Labels
bug Something isn't working

Comments

@fvictorio
Copy link

fvictorio commented Jun 27, 2023

Bug Description

The pipeline function of stream/promises doesn't work well with a response.body in node v18.16.0 and v18.16.1. To be honest, I don't know if this is a problem in undici or in node.

Reproducible By

This snippet works, but if you uncomment the line that awaits 1s, then the process finishes in the middle of the piping:

const path = require("path")
const fs = require("fs")

async function main() {
  const { pipeline } = require("stream/promises");
  const { request } = require("undici");

  const response = await request("https://binaries.soliditylang.org/linux-amd64/list.json");

  //await new Promise(resolve => setTimeout(resolve, 1000))

  const filePath = path.resolve(__dirname, "list.json")
  await pipeline(response.body, fs.createWriteStream(filePath))

  console.log("success")
}

main()
 .catch(console.error)

The resulting list.json is corrupted (doesn't have the full JSON).

The 1s sleep is not realistic, it's just the best way I found to consistently trigger this issue on my machine. In the actual scenario, these are just a few fs calls.

Expected Behavior

The list.json file should be correctly created.

Environment

This happens in node v18.16.0 and v18.16.1, but it doesn't in v18.15.0.

Additional context

I tried uploading the same file to a GitHub gist and the same thing happened, so I don't think it's something related to that particular server.

@fvictorio fvictorio added the bug Something isn't working label Jun 27, 2023
@fvictorio
Copy link
Author

In case someone lands here: one workaround is to immediately consume the response into a buffer and then write that:

const path = require("path")
const fsPromises = require("fs/promises")

async function main() {
  const { request } = require("undici");

  const response = await request("https://binaries.soliditylang.org/linux-amd64/list.json");

  const responseBuffer = await response.body.arrayBuffer();

  await new Promise(resolve => setTimeout(resolve, 1000))

  const filePath = path.resolve(__dirname, "list.json")
  await fsPromises.writeFile(filePath, responseBuffer)

  console.log("success")
}

main()
 .catch(console.error)

Of course, this is less performant.

fvictorio added a commit to NomicFoundation/hardhat that referenced this issue Jun 27, 2023
This is a workaround for an issue related to node v18.16.0, the stream module and undici. See nodejs/undici#2173 for the details.
@KhafraDev
Copy link
Member

cc @ronag

@marcbachmann
Copy link

marcbachmann commented Jun 28, 2023

I can confirm that there's an issue with the piping. In my case the first file download never resolved with await pipeline(body, filewriteStream) (also with node 20.3.1), while subsequent requests succeeded. reading using an async iterator into memory worked properly.

const {body} = await request(...)
const chunks = []
for await (const chunk of body) chunks.push(chunk)
const buffer = Buffer.concat(chunks)

@mcollina
Copy link
Member

I think this is fixed by nodejs/node#48435.

@marcbachmann
Copy link

marcbachmann commented Jun 29, 2023

The patch solves the issue with the reproduction in the issue description. I'll report back after we've tested our non-resolved promise case.

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

No branches or pull requests

4 participants