-
Notifications
You must be signed in to change notification settings - Fork 35
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
replace minipass pipeline with simple pipe #63
Conversation
I can reliably cause tar integrity errors while downloading packages from npm thru an http proxy that uses chunked encoding. The error seems to be in make-fetch-happen, minipass-fetch or one of the minipass libraries. I don't understand the root cause but I'm unable to reproduce the issue after applying this patch. My guess is that something in the stack expects res.body to be a Minipass instance but not MinipassPipeline, or creating the pipeline starts body flowing before all listeners are connected, missing some chunks of data. fixes npm/cli#3884 To reproduce 1. Start the [chunked-encoding proxy](https://github.com/everett1992/make-fetch-happen/blob/chunked-encoding/proxy.js) 2. In a new terminal create a npm project `mkdir chunked-encoding-error && cd chunked-encoding-error && echo '{}' > package.json` 3. configure npm to use the registry `npm config --location project set registry http://localhost:4000 4. remove node_modules, and npm cache 5. install any package from the registry (this should fail). - I recommend a package without dependencies, like lodash or typescript, debugging is easier. - The error isn't 100% consistent, you may want to run this step in a loop ``` while rm ~/.npm/_cacache node_modules package-lock.json -rf && npm install --no-save lodash; do; done ``` [chunked-encoding proxy]: https://github.com/everett1992/make-fetch-happen/blob/chunked-encoding/proxy.js
Related to npm/cli#3884, npm/make-fetch-happen#63 npm v7+ throws integrity and zlib corrupted errors when installing from a registry that uses chunked encoding. This package reproduces that error using make-fetch-happen and tar. The error only occurs when fetch is passed integrity and the server uses chunked encoding. Only the `extract with integirty` test fails.
I started carving chunks out of npm to create a smaller reproduction and I've reached make-fetch-happen and tar. Tomorrow I'll keep carving to try to eliminate tar. https://github.com/everett1992/make-fetch-happen-tar-extract-error/blob/main/error.test.js This test shows a lot of weird behavior.
Switching to my branch of make-fetch-happens fixes the bug and all tests pass.
|
if i'm understanding correctly, your pull request to |
Yes, isaacs/minipass#28 closes this and npm/cli#3884 |
you're awesome, thanks for taking the time to chase this one down all the way to the end. i know what these bug hunts are like so i really appreciate it! |
Thanks, I'm glad I followed it to the root cause instead of trying to merge this hot fix. |
I can reliably cause tar integrity errors while downloading packages
from npm thru an http proxy that uses chunked encoding. The error seems
to be in make-fetch-happen, minipass-fetch or one of the minipass
libraries. I don't understand the root cause but I'm unable to reproduce
the issue after applying this patch.
My guess is that something in the stack expects res.body to be a
Minipass instance but not MinipassPipeline, or creating the pipeline
starts body flowing before all listeners are connected, missing some
chunks of data.
fixes npm/cli#3884
To reproduce
mkdir chunked-encoding-error && cd chunked-encoding-error && echo '{}' > package.json
npm config --location project set registry http://localhost:4000
a loop