fix: error handling skips dropping backup stash after internal git er…

…rors

lib/gitWorkflow.js

@@ -31,19 +31,8 @@ const cleanUntrackedFiles = async execGit => {
   }
 }
 
-const isGitLockError = error =>
-  error.message.includes('Another git process seems to be running in this repository')
-
-const isGitMergeStateError = error =>
-  error.message.includes('Merge state could not be restored due to an error!')
-
 const handleError = (error, ctx) => {
-  ctx.hasErrors = true
-  if (isGitLockError(error)) {
-    ctx.hasGitLockError = true
-  } else if (isGitMergeStateError(error)) {
-    ctx.hasGitMergeStateError = true
-  }
+  ctx.gitError = true
   throw error
 }
 
@@ -66,10 +55,13 @@ class GitWorkflow {
   /**
    * Get name of backup stash
    */
-  async getBackupStash() {
+  async getBackupStash(ctx) {
     const stashes = await this.execGit(['stash', 'list'])
     const index = stashes.split('\n').findIndex(line => line.includes(STASH))
-    if (index === -1) throw new Error('lint-staged automatic backup is missing!')
+    if (index === -1) {
+      ctx.gitGetBackupStashError = true
+      throw new Error('lint-staged automatic backup is missing!')
+    }
     return `stash@{${index}}`
   }
 
@@ -136,7 +128,7 @@ class GitWorkflow {
         '--no-color',
         '--no-ext-diff',
         '--patch',
-        await this.getBackupStash(),
+        await this.getBackupStash(ctx),
         '-R' // Show diff in reverse
       ])
 
@@ -179,8 +171,11 @@ class GitWorkflow {
         } catch (error2) {
           debug('Error while restoring unstaged changes using 3-way merge:')
           debug(error2)
-          ctx.hasErrors = true
-          throw new Error('Unstaged changes could not be restored due to a merge conflict!')
+          ctx.gitApplyModificationsError = true
+          handleError(
+            new Error('Unstaged changes could not be restored due to a merge conflict!'),
+            ctx
+          )
         }
       }
       debug('Done restoring unstaged changes!')
