fix: robust git-tarball fallback (statusCode + TAR errors)

lib/git.js

@@ -254,8 +254,11 @@ class GitFetcher extends Fetcher {
           resolved: this.resolved,
           integrity: null, // it'll always be different, if we have one
         }).extract(tmp).then(() => handler(`${tmp}${this.spec.gitSubdir || ''}`), er => {
-          // fall back to ssh download if tarball fails
-          if (er.constructor.name.match(/^Http/)) {
+          // fall back to clone if the tarball download fails due to an
+          // HTTP error or if the response is not a valid tarball (e.g.
+          // a hosted provider returning an HTML sign-in page with 200)
+          if ((typeof er.statusCode === 'number' && er.statusCode >= 400) ||
+              /^TAR_/.test(er.code)) {
             return this.#clone(handler, false)
           } else {
             throw er

test/git.js

@@ -15,6 +15,7 @@ const rimraf = require('rimraf')
 const tar = require('tar')
 const spawnNpm = require('../lib/util/npm.js')
 const GitFetcher = require('../lib/git.js')
+const RemoteFetcher = require('../lib/remote.js')
 
 // set this up first, so we can use 127.0.0.1 as our "hosted git" service
 const httpPort = 18000 + (+process.env.TAP_CHILD_ID || 0)
@@ -631,14 +632,47 @@ t.test('fetch a private repo where the tgz is a 404', { skip: isWindows && 'posi
   return gf.extract(me + '/no-tgz')
 })
 
+t.test('fetch a private repo where the tgz is a 404 and http error has minified class name',
+  { skip: isWindows && 'posix only' }, async t => {
+    const gf = new GitFetcher(`localhost:repo/x#${REPO_HEAD}`, opts)
+    const origExtract = RemoteFetcher.prototype.extract
+    RemoteFetcher.prototype.extract = () => {
+      const err = new Error('404 Not Found')
+      err.statusCode = 404
+      err.code = 'E404'
+      return Promise.reject(err)
+    }
+    t.teardown(() => {
+      RemoteFetcher.prototype.extract = origExtract
+    })
+    await gf.extract(me + '/no-tgz')
+  })
+
 t.test('fetch a private repo where the tgz is not a tarball', { skip: isWindows && 'posix only' },
-  t => {
+  () => {
     const gf = new GitFetcher(`localhost:repo/x#${REPO_HEAD}`, opts)
     gf.spec.hosted.tarball = () => `${hostedUrl}/not-tar.tgz`
-    // should NOT retry, because the error was not an HTTP fetch error
-    return t.rejects(gf.extract(me + '/bad-tgz'), {
-      code: 'TAR_BAD_ARCHIVE',
+    // should fall back to clone, since the tarball content is not valid.
+    // this can happen when a hosted git provider returns an HTML page
+    // (e.g. a sign-in page) with HTTP 200 for private repo archives.
+    return gf.extract(me + '/bad-tgz')
+  })
+
+t.test('non-retriable tarball error is thrown, not retried', { skip: isWindows && 'posix only' },
+  async t => {
+    const gf = new GitFetcher(`localhost:repo/x#${REPO_HEAD}`, opts)
+    gf.spec.hosted.tarball = () => `${hostedUrl}/not-tar.tgz`
+    // override the hosted tarball to simulate a non-recoverable error
+    // that is neither an HTTP error nor a TAR error
+    const orig = gf.spec.hosted.tarball
+    gf.spec.hosted.tarball = () => orig()
+    const _extract = RemoteFetcher.prototype.extract
+    t.teardown(() => {
+      RemoteFetcher.prototype.extract = _extract
     })
+    RemoteFetcher.prototype.extract = () =>
+      Promise.reject(Object.assign(new Error('bad'), { code: 'EINTEGRITY' }))
+    await t.rejects(gf.extract(me + '/bad-tgz-other'), { code: 'EINTEGRITY' })
   })
 
 t.test('resolved is a git+ssh url for hosted repos that support it',