Close GH-794: Fallback to standard git clone if download/untar fails .

lib/core/resolvers/GitHubResolver.js

@@ -49,6 +49,7 @@ GitHubResolver.prototype._checkout = function () {
         return GitRemoteResolver.prototype._checkout.call(this);
     }
 
+    var msg;
     var tarballUrl = 'https://github.com/' + this._org + '/' + this._repo + '/archive/' + this._resolution.tag + '.tar.gz';
     var file = path.join(this._tempDir, 'archive.tar.gz');
     var reqHeaders = {};
@@ -71,12 +72,11 @@ GitHubResolver.prototype._checkout = function () {
         headers: reqHeaders
     })
     .progress(function (state) {
-        var msg;
-
         // Retry?
         if (state.retry) {
             msg = 'Download of ' + tarballUrl + ' failed with ' + state.error.code + ', ';
             msg += 'retrying in ' + (state.delay / 1000).toFixed(1) + 's';
+            that._logger.debug('error', state.error.message, { error: state.error });
             return that._logger.warn('retry', msg);
         }
 
@@ -93,7 +93,27 @@ GitHubResolver.prototype._checkout = function () {
             to: that._tempDir
         });
 
-        return extract(file, that._tempDir);
+        return extract(file, that._tempDir)
+        // Fallback to standard git clone if extraction failed
+        .fail(function (err) {
+            msg =  'Decompression of ' + path.basename(file) + ' failed' + (err.code ? ' with ' + err.code : '') + ', ';
+            msg += 'trying with git..';
+            that._logger.debug('error', err.message, { error: err });
+            that._logger.warn('retry', msg);
+
+            return that._cleanTempDir()
+            .then(GitRemoteResolver.prototype._checkout.bind(that));
+        });
+    // Fallback to standard git clone if download failed
+    }, function (err) {
+        msg = 'Download of ' + tarballUrl + ' failed' + (err.code ? ' with ' + err.code : '') + ', ';
+        msg += 'trying with git..';
+        that._logger.debug('error', err.message, { error: err });
+        that._logger.warn('retry', msg);
+
+        return that._cleanTempDir()
+        .then(GitRemoteResolver.prototype._checkout.bind(that));
+
     });
 };
 

lib/core/resolvers/Resolver.js

@@ -3,6 +3,7 @@ var path = require('path');
 var Q = require('q');
 var tmp = require('tmp');
 var mkdirp = require('mkdirp');
+var rimraf = require('rimraf');
 var readJson = require('../../util/readJson');
 var createError = require('../../util/createError');
 var removeIgnores = require('../../util/removeIgnores');
@@ -153,6 +154,23 @@ Resolver.prototype._createTempDir = function () {
     }.bind(this));
 };
 