@@ -190,7 +185,7 @@ class GitWorkflow {
     // Git will return with error code if the commit doesn't exist
     // See https://stackoverflow.com/a/52357762
     try {
-      const backupStash = await this.getBackupStash()
+      const backupStash = await this.getBackupStash(ctx)
       const output = await this.execGit(['show', '--format=%b', `${backupStash}^3`])
       const untrackedDiff = output.replace(/^\n*/, '') // remove empty lines from start of output
       if (!untrackedDiff) return
@@ -204,7 +199,7 @@ class GitWorkflow {
   async restoreOriginalState(ctx) {
     try {
       debug('Restoring original state...')
-      const backupStash = await this.getBackupStash()
+      const backupStash = await this.getBackupStash(ctx)
       await this.execGit(['reset', '--hard', 'HEAD'])
       await this.execGit(['stash', 'apply', '--quiet', '--index', backupStash])
       debug('Done restoring original state!')
@@ -222,7 +217,7 @@ class GitWorkflow {
   async dropBackup(ctx) {
     try {
       debug('Dropping backup stash...')
-      const backupStash = await this.getBackupStash()
+      const backupStash = await this.getBackupStash(ctx)
       await this.execGit(['stash', 'drop', '--quiet', backupStash])
       debug('Done dropping backup stash!')
     } catch (error) {

lib/resolveTaskFn.js

@@ -38,8 +38,7 @@ function throwError(message) {
  * @returns {Error}
  */
 function makeErr(linter, result, context = {}) {
-  // Indicate that some linter will fail so we don't update the index with formatting changes
-  context.hasErrors = true // eslint-disable-line no-param-reassign
+  context.taskError = true
   const { stdout, stderr, killed, signal } = result
   if (killed || (signal && signal !== '')) {
     return throwError(

lib/runAll.js

@@ -106,6 +106,7 @@ module.exports = async function runAll(
             exitOnError: true
           }),
         skip: () => {
+          // Skip task when no files matched
           if (task.fileList.length === 0) {
             return `No staged files match ${task.pattern}`
           }
@@ -119,8 +120,10 @@ module.exports = async function runAll(
       title:
         chunkCount > 1 ? `Running tasks (chunk ${index + 1}/${chunkCount})...` : 'Running tasks...',
       task: () => new Listr(chunkListrTasks, { ...listrOptions, concurrent }),
-      skip: ctx => {
-        if (ctx && ctx.hasErrors) return 'Skipped because of previous git error'
+      skip: (ctx = {}) => {
+        // Skip if the first step (backup) failed
+        if (ctx.gitError) return 'Skipped because of previous git error.'
+        // Skip chunk when no every task is skipped (due to no matches)
         if (chunkListrTasks.every(task => task.skip())) return 'No tasks to run.'
         return false
       }
@@ -143,6 +146,12 @@ module.exports = async function runAll(
 
   const git = new GitWorkflow({ gitDir, stagedFileChunks })
 
+  // Running git reset or dropping the backup stash should be skipped
+  // when there are git errors NOT related to applying unstaged modifications.
+  // In the latter case, the original state is restored.
+  const cleanupNotSafe = ctx =>
+    ctx.gitError && !ctx.gitApplyModificationsError && 'Skipped because of previous git error.'
+
   const runner = new Listr(
     [
       {
@@ -152,29 +161,33 @@ module.exports = async function runAll(
       ...listrTasks,
       {
         title: 'Applying modifications...',
-        skip: ctx => ctx.hasErrors && 'Skipped because of errors from tasks',
+        skip: ctx => {
+          if (ctx.gitError) return 'Skipped because of previous git error.'
+          if (ctx.taskError) return 'Skipped because of errors from tasks.'
+        },
         task: ctx => git.applyModifications(ctx)
       },
       {
         title: 'Reverting to original state...',
-        enabled: ctx => ctx.hasErrors,
+        enabled: ctx => ctx.taskError || ctx.gitApplyModificationsError,
+        skip: cleanupNotSafe,
         task: ctx => git.restoreOriginalState(ctx)
       },
       {
         title: 'Cleaning up...',
-        skip: ctx =>
-          (ctx.hasGitLockError || ctx.hasGitMergeStateError) &&
-          'Skipped because of previous git error',
+        skip: cleanupNotSafe,
         task: ctx => git.dropBackup(ctx)
       }
     ],
     listrOptions
   )
 
   try {
-    await runner.run()
+    await runner.run({})
   } catch (error) {
-    if (error.context.hasGitLockError || error.context.hasGitMergeStateError) {
+    // Show help text about manual restore in case of git errors.
+    // No sense to show this if the backup stash itself is missing.
+    if (error.context.gitError && !error.context.gitGetBackupStashError) {
       logger.error(`
   ${symbols.error} ${chalk.red(`lint-staged failed due to a git error.
     Any lost modifications can be restored from a git stash:

test/__snapshots__/runAll.spec.js.snap

@@ -35,7 +35,7 @@ LOG →
 LOG Running tasks... [failed]
 LOG Applying modifications... [started]
 LOG Applying modifications... [skipped]
-LOG → Skipped because of errors from tasks
+LOG → Skipped because of errors from tasks.
 LOG Reverting to original state... [started]
 LOG Reverting to original state... [completed]
 LOG Cleaning up... [started]
@@ -45,10 +45,10 @@ LOG {
   errors: [
     {
       privateMsg: '\\\\n\\\\n\\\\n× echo \\"sample\\" found some errors. Please fix them and try committing again.\\\\n\\\\nLinter finished with error',
-      context: {hasErrors: true}
+      context: {taskError: true}
     }
   ],
-  context: {hasErrors: true}
+  context: {taskError: true}
 }"
 `;
 
@@ -66,7 +66,7 @@ LOG →
 LOG Running tasks... [failed]
 LOG Applying modifications... [started]
 LOG Applying modifications... [skipped]
-LOG → Skipped because of errors from tasks
+LOG → Skipped because of errors from tasks.
 LOG Reverting to original state... [started]
 LOG Reverting to original state... [completed]
 LOG Cleaning up... [started]
@@ -76,10 +76,10 @@ LOG {
   errors: [
     {
       privateMsg: '\\\\n\\\\n\\\\n‼ echo \\"sample\\" was terminated with SIGINT',
-      context: {hasErrors: true}
+      context: {taskError: true}
     }
   ],
-  context: {hasErrors: true}
+  context: {taskError: true}
 }"
 `;
 

test/resolveTaskFn.spec.js

@@ -175,15 +175,15 @@ describe('resolveTaskFn', () => {
     }
   })
 
-  it('should not set hasErrors on context if no error occur', async () => {
+  it('should not set taskError on context if no error occur', async () => {
     expect.assertions(1)
     const context = {}
     const taskFn = resolveTaskFn({ ...defaultOpts, command: 'jest', gitDir: '../' })
     await taskFn(context)
-    expect(context.hasErrors).toBeUndefined()
+    expect(context.taskError).toBeUndefined()
   })
 
-  it('should set hasErrors on context to true on error', async () => {
+  it('should set taskError on context to true on error', async () => {
     execa.mockResolvedValueOnce({
       stdout: 'Mock error',
       stderr: '',
@@ -197,7 +197,7 @@ describe('resolveTaskFn', () => {
     try {
       await taskFn(context)
     } catch (err) {
-      expect(context.hasErrors).toEqual(true)
+      expect(context.taskError).toEqual(true)
     }
   })
 })

test/runAll.unmocked.2.spec.js

@@ -111,16 +111,13 @@ describe('runAll', () => {
       LOG → Merge state could not be restored due to an error!
       LOG Running tasks... [started]
       LOG Running tasks... [skipped]
-      LOG → Skipped because of previous git error
+      LOG → Skipped because of previous git error.
       LOG Applying modifications... [started]
       LOG Applying modifications... [skipped]
-      LOG → Skipped because of errors from tasks
-      LOG Reverting to original state... [started]
-      LOG Reverting to original state... [failed]
-      LOG → Merge state could not be restored due to an error!
+      LOG → Skipped because of previous git error.
       LOG Cleaning up... [started]
       LOG Cleaning up... [skipped]
-      LOG → Skipped because of previous git error
+      LOG → Skipped because of previous git error.
       ERROR 
         × lint-staged failed due to a git error.
           Any lost modifications can be restored from a git stash: