improve error reporting and use rolling timeout

[alanning]

Changes

1. Uses new rolling_timeout_exec npm package which provides rollingTimeout options field and will kill child process if no activity is observed on stdout or stderr for rollingTimeout milliseconds.
2. Improves error reporting in case of child process error:

   Old:

$ mrt update
✓ bad-repo
    branch: https://github.com/alanning/xyz.git#master

/Users/alanning/src/js/meteorite/lib/sources/git.js:114
        throw "There was a problem cloning repo: " + self.url +
                                                              ^
There was a problem cloning repo: https://github.com/alanning/xyz.git
Please check https://github.com/oortcloud/meteorite/blob/master/CONTRIBUTING.md#troubleshooting for potential explanations.

New:

$ mrt update
✓ bad-repo
    branch: https://github.com/alanning/xyz.git#master
ERROR: 128 Command failed: Cloning into '/Users/alanning/.meteorite/source/alanning/xyz'...
remote: Repository not found.
fatal: repository 'https://github.com/alanning/xyz.git/' not found

STDOUT:

STDERR: Cloning into '/Users/alanning/.meteorite/source/alanning/xyz'...
remote: Repository not found.
fatal: repository 'https://github.com/alanning/xyz.git/' not found

/Users/alanning/src/js/meteorite/lib/sources/git.js:124
        throw "There was a problem cloning repo: " + self.url +
                                                              ^
There was a problem cloning repo: https://github.com/alanning/xyz.git
Please check https://github.com/oortcloud/meteorite/blob/master/CONTRIBUTING.md#troubleshooting for potential explanations.

[alanning]

I'm not quite sure what tests are needed for the rolling-timeout part. The rolling_timeout_exec package has tests so maybe that's enough?

The most formal way would be to have a git server running that accepts connections but never completes requests. Could fork this but that would be a lot of work.

I also wasn't sure if anything needed to be changed in the install script to accommodate the new package dependency.

NOTE: For testing locally, you'll need to run npm install in meteorite root to pull in the required npm package.

[alanning]

Related to Issue #204

[tmeasday]

Hey @alanning -- I tried running the tests against this and they failed. Happy to not introduce more tests but I think the existing ones should work.

It could be something to do with the way we use a cached version of git, but can you try to get the test running off master and then see if you can identify the problem? https://github.com/oortcloud/meteorite/blob/master/CONTRIBUTING.md#testing

[alanning]

Hi Tom,

Will do. Unfortunately the tests are failing for me on a clean pull of master but I'll see what I can work out.

Which test was failing for you?

On Jan 16, 2014, at 2:18 AM, Tom Coleman notifications@github.com wrote:

Hey @alanning -- I tried running the tests against this and they failed. Happy to not introduce more tests but I think the existing ones should work.

It could be something to do with the way we use a cached version of git, but can you try to get the test running off master and then see if you can identify the problem? https://github.com/oortcloud/meteorite/blob/master/CONTRIBUTING.md#testing

— Reply to this email directly or view it on GitHub.

[tmeasday]

I think it was basically all of the tests that involve git.

Let me know how the tests are failing, it'd be nice to get them working properly...

[alanning]

This is how tests are failing on my machine:

https://github.com/oortcloud/meteorite/issues/230#issuecomment-32172845

I'll see if I can figure out why but for this PR I'll spin up a new ec2 instance and see if I can get the tests working there.

On Thu, Jan 16, 2014 at 6:32 PM, Tom Coleman notifications@github.comwrote:

I think it was basically all of the tests that involve git.

Let me know how the tests are failing, it'd be nice to get them working properly...

— Reply to this email directly or view it on GitHubhttps://github.com/oortcloud/meteorite/pull/233#issuecomment-32564726 .

[alanning]

PR 235 got npm test working for me with a clean git clone [meteorite] so now I can see the test failures you mentioned.

On the rolling-timeout branch, it looks like the tests are failing because git calls are being intercepted and the spec/support/bin/git script is hardcoded to expect no options.

Rolling timeout needs git clone --progress to be effective (so stdout is updated as progress is made and the timeout resets).

If you by-pass the helper git script, the tests pass:

$ mv spec/support/bin/git spec/support/bin/git_orig
$ ln -s /usr/local/bin/git spec/support/bin/git

I'll see what I can do to get the helper script working.

[tmeasday]

Oh, good stuff. Thanks a lot for going the extra mile @alanning

[alanning]

Passes all tests for me now on a clean git clone git@github.com:oortcloud/meteorite.git

[tmeasday]

Works here too. Great stuff!

@seanmwalker, @yeputons - FYI.

[choccy]

I'm finding that 15 seconds is not enough, any chance for a higher ROLLING_TIMEOUT?

[alanning]

Fine with me. The goal is really to just have a sane circuit breaker value so longer wait also works. What's your use case (dial-up?) and how long were you thinking?

[choccy]

Mostly weird latency issues with Github I assume, I doubled it to 30 seconds to be safe.