fix: cleanup on clone error (#1380)

* chore: fix broken link in README.md (#1154)

* fix(react-native): fix for "<Intermediate Value>.stream is not a function" errors in React Native (#1156)

* Added an error handler for cleanup after failed clone

* chore: add comments

* fix: add recursive delete helper & tests

Co-authored-by: Xavier Francisco <xavier.n.francisco@gmail.com>
Co-authored-by: Corbin Crutchley <crutchcorn@gmail.com>

__tests__/test-clone.js

@@ -172,4 +172,24 @@ describe('clone', () => {
       `'gitdir/refs/heads/master' does not exist`
     )
   })
+
+  it('removes the gitdir when clone fails', async () => {
+    const { fs, dir, gitdir } = await makeFixture('isomorphic-git')
+    const url = `foobar://github.com/isomorphic-git/isomorphic-git`
+    try {
+      await clone({
+        fs,
+        http,
+        dir,
+        gitdir,
+        depth: 1,
+        singleBranch: true,
+        ref: 'test-tag',
+        url,
+      })
+    } catch (err) {
+      // Intentionally left blank.
+    }
+    expect(await fs.exists(gitdir)).toBe(false, `'gitdir' does not exist`)
+  })
 })

src/commands/clone.js

@@ -6,6 +6,7 @@ import { _checkout } from '../commands/checkout.js'
 import { _fetch } from '../commands/fetch.js'
 import { _init } from '../commands/init.js'
 import { GitConfigManager } from '../managers/GitConfigManager.js'
+import { deleteRecursively } from '../utils/deleteRecursively.js'
 
 /**
  * @param {object} args
@@ -59,46 +60,59 @@ export async function _clone({
   noTags,
   headers,
 }) {
-  await _init({ fs, gitdir })
-  await _addRemote({ fs, gitdir, remote, url, force: false })
-  if (corsProxy) {
-    const config = await GitConfigManager.get({ fs, gitdir })
-    await config.set(`http.corsProxy`, corsProxy)
-    await GitConfigManager.save({ fs, gitdir, config })
+  try {
+    await _init({ fs, gitdir })
+    await _addRemote({ fs, gitdir, remote, url, force: false })
+    if (corsProxy) {
+      const config = await GitConfigManager.get({ fs, gitdir })
+      await config.set(`http.corsProxy`, corsProxy)
+      await GitConfigManager.save({ fs, gitdir, config })
+    }
+    const { defaultBranch, fetchHead } = await _fetch({
+      fs,
+      cache,
+      http,
+      onProgress,
+      onMessage,
+      onAuth,
+      onAuthSuccess,
+      onAuthFailure,
+      gitdir,
+      ref,
+      remote,
+      corsProxy,
+      depth,
+      since,
+      exclude,
+      relative,
+      singleBranch,
+      headers,
+      tags: !noTags,
+    })
+    if (fetchHead === null) return
+    ref = ref || defaultBranch
+    ref = ref.replace('refs/heads/', '')
+    // Checkout that branch
+    await _checkout({
+      fs,
+      cache,
+      onProgress,
+      dir,
+      gitdir,
+      ref,
+      remote,
+      noCheckout,
+    })
+  } catch (err) {
+    // Remove partial local repository, see #1283
+    try {
+      await deleteRecursively({ fs, dirname: gitdir })
+    } catch (err) {
+      // Ignore this error, we are already failing.
+      // This try-catch is necessary so the original error is
+      // not masked by potential errors in deleteRecursively.
+    }
+
+    throw err
   }
-  const { defaultBranch, fetchHead } = await _fetch({
-    fs,
-    cache,
-    http,
-    onProgress,
-    onMessage,
-    onAuth,
-    onAuthSuccess,
-    onAuthFailure,
-    gitdir,
-    ref,
-    remote,
-    corsProxy,
-    depth,
-    since,
-    exclude,
-    relative,
-    singleBranch,
-    headers,
-    tags: !noTags,
-  })
-  if (fetchHead === null) return
-  ref = ref || defaultBranch
-  ref = ref.replace('refs/heads/', '')
-  // Checkout that branch
-  await _checkout({
-    fs,
-    cache,
-    onProgress,
-    dir,
-    gitdir,
-    ref,
-    remote,
-    noCheckout,
-  })
 }

src/utils/deleteRecursively.js

@@ -0,0 +1,32 @@
+import { join } from './join'
+
+/**
+ * @param {object} args
+ * @param {import('../models/FileSystem.js').FileSystem} args.fs
+ * @param {string} args.dirname
+ */
+export async function deleteRecursively({ fs, dirname }) {
+  const filesToDelete = []
+  const directoriesToDelete = []
+  const pathsToTraverse = [dirname]
+
+  while (pathsToTraverse.length > 0) {
+    const path = pathsToTraverse.pop()
+
+    if ((await fs._stat(path)).isDirectory()) {
+      directoriesToDelete.push(path)
+      pathsToTraverse.push(
+        ...(await fs.readdir(path)).map(subPath => join(path, subPath))
+      )
+    } else {
+      filesToDelete.push(path)
+    }
+  }
+
+  for (const path of filesToDelete) {
+    await fs.rm(path)
+  }
+  for (const path of directoriesToDelete.reverse()) {
+    await fs.rmdir(path)
+  }
+}