fix(git): fix confusing error messages on exit code 128 (#5439)

* fix(git): fix confusing error messages on exit code 128

* refactor(git): extract helper function to examine error messages

* refactor(git): inline helper function

It's used in the only place an is error-specific.

* chore: re-arrange imports

* refactor(git): restructure if/else blocks for better readability

* refactor(git): extract helper for error explanation

* test(git): add unit tests for `explainGitError`

* improvement(git): print original Git error on repo ownership issues

core/src/vcs/git.ts

@@ -10,11 +10,10 @@ import { performance } from "perf_hooks"
 import { isAbsolute, join, posix, relative, resolve } from "path"
 import { isString } from "lodash-es"
 import fsExtra from "fs-extra"
-
-const { createReadStream, ensureDir, lstat, pathExists, readlink, realpath, stat } = fsExtra
 import { PassThrough } from "stream"
-import type { GetFilesParams, RemoteSourceParams, VcsFile, VcsInfo, VcsHandlerParams } from "./vcs.js"
+import type { GetFilesParams, RemoteSourceParams, VcsFile, VcsHandlerParams, VcsInfo } from "./vcs.js"
 import { VcsHandler } from "./vcs.js"
+import type { GardenError } from "../exceptions.js"
 import { ChildProcessError, ConfigurationError, isErrnoException, RuntimeError } from "../exceptions.js"
 import { getStatsType, joinWithPosix, matchPath } from "../util/fs.js"
 import { dedent, deline, splitLast } from "../util/string.js"
@@ -34,6 +33,8 @@ import { execa } from "execa"
 import hasha from "hasha"
 import { styles } from "../logger/styles.js"
 
+const { createReadStream, ensureDir, lstat, pathExists, readlink, realpath, stat } = fsExtra
+
 const submoduleErrorSuggestion = `Perhaps you need to run ${styles.underline(`git submodule update --recursive`)}?`
 
 interface GitEntry extends VcsFile {
@@ -133,21 +134,7 @@ export class GitHandler extends VcsHandler {
         if (!(err instanceof ChildProcessError)) {
           throw err
         }
-        if (err.details.code === 128 && err.details.stderr.toLowerCase().includes("fatal: unsafe repository")) {
-          // Throw nice error when we detect that we're not in a repo root
-          throw new RuntimeError({
-            message:
-              err.details.stderr +
-              `\nIt looks like you're using Git 2.36.0 or newer and the repo directory containing "${path}" is owned by someone else. If this is intentional you can run "git config --global --add safe.directory '<repo root>'" and try again.`,
-          })
-        } else if (err.details.code === 128) {
-          // Throw nice error when we detect that we're not in a repo root
-          throw new RuntimeError({
-            message: notInRepoRootErrorMessage(path),
-          })
-        } else {
-          throw err
-        }
+        throw explainGitError(err, path)
       }
     })
   }
@@ -690,11 +677,28 @@ export class GitHandler extends VcsHandler {
   }
 }
 
