Treat non-zero exit code as error by default

lib/errors.ts

@@ -172,10 +172,12 @@ export const GitErrorRegexes: { [regexp: string]: GitError } = {
 export class ExecError extends Error {
   /**
    * The error.code property is a string label that identifies the kind of error
+   * or, in the case of code being a Number it's indicating the exit code of
+   * the executed process.
    *
    * See https://nodejs.org/api/errors.html#errorcode
    */
-  public readonly code?: string
+  public readonly code?: string | number
 
   /**
    * The signal that terminated the process
@@ -199,7 +201,7 @@ export class ExecError extends Error {
 
     if (cause && typeof cause === 'object') {
       if ('code' in cause) {
-        if (typeof cause.code === 'string') {
+        if (typeof cause.code === 'string' || typeof cause.code === 'number') {
           this.code = cause.code
         }
       }

lib/exec.ts

@@ -2,6 +2,9 @@ import { ChildProcess, execFile, ExecFileOptions } from 'child_process'
 import { setupEnvironment } from './git-environment'
 import { ExecError } from './errors'
 import { ignoreClosedInputStream } from './ignore-closed-input-stream'
+import { promisify } from 'util'
+
+const execFileAsync = promisify(execFile)
 
 export interface IGitResult {
   /** The standard output from git. */
@@ -95,6 +98,8 @@ export interface IGitExecutionOptions {
    * AbortSignal being triggered. Defaults to 'SIGTERM'
    */
   readonly killSignal?: ExecFileOptions['killSignal']
+
+  readonly ignoreExitCodes?: ReadonlyArray<number> | true
 }
 
 export interface IGitStringExecutionOptions extends IGitExecutionOptions {
@@ -105,6 +110,17 @@ export interface IGitBufferExecutionOptions extends IGitExecutionOptions {
   readonly encoding: 'buffer'
 }
 
+const coerceStdio = (
+  stdio: string | Buffer,
+  encoding: BufferEncoding | 'buffer'
+) => {
+  if (encoding === 'buffer') {
+    return Buffer.isBuffer(stdio) ? stdio : Buffer.from(stdio)
+  }
+
+  return Buffer.isBuffer(stdio) ? stdio.toString(encoding) : stdio
+}
+
 /**
  * Execute a command and read the output using the embedded Git environment.
  *
@@ -141,42 +157,42 @@ export function exec(
     killSignal: options?.killSignal,
   }
 
-  return new Promise<IGitResult>((resolve, reject) => {
-    const cp = execFile(gitLocation, args, opts, (err, stdout, stderr) => {
-      if (!err || typeof err.code === 'number') {
-        const exitCode = typeof err?.code === 'number' ? err.code : 0
-        resolve({ stdout, stderr, exitCode })
-        return
-      }
-
-      // If the error's code is a string then it means the code isn't the
-      // process's exit code but rather an error coming from Node's bowels,
-      // e.g., ENOENT.
-      let { message } = err
-
-      if (err.code === 'ENOENT') {
-        message =
-          `ENOENT: Git failed to execute. This typically means that ` +
-          `the path provided doesn't exist or that the Git executable ` +
-          `could not be found which could indicate a problem with the ` +
-          `packaging of dugite. Verify that resolveGitBinary returns a ` +
-          `valid path to the git binary.`
-      }
-
-      reject(new ExecError(message, stdout, stderr, err))
-    })
+  const promise = execFileAsync(gitLocation, args, opts)
 
-    ignoreClosedInputStream(cp)
+  ignoreClosedInputStream(promise.child)
 
-    if (options?.stdin !== undefined && cp.stdin) {
+  promise.child.on('spawn', () => {
+    if (options?.stdin !== undefined && promise.child.stdin) {
       // See https://github.com/nodejs/node/blob/7b5ffa46fe4d2868c1662694da06eb55ec744bde/test/parallel/test-stdin-pipe-large.js
       if (options.stdinEncoding) {
-        cp.stdin.end(options.stdin, options.stdinEncoding)
+        promise.child.stdin.end(options.stdin, options.stdinEncoding)
       } else {
-        cp.stdin.end(options.stdin)
+        promise.child.stdin.end(options.stdin)
       }
     }
-
-    options?.processCallback?.(cp)
   })
+
+  options?.processCallback?.(promise.child)
+
+  return promise
+    .then(({ stdout, stderr }) => ({ stdout, stderr, exitCode: 0 }))
+    .catch(e => {
+      if (typeof e !== 'object') {
+        const stdio = coerceStdio('', opts.encoding)
+        throw new ExecError(typeof e === 'string' ? e : `${e}`, stdio, stdio, e)
+      }
+
+      const stdout = coerceStdio('stdout' in e ? e.stdout : '', opts.encoding)
+      const stderr = coerceStdio('stderr' in e ? e.stderr : '', opts.encoding)
+
+      if ('code' in e && typeof e.code === 'number') {
+        const ignoreExitCodes = options?.ignoreExitCodes
+        if (ignoreExitCodes === true || ignoreExitCodes?.includes(e.code)) {
+          return { stdout, stderr, exitCode: e.code }
+        }
+        throw new ExecError(`Git failed with code ${e.code}`, stdout, stderr, e)
+      }
+
+      throw new ExecError(e.message, stdout, stderr, e)
+    })
 }

test/fast/config-test.ts

@@ -19,8 +19,10 @@ describe('config', () => {
     if (process.platform === 'win32') {
       const result = await exec(
         ['config', '--system', 'http.sslCAInfo'],
-        os.homedir()
+        os.homedir(),
+        { ignoreExitCodes: [1] }
       )
+      assert.equal(result.exitCode, 1)
       assert.equal(result.stdout.trim(), '')
     }
   })

test/fast/errors-test.ts

@@ -14,7 +14,8 @@ describe('detects errors', () => {
 
     const result = await exec(
       ['remote', 'add', 'new-remote', 'https://gitlab.com'],
-      repoPath
+      repoPath,
+      { ignoreExitCodes: true }
     )
 
     assertHasGitError(result, GitError.RemoteAlreadyExists)
@@ -30,15 +31,19 @@ describe('detects errors', () => {
     await exec(['tag', 'v0.1'], repoPath)
 
     // try to make the same tag again
-    const result = await exec(['tag', 'v0.1'], repoPath)
+    const result = await exec(['tag', 'v0.1'], repoPath, {
+      ignoreExitCodes: true,
+    })
 
     assertHasGitError(result, GitError.TagAlreadyExists)
   })
   it('BranchAlreadyExists', async () => {
     const path = await initialize('branch-already-exists', 'foo')
     await exec(['commit', '-m', 'initial', '--allow-empty'], path)
 
-    const result = await exec(['branch', 'foo'], path)
+    const result = await exec(['branch', 'foo'], path, {
+      ignoreExitCodes: true,
+    })
 
     assertHasGitError(result, GitError.BranchAlreadyExists)
   })
@@ -50,6 +55,7 @@ describe('detects errors', () => {
       env: {
         GIT_TEST_ASSUME_DIFFERENT_OWNER: '1',
       },
+      ignoreExitCodes: true,
     })
 
     assertHasGitError(result, GitError.UnsafeDirectory)
@@ -74,7 +80,9 @@ describe('detects errors', () => {
 
       await exec(['config', 'core.autocrlf', 'nab'], repoPath)
 
-      const result = await exec(['commit', '-m', 'add a text file'], repoPath)
+      const result = await exec(['commit', '-m', 'add a text file'], repoPath, {
+        ignoreExitCodes: true,
+      })
 
       assertHasGitError(result, GitError.BadConfigValue)
 
@@ -92,7 +100,9 @@ describe('detects errors', () => {
 
       await exec(['config', 'core.repositoryformatversion', 'nan'], repoPath)
 
-      const result = await exec(['commit', '-m', 'add a text file'], repoPath)
+      const result = await exec(['commit', '-m', 'add a text file'], repoPath, {
+        ignoreExitCodes: true,
+      })
 
       assertHasGitError(result, GitError.BadConfigValue)
 

test/fast/git-process-test.ts

@@ -62,10 +62,10 @@ describe('git-process', () => {
   describe('exitCode', () => {
     it('returns exit code when folder is empty', async () => {
       const testRepoPath = temp.mkdirSync('desktop-git-test-blank')
-      const result = await git(['show', 'HEAD'], testRepoPath)
-      verify(result, r => {
-        assert.equal(r.exitCode, 128)
+      const result = await git(['show', 'HEAD'], testRepoPath, {
+        ignoreExitCodes: [128],
       })
+      assert.equal(result.exitCode, 128)
     })
 
     it('handles stdin closed errors', async () => {
@@ -77,6 +77,7 @@ describe('git-process', () => {
       // EPIPE/EOF error thrown from process.stdin
       const result = await git(['--trololol'], testRepoPath, {
         stdin: '\n'.repeat(1024 * 1024),
+        ignoreExitCodes: true,
       })
       verify(result, r => {
         assert.equal(r.exitCode, 129)
@@ -99,7 +100,8 @@ describe('git-process', () => {
             '/dev/null',
             'new-file.md',
           ],
-          testRepoPath
+          testRepoPath,
+          { ignoreExitCodes: true }
         )
 
         verify(result, r => {
@@ -131,7 +133,8 @@ describe('git-process', () => {
             '/dev/null',
             'new-file.md',
           ],
-          testRepoPath
+          testRepoPath,
+          { ignoreExitCodes: true }
         )
 
         verify(result, r => {
@@ -169,7 +172,9 @@ describe('git-process', () => {
       it('missing from index', async () => {
         const testRepoPath = await initialize('desktop-show-missing-index')
 
-        const result = await git(['show', ':missing.txt'], testRepoPath)
+        const result = await git(['show', ':missing.txt'], testRepoPath, {
+          ignoreExitCodes: true,
+        })
 
         assertHasGitError(result, GitError.PathDoesNotExist)
       })
@@ -183,7 +188,9 @@ describe('git-process', () => {
         await git(['add', '.'], testRepoPath)
         await git(['commit', '-m', '"added a file"'], testRepoPath)
 
-        const result = await git(['show', 'HEAD:missing.txt'], testRepoPath)
+        const result = await git(['show', 'HEAD:missing.txt'], testRepoPath, {
+          ignoreExitCodes: true,
+        })
 
         assertHasGitError(result, GitError.PathDoesNotExist)
       })
@@ -192,7 +199,9 @@ describe('git-process', () => {
           'desktop-show-invalid-object-empty'
         )
 
-        const result = await git(['show', 'HEAD:missing.txt'], testRepoPath)
+        const result = await git(['show', 'HEAD:missing.txt'], testRepoPath, {
+          ignoreExitCodes: true,
+        })
 
         assertHasGitError(result, GitError.InvalidObjectName)
       })
@@ -206,7 +215,9 @@ describe('git-process', () => {
         await git(['add', '.'], testRepoPath)
         await git(['commit', '-m', '"added a file"'], testRepoPath)
 
-        const result = await git(['show', '--', '/missing.txt'], testRepoPath)
+        const result = await git(['show', '--', '/missing.txt'], testRepoPath, {
+          ignoreExitCodes: true,
+        })
 
         assertHasGitError(result, GitError.OutsideRepository)
       })
@@ -234,15 +245,9 @@ describe('git-process', () => {
     it('raises error when folder does not exist', async () => {
       const testRepoPath = path.join(temp.path(), 'desktop-does-not-exist')
 
-      let error: Error | null = null
-      try {
-        await git(['show', 'HEAD'], testRepoPath)
-      } catch (e) {
-        error = e as Error
-      }
+      const e = await git(['show', 'HEAD'], testRepoPath).catch(e => e)
 
-      assert.ok(error?.message.includes('Git failed to execute.'))
-      assert.equal((error as any).code, 'ENOENT')
+      assert.equal(e.code, 'ENOENT')
     })
 
     it('can parse HTTPS auth errors', () => {
@@ -524,7 +529,9 @@ mark them as resolved using git add`
       })
 
       // Execute a merge.
-      const result = await git(['merge', 'some-other-branch'], repoPath)
+      const result = await git(['merge', 'some-other-branch'], repoPath, {
+        ignoreExitCodes: true,
+      })
 
       assertHasGitError(result, GitError.MergeWithLocalChanges)
     })
@@ -553,7 +560,9 @@ mark them as resolved using git add`
       })
 
       // Execute a rebase.
-      const result = await git(['rebase', 'some-other-branch'], repoPath)
+      const result = await git(['rebase', 'some-other-branch'], repoPath, {
+        ignoreExitCodes: true,
+      })
 
       assertHasGitError(result, GitError.RebaseWithLocalChanges)
     })
@@ -595,7 +604,9 @@ mark them as resolved using git add`
       })
 
       // Pull from the fork
-      const result = await git(['pull', 'origin', 'HEAD'], forkRepoPath)
+      const result = await git(['pull', 'origin', 'HEAD'], forkRepoPath, {
+        ignoreExitCodes: true,
+      })
 
       assertHasGitError(result, GitError.MergeWithLocalChanges)
     })
@@ -637,7 +648,9 @@ mark them as resolved using git add`
       })
 
       // Pull from the fork
-      const result = await git(['pull', 'origin', 'HEAD'], forkRepoPath)
+      const result = await git(['pull', 'origin', 'HEAD'], forkRepoPath, {
+        ignoreExitCodes: true,
+      })
 
       assertHasGitError(result, GitError.RebaseWithLocalChanges)
     })
@@ -671,7 +684,9 @@ mark them as resolved using git add`
       )
 
       // Try to merge the branch.
-      const result = await git(['merge', 'my-branch'], repoPath)
+      const result = await git(['merge', 'my-branch'], repoPath, {
+        ignoreExitCodes: true,
+      })
 
       assertHasGitError(result, GitError.MergeConflicts)
     })
@@ -705,7 +720,9 @@ mark them as resolved using git add`
       )
 
       // Try to merge the branch.
-      const result = await git(['rebase', 'my-branch'], repoPath)
+      const result = await git(['rebase', 'my-branch'], repoPath, {
+        ignoreExitCodes: [1],
+      })
 
       assertHasGitError(result, GitError.RebaseConflicts)
     })