From 30b480925a313f5c2b614eb40eb1a340a6cefae5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Iiro=20J=C3=A4ppinen?= Date: Tue, 24 Dec 2019 11:41:21 +0200 Subject: [PATCH] fix: error handling skips dropping backup stash after internal git errors --- lib/gitWorkflow.js | 35 +++++++++++--------------- lib/resolveTaskFn.js | 3 +-- lib/runAll.js | 31 ++++++++++++++++------- test/__snapshots__/runAll.spec.js.snap | 12 ++++----- test/resolveTaskFn.spec.js | 8 +++--- test/runAll.unmocked.2.spec.js | 9 +++---- 6 files changed, 51 insertions(+), 47 deletions(-) diff --git a/lib/gitWorkflow.js b/lib/gitWorkflow.js index 0c34391fa..f1d192af3 100644 --- a/lib/gitWorkflow.js +++ b/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) { diff --git a/lib/resolveTaskFn.js b/lib/resolveTaskFn.js index 39d999629..44c23db89 100644 --- a/lib/resolveTaskFn.js +++ b/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( diff --git a/lib/runAll.js b/lib/runAll.js index 58c5b570b..f803441b1 100644 --- a/lib/runAll.js +++ b/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,19 +161,21 @@ 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) } ], @@ -172,9 +183,11 @@ module.exports = async function runAll( ) 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: diff --git a/test/__snapshots__/runAll.spec.js.snap b/test/__snapshots__/runAll.spec.js.snap index 3ce7f0e71..29ac4200d 100644 --- a/test/__snapshots__/runAll.spec.js.snap +++ b/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} }" `; diff --git a/test/resolveTaskFn.spec.js b/test/resolveTaskFn.spec.js index 56bab7270..bcfefc7dc 100644 --- a/test/resolveTaskFn.spec.js +++ b/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) } }) }) diff --git a/test/runAll.unmocked.2.spec.js b/test/runAll.unmocked.2.spec.js index 1bfcaf902..47b7805a0 100644 --- a/test/runAll.unmocked.2.spec.js +++ b/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: