Skip to content

Commit 84535a3

Browse files
committedNov 11, 2019
git: Fall back from tgz to ssh on HTTP errors
An HTTP error on fetching a remote tarball for a git repo is usually going to be the result of pulling in a private repo, which is available via SSH, but not as a pre-rendered gzipped tarball over HTTPS. Fall back to SSH when the hosted.tarball() fetch fails, but only if the error is an HTTP error of some kind (not, eg, an invalid tarball response.) Part 2 of fix for #15
1 parent 7ee23c3 commit 84535a3

File tree

2 files changed

+34
-5
lines changed

2 files changed

+34
-5
lines changed
 

‎lib/git.js

+13-3
Original file line numberDiff line numberDiff line change
@@ -174,22 +174,32 @@ class GitFetcher extends Fetcher {
174174
// TODO: after cloning, create a tarball of the folder, and add to the cache
175175
// with cacache.put.stream(), using a key that's deterministic based on the
176176
// spec and repo, so that we don't ever clone the same thing multiple times.
177-
[_clone] (handler) {
177+
[_clone] (handler, tarballOk = true) {
178178
const o = { tmpPrefix: 'git-clone' }
179179
const ref = this.resolvedSha || this.spec.gitCommittish
180180
const h = this.spec.hosted
181181
const resolved = this.resolved
182182

183+
// can be set manually to false to fall back to sshurl
184+
tarballOk = tarballOk &&
185+
h && resolved === repoUrl(h, { noCommittish: false }) && h.tarball
186+
183187
return cacache.tmp.withTmp(this.cache, o, tmp => {
184188
// if we're resolved, and have a tarball url, shell out to RemoteFetcher
185-
if (h && resolved === repoUrl(h, { noCommittish: false }) && h.tarball) {
189+
if (tarballOk) {
186190
const nameat = this.spec.name ? `${this.spec.name}@` : ''
187191
return new RemoteFetcher(h.tarball({ noCommittish: false }), {
188192
...this.opts,
189193
pkgid: `git:${nameat}${this.resolved}`,
190194
resolved: this.resolved,
191195
integrity: null, // it'll always be different, if we have one
192-
}).extract(tmp).then(() => handler(tmp))
196+
}).extract(tmp).then(() => handler(tmp), er => {
197+
// fall back to ssh download if tarball fails
198+
if (er.constructor.name.match(/^Http/))
199+
return this[_clone](handler, false)
200+
else
201+
throw er
202+
})
193203
}
194204

195205
const repo = h && !this.spec.fetchSpec

‎test/git.js

+21-2
Original file line numberDiff line numberDiff line change
@@ -219,16 +219,19 @@ t.test('setup', { bail: true }, t => {
219219
return res.end(data)
220220
} catch (er) {
221221
res.statusCode = 500
222-
res.end(er.stack)
222+
return res.end(er.stack)
223223
}
224224
case '/repo-1.0.0.tgz': try {
225225
const data = fs.readFileSync(me + '/repo-1.0.0.tgz')
226226
res.setHeader('content-length', data.length)
227227
return res.end(data)
228228
} catch (er) {
229229
res.statusCode = 500
230-
res.end(er.stack)
230+
return res.end(er.stack)
231231
}
232+
case '/not-tar.tgz':
233+
res.statusCode = 200
234+
return res.end('this is not a gzipped tarball tho')
232235
default:
233236
res.statusCode = 404
234237
return res.end('not found')
@@ -377,3 +380,19 @@ t.test('fetch a weird ref', t => {
377380

378381
t.end()
379382
})
383+
384+
t.test('fetch a private repo where the tgz is a 404', t => {
385+
const gf = new GitFetcher(`localhost:repo/x#${REPO_HEAD}`, {cache})
386+
gf.spec.hosted.tarball = () => `${hostedUrl}/not-found.tgz`
387+
// should fetch it by falling back to ssh when it gets an http error
388+
return gf.extract(me + '/no-tgz')
389+
})
390+
391+
t.test('fetch a private repo where the tgz is not a tarball', t => {
392+
const gf = new GitFetcher(`localhost:repo/x#${REPO_HEAD}`, {cache})
393+
gf.spec.hosted.tarball = () => `${hostedUrl}/not-tar.tgz`
394+
// should NOT retry, because the error was not an HTTP fetch error
395+
return t.rejects(gf.extract(me + '/bad-tgz'), {
396+
code: 'TAR_BAD_ARCHIVE',
397+
})
398+
})

0 commit comments

Comments
 (0)
Please sign in to comment.