Skip to content

Commit

Permalink
fix: kill other running tasks on failure
Browse files Browse the repository at this point in the history
  • Loading branch information
s-h-a-d-o-w committed Mar 13, 2022
1 parent 531275c commit 66f8c04
Show file tree
Hide file tree
Showing 8 changed files with 103 additions and 16 deletions.
29 changes: 26 additions & 3 deletions lib/resolveTaskFn.js
Expand Up @@ -2,14 +2,17 @@ import { redBright, dim } from 'colorette'
import execa from 'execa'
import debug from 'debug'
import { parseArgsStringToArgv } from 'string-argv'
import pidTree from 'pidtree'

import { error, info } from './figures.js'
import { getInitialState } from './state.js'
import { TaskError } from './symbols.js'

export const FAIL_CHECK_INTERVAL = 200

const debugLog = debug('lint-staged:resolveTaskFn')

const getTag = ({ code, killed, signal }) => signal || (killed && 'KILLED') || code || 'FAILED'
const getTag = ({ code, killed, signal }) => (killed && 'KILLED') || signal || code || 'FAILED'

/**
* Handle task console output.
Expand Down Expand Up @@ -101,9 +104,29 @@ export const resolveTaskFn = ({
debugLog('execaOptions:', execaOptions)

return async (ctx = getInitialState()) => {
const result = await (shell
let isExecutionDone = false
const execaChildProcess = shell
? execa.command(isFn ? command : `${command} ${files.join(' ')}`, execaOptions)
: execa(cmd, isFn ? args : args.concat(files), execaOptions))
: execa(cmd, isFn ? args : args.concat(files), execaOptions)

const interruptExecutionOnError = async () => {
if (!isExecutionDone) {
if (ctx.errors.size > 0) {
const ids = await pidTree(execaChildProcess.pid)
ids.forEach(process.kill)

// The execa process is killed separately in order
// to get the `KILLED` status.
execaChildProcess.kill()
} else {
setTimeout(interruptExecutionOnError, FAIL_CHECK_INTERVAL)
}
}
}
interruptExecutionOnError()

const result = await execaChildProcess
isExecutionDone = true

if (result.failed || result.killed || result.signal != null) {
throw makeErr(command, result, ctx)
Expand Down
17 changes: 17 additions & 0 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions package.json
Expand Up @@ -42,6 +42,7 @@
"micromatch": "^4.0.4",
"normalize-path": "^3.0.0",
"object-inspect": "^1.12.0",
"pidtree": "^0.5.0",
"string-argv": "^0.3.1",
"supports-color": "^9.2.1",
"yaml": "^1.10.2"
Expand Down
4 changes: 3 additions & 1 deletion test/__mocks__/execa.js
@@ -1,5 +1,7 @@
import { createExecaReturnValue } from '../utils/createExecaReturnValue'

const execa = jest.fn(() =>
Promise.resolve({
createExecaReturnValue({
stdout: 'a-ok',
stderr: '',
code: 0,
Expand Down
22 changes: 14 additions & 8 deletions test/resolveTaskFn.spec.js
Expand Up @@ -4,8 +4,14 @@ import { resolveTaskFn } from '../lib/resolveTaskFn'
import { getInitialState } from '../lib/state'
import { TaskError } from '../lib/symbols'

import { createExecaReturnValue } from './utils/createExecaReturnValue'

const defaultOpts = { files: ['test.js'] }

function mockExecaImplementationOnce(value) {
execa.mockImplementationOnce(() => createExecaReturnValue(value))
}

describe('resolveTaskFn', () => {
beforeEach(() => {
execa.mockClear()
Expand Down Expand Up @@ -135,7 +141,7 @@ describe('resolveTaskFn', () => {

it('should throw error for failed linters', async () => {
expect.assertions(1)
execa.mockResolvedValueOnce({
mockExecaImplementationOnce({
stdout: 'Mock error',
stderr: '',
code: 0,
Expand All @@ -149,7 +155,7 @@ describe('resolveTaskFn', () => {

it('should throw error for interrupted processes', async () => {
expect.assertions(1)
execa.mockResolvedValueOnce({
mockExecaImplementationOnce({
stdout: 'Mock error',
stderr: '',
code: 0,
Expand All @@ -167,7 +173,7 @@ describe('resolveTaskFn', () => {

it('should throw error for killed processes without signal', async () => {
expect.assertions(1)
execa.mockResolvedValueOnce({
mockExecaImplementationOnce({
stdout: 'Mock error',
stderr: '',
code: 0,
Expand All @@ -192,7 +198,7 @@ describe('resolveTaskFn', () => {
})

it('should add TaskError on error', async () => {
execa.mockResolvedValueOnce({
mockExecaImplementationOnce({
stdout: 'Mock error',
stderr: '',
code: 0,
Expand All @@ -210,7 +216,7 @@ describe('resolveTaskFn', () => {

it('should not add output when there is none', async () => {
expect.assertions(2)
execa.mockResolvedValueOnce({
mockExecaImplementationOnce({
stdout: '',
stderr: '',
code: 0,
Expand All @@ -236,7 +242,7 @@ describe('resolveTaskFn', () => {

it('should add output even when task succeeds if `verbose: true`', async () => {
expect.assertions(2)
execa.mockResolvedValueOnce({
mockExecaImplementationOnce({
stdout: 'Mock success',
stderr: '',
code: 0,
Expand Down Expand Up @@ -266,7 +272,7 @@ describe('resolveTaskFn', () => {

it('should not add title to output when task errors while quiet', async () => {
expect.assertions(2)
execa.mockResolvedValueOnce({
mockExecaImplementationOnce({
stdout: '',
stderr: 'stderr',
code: 1,
Expand Down Expand Up @@ -296,7 +302,7 @@ describe('resolveTaskFn', () => {

it('should not print anything when task errors without output while quiet', async () => {
expect.assertions(2)
execa.mockResolvedValueOnce({
mockExecaImplementationOnce({
stdout: '',
stderr: '',
code: 1,
Expand Down
24 changes: 24 additions & 0 deletions test/resolveTaskFn.unmocked.spec.js
@@ -1,4 +1,5 @@
import { resolveTaskFn } from '../lib/resolveTaskFn'
import { getInitialState } from '../lib/state'

jest.unmock('execa')

Expand All @@ -13,4 +14,27 @@ describe('resolveTaskFn', () => {

await expect(taskFn()).resolves.toMatchInlineSnapshot(`undefined`)
})

it('should kill a long running task when another fails', async () => {
const context = getInitialState()

const taskFn = resolveTaskFn({
command: 'node -e "while(true) {}"',
isFn: true,
})
const taskPromise = taskFn(context)

const taskFn2 = resolveTaskFn({
command: 'node -e "process.exit(1)"',
isFn: true,
})
const task2Promise = taskFn2(context)

await expect(task2Promise).rejects.toThrowErrorMatchingInlineSnapshot(
`"node -e \\"process.exit(1)\\" [FAILED]"`
)
await expect(taskPromise).rejects.toThrowErrorMatchingInlineSnapshot(
`"node -e \\"while(true) {}\\" [KILLED]"`
)
})
})
10 changes: 6 additions & 4 deletions test/runAll.spec.js
Expand Up @@ -12,6 +12,8 @@ import { ConfigNotFoundError, GitError } from '../lib/symbols'
import * as searchConfigsNS from '../lib/searchConfigs'
import * as getConfigGroupsNS from '../lib/getConfigGroups'

import { createExecaReturnValue } from './utils/createExecaReturnValue'

jest.mock('../lib/file')
jest.mock('../lib/getStagedFiles')
jest.mock('../lib/gitWorkflow')
Expand Down Expand Up @@ -192,7 +194,7 @@ describe('runAll', () => {
expect.assertions(2)
getStagedFiles.mockImplementationOnce(async () => ['sample.js'])
execa.mockImplementation(() =>
Promise.resolve({
createExecaReturnValue({
stdout: '',
stderr: 'Linter finished with error',
code: 1,
Expand Down Expand Up @@ -229,7 +231,7 @@ describe('runAll', () => {
expect.assertions(2)
getStagedFiles.mockImplementationOnce(async () => ['sample.js'])
execa.mockImplementation(() =>
Promise.resolve({
createExecaReturnValue({
stdout: '',
stderr: '',
code: 0,
Expand All @@ -252,8 +254,8 @@ describe('runAll', () => {
LOG [STARTED] — 1 file
LOG [STARTED] *.js — 1 file
LOG [STARTED] echo \\"sample\\"
ERROR [FAILED] echo \\"sample\\" [SIGINT]
ERROR [FAILED] echo \\"sample\\" [SIGINT]
ERROR [FAILED] echo \\"sample\\" [KILLED]
ERROR [FAILED] echo \\"sample\\" [KILLED]
LOG [SUCCESS] Running tasks for staged files...
LOG [STARTED] Applying modifications from tasks...
INFO [SKIPPED] Skipped because of errors from tasks.
Expand Down
12 changes: 12 additions & 0 deletions test/utils/createExecaReturnValue.js
@@ -0,0 +1,12 @@
export function createExecaReturnValue(value, executionTime) {
const returnValue = { ...value }
const returnedPromise = executionTime
? new Promise((resolve) => setTimeout(() => resolve(returnValue), executionTime))
: Promise.resolve(returnValue)

returnedPromise.kill = () => {
returnValue.killed = true
}

return returnedPromise
}

0 comments on commit 66f8c04

Please sign in to comment.