search

zkat / make-fetch-happen

Get in loser, we're making requests!
Other
384 stars 27 forks source link

Handle highWaterMark potentially false #72

Open adiktofsugar opened 5 years ago

adiktofsugar commented 5 years ago

Fixes: #70 (I think)

If the content-length header is empty, size gets set to false, which is sent in as the highWaterMark option to through2, which is passed to readable-stream, which eventually raises an error saying "false is not a valid value" or something similar, but only on version 3.x, which is what npm uses (even though this project doesn't).

I did write a test for this, but could never get it to trigger because it's caused by some sort of dependency mismatch that happens in npm but not in this repo, so I removed the test. Even when I installed readable-stream@3.x, it didn't trigger, because through2 is the only dependency that requires readable-stream, which is not the case in npm. When I updated just through2's readable-stream version, I got a different error entirely, so I'm not sure if this weird dependency thing is the only issue, but I do know we're not supposed to pass highWaterMark: false, so this should still be a valid fix.

adiktofsugar commented 3 years ago

appveyor failing because npm@latest doesn't support node 9, apparently travis is failing because it couldn't find a .staging folder

Definitely seems like issues with not maintaining CI, but I'm guessing nothing else is ever getting in since no one uses this module now. Oh well.