fixes crashes due to lost of connection

[DrorT]

Fix for issue #104 that I opened a few days ago

[nolanlawson]

Thanks for the PR! Can you provide a little more detail on how exactly this fixes the issue? Looking at the code, it seems that you convert an error from request into a 500 error sent to the user. Seems this is exactly the same behavior that occurs when you pipe the response directly, so I don't understand how it would fix any issues with going offline.

Also another thing about going offline is that this module is designed to fail if it hasn't cached a tarball and you try to request it while offline. The npm CLI should not blithely continue if the network request fails.

[nolanlawson]

Oh sorry, didn't read #104. Based on that issue, it seems like the real issue is that you want to increase the timeout. Is that correct?

[DrorT]

Hey Nolan.

I tried increasing the timeout, but it seems that on bad connection there might be situations where the reply can come back after up to 5min (I know my connection at times is terrible) and it seems unreasonable to increase the timeout to that level. Also part of the problem is that when local-npm fails the npm install process keeps running unaware that there is nothing on the other end.

So my solution was to pass the failure to the npm install process, this way npm can request the package again and at the end fail showing the user that a specific package could not be retrieved.

Dror

[nolanlawson]

@DrorT OK, so I'm still not quite understanding how this PR changes the existing behavior? Aren't timeouts still passed as errors to the npm CLI?

[DrorT]

Hey @nolanlawson,
Before this change local-npm use to crash on timeouts, but the nom cli will keep on running trying to connect and will never stop.
with this change npm cli is notified the file cannot be retrieved and will retry or fail the package.
Since the npm cli is a "monitored" process by the developer but local-npm is less likely to be monitored, with this fix the problem is shown to the developer who can then rerun npm install and/or check his connection.

[nolanlawson]

I just double-checked the request docs and realized you're right. I thought pipe() would propagate errors to the caller, but apparently you have to do .on('error') as well. Shows what I know about streams. 😛

Anyway thanks for the PR, will merge and publish a fix shortly.

[nolanlawson]

0506206

[DrorT]

Thanks :)
This packages is awesome and saves me time daily because of the bad connection we have, while your tests shows small improvement, on a big package I go from over an hour to a minute or two.

[nolanlawson]

nice! glad to hear it. :)

[gr2m]

I can second that. local-npm is such a no-brainer, I use it every day! Thanks for the amazing work 👏