fix(core): warn instead of error when attempting to scan non-directory

Fixes #2430

Also adds more specific and helpful error messages when we encounter
missing/invalid submodule paths.

core/src/util/fs.ts

@@ -11,7 +11,7 @@ import klaw = require("klaw")
 import glob from "glob"
 import tmp from "tmp-promise"
 import _spawn from "cross-spawn"
-import { pathExists, readFile, writeFile, lstat, realpath } from "fs-extra"
+import { pathExists, readFile, writeFile, lstat, realpath, Stats } from "fs-extra"
 import { join, basename, win32, posix } from "path"
 import { platform } from "os"
 
@@ -308,3 +308,28 @@ export async function makeTempDir({ git = false }: { git?: boolean } = {}): Prom
 
   return tmpDir
 }
+
+/**
+ * Returns the type of the given fs.Stats object as a string.
+ *
+ * @param stats an fs.Stats instance
+ */
+export function getStatsType(stats: Stats) {
+  if (stats.isBlockDevice()) {
+    return "block device"
+  } else if (stats.isCharacterDevice()) {
+    return "character device"
+  } else if (stats.isDirectory()) {
+    return "directory"
+  } else if (stats.isFIFO()) {
+    return "named pipe"
+  } else if (stats.isFile()) {
+    return "file"
+  } else if (stats.isSocket()) {
+    return "socket"
+  } else if (stats.isSymbolicLink()) {
+    return "symbolic link"
+  } else {
+    return "unknown"
+  }
+}

core/src/vcs/git.ts

@@ -16,7 +16,7 @@ import split2 = require("split2")
 import { VcsHandler, RemoteSourceParams, VcsFile, GetFilesParams } from "./vcs"
 import { ConfigurationError, RuntimeError } from "../exceptions"
 import Bluebird from "bluebird"
-import { joinWithPosix, matchPath } from "../util/fs"
+import { getStatsType, joinWithPosix, matchPath } from "../util/fs"
 import { deline } from "../util/string"
 import { splitLast, exec } from "../util/util"
 import { LogEntry } from "../logger/log-entry"
@@ -25,6 +25,9 @@ import { Profile } from "../util/profiling"
 import { SortedStreamIntersection } from "../util/streams"
 import execa = require("execa")
 import isGlob = require("is-glob")
+import chalk = require("chalk")
+
+const submoduleErrorSuggestion = `Perhaps you need to run ${chalk.underline(`git submodule update --recursive`)}?`
 
 export function getCommitIdFromRefList(refList: string[]): string {
   try {
@@ -123,6 +126,29 @@ export class GitHandler extends VcsHandler {
 
     log = log.debug(`Scanning ${pathDescription} at ${path}\nIncludes: ${include}\nExcludes:${exclude}`)
 
+    try {
+      const pathStats = await stat(path)
+
+      if (!pathStats.isDirectory()) {
+        log.warn({
+          symbol: "warning",
+          msg: chalk.gray(`Expected directory at ${path}, but found ${getStatsType(pathStats)}.`),
+        })
+        return []
+      }
+    } catch (err) {
+      // 128 = File no longer exists
+      if (err.exitCode === 128 || err.code === "ENOENT") {
+        log.warn({
+          symbol: "warning",
+          msg: chalk.gray(`Attempted to scan directory at ${path}, but it does not exist.`),
+        })
+        return []
+      } else {
+        throw err
+      }
+    }
+
     const git = this.gitCli(log, path)
     const gitRoot = await this.getRepoRoot(log, path)
 
@@ -306,6 +332,35 @@ export class GitHandler extends VcsHandler {
           // Note: We apply include/exclude filters after listing files from submodule
           const submoduleRelPath = relative(path, submodulePath)
 
+          // Catch and show helpful message in case the submodule path isn't a valid directory
+          try {
+            const pathStats = await stat(path)
+
+            if (!pathStats.isDirectory()) {
+              const pathType = getStatsType(pathStats)
+              log.warn({
+                symbol: "warning",
+                msg: chalk.gray(
+                  `Expected submodule directory at ${path}, but found ${pathType}. ${submoduleErrorSuggestion}`
+                ),
+              })
+              return
+            }
+          } catch (err) {
+            // 128 = File no longer exists
+            if (err.exitCode === 128 || err.code === "ENOENT") {
+              log.warn({
+                symbol: "warning",
+                msg: chalk.yellow(
+                  `Found reference to submodule at ${submoduleRelPath}, but the path could not be found. ${submoduleErrorSuggestion}`
+                ),
+              })
+              return
+            } else {
+              throw err
+            }
+          }
+
           files.push(
             ...(await this.getFiles({
               log,
@@ -518,14 +573,9 @@ export class GitHandler extends VcsHandler {
       return (await git("rev-parse", "--abbrev-ref", "HEAD"))[0]
     } catch (err) {
       if (err.exitCode === 128) {
-        try {
-          // If this doesn't throw, then we're in a repo with no commits, or with a detached HEAD.
-          await git("rev-parse", "--show-toplevel")
-          return undefined
-        } catch (notInRepoError) {
-          // Throw nice error when we detect that we're not in a repo root
-          throw new RuntimeError(notInRepoRootErrorMessage(path), { path })
-        }
+        // If this doesn't throw, then we're in a repo with no commits, or with a detached HEAD.
+        await this.getRepoRoot(log, path)
+        return undefined
       } else {
         throw err
       }

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

@@ -435,6 +435,21 @@ describe("GitHandler", () => {
       expect(files).to.eql([filePath])
     })
 
+    it("gracefully aborts if given path doesn't exist", async () => {
+      const path = resolve(tmpPath, "foo")
+
+      const files = (await handler.getFiles({ path, exclude: [], log })).map((f) => f.path)
+      expect(files).to.eql([])
+    })
+
+    it("gracefully aborts if given path is not a directory", async () => {
+      const path = resolve(tmpPath, "foo")
+      await createFile(path)
+
+      const files = (await handler.getFiles({ path, exclude: [], log })).map((f) => f.path)
+      expect(files).to.eql([])
+    })
+
     context("path contains a submodule", () => {
       let submodule: tmp.DirectoryResult
       let submodulePath: string
@@ -545,6 +560,27 @@ describe("GitHandler", () => {
         expect(paths).to.include(join("sub", initFile))
       })
 
+      it("gracefully skips submodule if its path doesn't exist", async () => {
+        const subPath = join(tmpPath, "sub")
+        await remove(subPath)
+
+        const files = await handler.getFiles({ path: tmpPath, log })
+        const paths = files.map((f) => relative(tmpPath, f.path))
+
+        expect(paths).to.eql([".gitmodules"])
+      })
+
+      it("gracefully skips submodule if its path doesn't point to a directory", async () => {
+        const subPath = join(tmpPath, "sub")
+        await remove(subPath)
+        await createFile(subPath)
+
+        const files = await handler.getFiles({ path: tmpPath, log })
+        const paths = files.map((f) => relative(tmpPath, f.path))
+
+        expect(paths).to.eql([".gitmodules"])
+      })
+
       context("submodule contains another submodule", () => {
         let submoduleB: tmp.DirectoryResult
         let submodulePathB: string