+Resolver.prototype._cleanTempDir = function () {
+    var tempDir = this._tempDir;
+
+    if (!tempDir) {
+        return Q.resolve();
+    }
+
+    // Delete and create folder
+    return Q.nfcall(rimraf, tempDir)
+    .then(function () {
+        return Q.nfcall(mkdirp, tempDir, 0777 & ~process.umask());
+    })
+    .then(function () {
+        return tempDir;
+    });
+};
+
 Resolver.prototype._readJson = function (dir) {
     var that = this;
 

lib/core/resolvers/UrlResolver.js

@@ -132,8 +132,9 @@ UrlResolver.prototype._download = function () {
 
         // Retry?
         if (state.retry) {
-            msg = 'Download of ' + that._source + ' failed with ' + state.error.code + ', ';
+            msg = 'Download of ' + that._source + ' failed' + (state.error.code ? ' with ' + state.error.code : '') + ', ';
             msg += 'retrying in ' + (state.delay / 1000).toFixed(1) + 's';
+            that._logger.debug('error', state.error.message, { error: state.error });
             return that._logger.warn('retry', msg);
         }
 

test/core/resolvers/gitHubResolver.js

@@ -77,6 +77,66 @@ describe('GitHub', function () {
             .done();
         });
 
+        it('should retry using the GitRemoteResolver mechanism if download failed', function (next) {
+            var resolver;
+            var retried;
+
+            nock('https://github.com')
+            .get('/IndigoUnited/events-emitter/archive/0.1.0.tar.gz')
+            .reply(200, 'this is not a valid tar');
+
+            logger.on('log', function (entry) {
+                if (entry.level === 'warn' && entry.id === 'retry') {
+                    retried = true;
+                }
+            });
+
+            resolver = create({ source: 'git://github.com/IndigoUnited/events-emitter.git', target: '0.1.0' });
+
+            // Monkey patch source to file://
+            resolver._source = 'file://' + testPackage;
+
+            resolver.resolve()
+            .then(function (dir) {
+                expect(retried).to.be(true);
+                expect(fs.existsSync(path.join(dir, 'foo'))).to.be(true);
+                expect(fs.existsSync(path.join(dir, 'bar'))).to.be(true);
+                expect(fs.existsSync(path.join(dir, 'baz'))).to.be(true);
+                next();
+            })
+            .done();
+        });
+
+        it('should retry using the GitRemoteResolver mechanism if extraction failed', function (next) {
+            var resolver;
+            var retried;
+
+            nock('https://github.com')
+            .get('/IndigoUnited/events-emitter/archive/0.1.0.tar.gz')
+            .reply(500);
+
+            logger.on('log', function (entry) {
+                if (entry.level === 'warn' && entry.id === 'retry') {
+                    retried = true;
+                }
+            });
+
+            resolver = create({ source: 'git://github.com/IndigoUnited/events-emitter.git', target: '0.1.0' });
+
+            // Monkey patch source to file://
+            resolver._source = 'file://' + testPackage;
+
+            resolver.resolve()
+            .then(function (dir) {
+                expect(retried).to.be(true);
+                expect(fs.existsSync(path.join(dir, 'foo'))).to.be(true);
+                expect(fs.existsSync(path.join(dir, 'bar'))).to.be(true);
+                expect(fs.existsSync(path.join(dir, 'baz'))).to.be(true);
+                next();
+            })
+            .done();
+        });
+
         it('should fallback to the GitRemoteResolver mechanism if resolution is not a tag', function (next) {
             var resolver = create({ source: 'git://github.com/foo/bar.git', target: '2af02ac6ddeaac1c2f4bead8d6287ce54269c039' });
             var originalCheckout = GitRemoteResolver.prototype._checkout;

test/core/resolvers/resolver.js

@@ -16,8 +16,16 @@ describe('Resolver', function () {
     var tempDir = path.resolve(__dirname, '../../assets/tmp');
     var testPackage = path.resolve(__dirname, '../../assets/package-a');
     var logger;
+    var dirMode0777;
 
     before(function () {
+        var stat;
+
+        mkdirp.sync(tempDir);
+        stat = fs.statSync(tempDir);
+        dirMode0777 = stat.mode;
+        rimraf.sync(tempDir);
+
         logger = new Logger();
     });
 
@@ -438,20 +446,6 @@ describe('Resolver', function () {
     });
 
     describe('._createTempDir', function () {
-        var dirMode0777;
-
-        before(function () {
-            var stat;
-
-            mkdirp.sync(tempDir);
-            stat = fs.statSync(tempDir);
-            dirMode0777 = stat.mode;
-        });
-
-        after(function (next) {
-            rimraf(tempDir, next);
-        });
-
         it('should create a directory inside a "username/bower" folder, located within the OS temp folder', function (next) {
             var resolver = create('foo');
 
@@ -537,6 +531,60 @@ describe('Resolver', function () {
         });
     });
 
+    describe('._cleanTempDir', function () {
+        it('should not error out if temporary dir is not yet created', function (next) {
+            var resolver = create('foo');
+
+            resolver._cleanTempDir()
+            .then(next.bind(null))
+            .done();
+        });
+
+        it('should delete the temporary folder contents', function (next) {
+            var resolver = create('foo');
+
+            resolver._createTempDir()
+            .then(resolver._cleanTempDir.bind(resolver))
+            .then(function (dir) {
+                expect(dir).to.equal(resolver.getTempDir());
+                expect(fs.readdirSync(dir).length).to.be(0);
+                next();
+            })
+            .done();
+        });
+
+        it('should keep the mode', function (next) {
+            var resolver = create('foo');
+
+            resolver._createTempDir()
+            .then(resolver._cleanTempDir.bind(resolver))
+            .then(function (dir) {
+                var stat = fs.statSync(dir);
+                var expectedMode = dirMode0777 & ~process.umask();
+
+                expect(stat.mode).to.equal(expectedMode);
+                next();
+            })
+            .done();
+        });
+
+        it('should keep the dir path', function (next) {
+            var resolver = create('foo');
+            var tempDir;
+
+            resolver._createTempDir()
+            .then(function (dir) {
+                tempDir = dir;
+                return resolver._cleanTempDir();
+            })
+            .then(function (dir) {
+                expect(dir).to.equal(tempDir);
+                next();
+            })
+            .done();
+        });
+    });
+
     describe('._readJson', function () {
         afterEach(function (next) {
             rimraf(tempDir, next);