-const notInRepoRootErrorMessage = (path: string) => deline`
+function gitErrorContains(err: ChildProcessError, substring: string): boolean {
+  return err.details.stderr.toLowerCase().includes(substring.toLowerCase())
+}
+
+export function explainGitError(err: ChildProcessError, path: string): GardenError {
+  // handle some errors with exit codes 128 in a specific manner
+  if (err.details.code === 128) {
+    if (gitErrorContains(err, "fatal: not a git repository")) {
+      // Throw nice error when we detect that we're not in a repo root
+      return new RuntimeError({
+        message: deline`
     Path ${path} is not in a git repository root. Garden must be run from within a git repo.
     Please run \`git init\` if you're starting a new project and repository, or move the project to an
     existing repository, and try again.
-  `
+  `,
+      })
+    }
+  }
+
+  // otherwise just re-throw the original error
+  return err
+}
 
 /**
  * Given a list of POSIX-style globs/paths and a `basePath`, find paths that point to a directory and append `**\/*`

core/test/unit/src/vcs/git.ts

@@ -10,21 +10,21 @@ import { execa } from "execa"
 import { expect } from "chai"
 import tmp from "tmp-promise"
 import fsExtra from "fs-extra"
-
-const { createFile, ensureSymlink, lstat, mkdir, mkdirp, realpath, remove, symlink, writeFile } = fsExtra
 import { basename, join, relative, resolve } from "path"
 
 import type { TestGarden } from "../../../helpers.js"
 import { expectError, makeTestGardenA } from "../../../helpers.js"
 import type { GitCli } from "../../../../src/vcs/git.js"
-import { getCommitIdFromRefList, GitHandler, parseGitUrl } from "../../../../src/vcs/git.js"
+import { explainGitError, getCommitIdFromRefList, GitHandler, parseGitUrl } from "../../../../src/vcs/git.js"
 import type { Log } from "../../../../src/logger/log-entry.js"
 import { hashRepoUrl } from "../../../../src/util/ext-source-util.js"
-import { deline } from "../../../../src/util/string.js"
+import { dedent, deline } from "../../../../src/util/string.js"
 import { uuidv4 } from "../../../../src/util/random.js"
 import type { VcsHandlerParams } from "../../../../src/vcs/vcs.js"
 import { repoRoot } from "../../../../src/util/testing.js"
-import { GardenError } from "../../../../src/exceptions.js"
+import { ChildProcessError, GardenError, RuntimeError } from "../../../../src/exceptions.js"
+
+const { createFile, ensureSymlink, lstat, mkdir, mkdirp, realpath, remove, symlink, writeFile } = fsExtra
 
 // Overriding this to make sure any ignorefile name is respected
 export const defaultIgnoreFilename = ".testignore"
@@ -988,4 +988,69 @@ describe("git", () => {
       await expectError(() => parseGitUrl(url), "configuration")
     })
   })
+
+  describe("explainGitError", () => {
+    const path = "/tmp"
+
+    function getChildProcessError(exitCode: number, stderr: string): ChildProcessError {
+      return new ChildProcessError({
+        cmd: "git",
+        args: ["rev-parse", "--show-toplevel"],
+        code: exitCode,
+        output: stderr,
+        stderr,
+        stdout: "",
+      })
+    }
+
+    context("on error code 128", () => {
+      const exitCode = 128
+
+      it("should throw a nice error when given a path outside of a repo", async () => {
+        const stderr = "fatal: not a git repository (or any of the parent directories): .git"
+        const gitError = getChildProcessError(exitCode, stderr)
+
+        const explainedGitError = explainGitError(gitError, path)
+        expect(explainedGitError).to.be.instanceof(RuntimeError)
+        expect(explainedGitError.message).to.eql(deline`
+          Path ${path} is not in a git repository root. Garden must be run from within a git repo.
+          Please run \`git init\` if you're starting a new project and repository, or move the project to
+          an existing repository, and try again.
+        `)
+      })
+
+      it("should throw an original error with exit code 128 the rest cases", async () => {
+        const stderr = `fatal: another git error with exit code ${exitCode}`
+        const gitError = getChildProcessError(exitCode, stderr)
+
+        const explainedGitError = explainGitError(gitError, path)
+        expect(explainedGitError).to.be.instanceof(ChildProcessError)
+
+        const castedExplainedGitError = explainedGitError as ChildProcessError
+        expect(castedExplainedGitError.details.code).to.eql(exitCode)
+        expect(castedExplainedGitError.message).to.eql(dedent`
+          Command "git rev-parse --show-toplevel" failed with code ${exitCode}:
+
+          ${stderr}
+        `)
+      })
+    })
+
+    it("should throw an original error when exit code is not 128", async () => {
+      const exitCode = -1
+      const stderr = "fatal: unexpected git error"
+      const gitError = getChildProcessError(exitCode, stderr)
+
+      const explainedGitError = explainGitError(gitError, path)
+      expect(explainedGitError).to.be.instanceof(ChildProcessError)
+
+      const castedExplainedGitError = explainedGitError as ChildProcessError
+      expect(castedExplainedGitError.details.code).to.eql(exitCode)
+      expect(castedExplainedGitError.message).to.eql(dedent`
+          Command "git rev-parse --show-toplevel" failed with code ${exitCode}:
+
+          ${stderr}
+        `)
+    })
+  })
 })