Specify TCP timeout when downloading packages

[kirs]

It's been an old and annoying bug: yarn doesn't specify fetch timeout when downloading packages, causing yarn install to hang forever on network issues.

Steps to reproduce

1. Install toxiproxy (proxy to simulate network latency [1], [2])

2. Configure toxiproxy to simulate the latency:

~/.bin/toxiproxy-cli create yarnpkg --listen localhost:8888 -u registry.yarnpkg.com:443
~/.bin/toxiproxy-cli toxic add yarnpkg -t latency -a latency=9000 -a jitter=50

3. Point yarn.lock to use localhost:8888 as a host for package registry

4. Run yarn install and see the process hanging

The issue has been reported multiple times and it's a very easy fix; I'm surprised that I'm the first one to look into it. As mentioned in the fetch documentation, "requests to external servers should have a timeout attached".

Specifying timeout is a critical thing for distributed software and build tools. Earlier this week we found our build servers hanging on yarn install step for 40 minutes until it was hard killed. It took us a while to find that it was a network problem with connectivity between EC2 and registry.yarnpkg.com. Having the fetch timeout would help us to discover the issue and see the timeout error, as well as not causing build servers to hang until a hard kill.

fixes #764, #1289, #2351

@bestander

[bestander]

Nice.
Could you fix lint issues?
Also how about making it a configuration parameter?
I wonder if 10 seconds may be too little for some cases.

I think there is huge need to write a test here but what would be your Test Plan for the change?

[bestander]

lint + test + config

[kirs]

Thanks for feedback! I'll work on it.

I wish RequestManager had better test coverage to have more examples how I can test it...

[bestander]

Start with a manual Test Plan and think if this could be automated in some way

[bestander]

It could be an e2e test with http requests mocked

[kirs]

@bestander fixed lint, added the config option and the unit test 🎉

Random tests are failing on the CI but I can't reproduce them locally with the same version of node. Taking the fact that CI for master is also red, I assume the test suite is not 100% stable.

[kirs]

@bestander please let me know if there's something else I can improve.

[kirs]

CircleCI is now green 🎉

[aliyazdani]

@kirs - thanks for this. Would love to see this merged in soon.

[bestander]

Great work, @kirs.
What do you think of implementing retries?
Would it be hard to have this feature in?

[kirs]

Happy to see it merged!

@bestander in my implementation timeout errors are considered the same as other networking errors and they will be retried:

yarn/src/util/request-manager.js

Line 348 in b2c2c3b

if (attempts < 5 && this.isPossibleOfflineError(err)) {

yarn/src/util/request-manager.js

Line 281 in b2c2c3b

// TCP timeout

[kirs]

@casperisfine @stefanmb in case you've been wondering about Locutus builds stuck on yarn install... This fixes the issue.