Skip to content

Commit

Permalink
fix(error): when an error occurs, stop processes
Browse files Browse the repository at this point in the history
This is a significant refactor to make it so when an error occurs in one
parallel script, the others are killed and also to ensure that an error
in one serial script will result in the others not being executed.

Closes #48
  • Loading branch information
Kent C. Dodds committed Sep 4, 2016
1 parent 00f9cba commit c911c67
Show file tree
Hide file tree
Showing 6 changed files with 319 additions and 81 deletions.
18 changes: 18 additions & 0 deletions other/ERRORS_AND_WARNINGS.md
Original file line number Diff line number Diff line change
Expand Up @@ -27,3 +27,21 @@ This happens when you use the `--require` flag and the module you specify cannot

1. Check that you spelled the module correctly
2. Check that the module you wish to require is require-able

## Failed with exit code

This means that one of the scripts p-s tried to run resulted in a non-zero exit code (a failing exit code)

### To Fix:

Try to run the script without `p-s` and verify that the script is working. If not, fix that. If it's working without `p-s` it could be a problem with `p-s`. Please file an issue.

## Emitted an error

This means that the child process for the specified script emitted an error.

### To Fix:

Look at the error and try to figure out why the script would be failing.

Try to run the script without `p-s` and verify that the script is working. If not, fix that. If it's working without `p-s` it could be a problem with `p-s`. Please file an issue.
4 changes: 2 additions & 2 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@
"license": "MIT",
"dependencies": {
"arrify": "1.0.1",
"async": "2.0.1",
"bluebird": "3.4.6",
"colors": "1.1.2",
"commander": "2.9.0",
"find-up": "1.1.2",
Expand All @@ -30,7 +30,7 @@
"omelette": "0.3.1",
"prefix-matches": "0.0.9",
"shell-escape": "0.2.0",
"spawn-command": "0.0.2"
"spawn-command-with-kill": "1.0.0"
},
"devDependencies": {
"all-contributors-cli": "3.0.6",
Expand Down
9 changes: 3 additions & 6 deletions src/bin/p-s.js
Original file line number Diff line number Diff line change
Expand Up @@ -60,12 +60,9 @@ function loadAndRun(scriptsAndArgs, psConfig) {
parallel: scriptsAndArgs.parallel,
logLevel: program.logLevel,
}),
}, result => {
if (result.error) {
log.error(result.error)
process.exit(FAIL_CODE)
}
process.exit(result.code)
}).catch(error => {
log.error(error)
process.exitCode = error.code || FAIL_CODE
})
}

Expand Down
7 changes: 6 additions & 1 deletion src/get-logger.js
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,12 @@ function getLogger(logLevel) {

function getMessage(first, ...rest) {
if (isPlainObject(first) && first.message && first.ref) {
return [...arrify(first.message), getLink(first.ref), ...rest]
return [
...arrify(first.message),
getLink(first.ref),
first.error,
...rest,
].filter(i => !!i)
} else {
return [first, ...rest]
}
Expand Down
116 changes: 92 additions & 24 deletions src/index.js
Original file line number Diff line number Diff line change
@@ -1,57 +1,125 @@
import spawn from 'spawn-command'
import async from 'async'
import spawn from 'spawn-command-with-kill'
import Promise from 'bluebird'
import colors from 'colors/safe'
import {isString, find, clone} from 'lodash'
import {isString, clone} from 'lodash'
import {sync as findUpSync} from 'find-up'
import managePath from 'manage-path'
import arrify from 'arrify'
import getScriptToRun from './get-script-to-run'
import getScriptsFromConfig from './get-scripts-from-config'
import getLogger from './get-logger'

const noop = () => {} // eslint-disable-line func-style, no-empty-function
const NON_ERROR = 0

export default runPackageScripts

function runPackageScripts({scriptConfig, scripts, args, options = {}}, callback = noop) {
function runPackageScripts({scriptConfig, scripts, args, options = {}}) {
if (scripts.length === 0) {
scripts = ['default']
}
const scriptNames = arrify(scripts)
const method = options.parallel ? 'map' : 'mapSeries'
async[method](scriptNames, (scriptName, cb) => {
const child = runPackageScript({scriptConfig, options, scriptName, args})
if (child.on) {
child.on('exit', exitCode => cb(null, exitCode))
} else {
cb(child)
}
}, (err, results) => {
if (err) {
callback({error: err})
} else {
const NON_ERROR = 0
const result = find(results, r => r !== NON_ERROR)
callback({code: result})
if (options.parallel) {
return runParallel()
} else {
return runSeries()
}

function runSeries() {
return scriptNames.reduce((res, scriptName) => {
return res.then(() => (
runPackageScript({scriptConfig, options, scriptName, args})
))
}, Promise.resolve())
}

function runParallel() {
const results = scriptNames.map(script => ({script, code: undefined}))
let aborted = false

const promises = scriptNames.map(scriptName => {
return runPackageScript({scriptConfig, options, scriptName, args})
})

const allPromise = Promise.all(promises.map((promise, index) => {
return promise.then(code => {
if (!aborted) {
results[index].code = code
}
})
})).then(() => results)

allPromise.catch(() => {
/* istanbul ignore if */
if (aborted) {
// this is very unlikely to happen
} else {
abortAll()
}
})

return allPromise

function abortAll() {
aborted = true
promises.forEach(p => p.abort())
}
})
}
}


function runPackageScript({scriptConfig, options, scriptName, args}) {
const scripts = getScriptsFromConfig(scriptConfig, scriptName)
const script = getScriptToRun(scripts, scriptName)
if (!isString(script)) {
return {
return Promise.reject({
message: colors.red(
`Scripts must resolve to strings. There is no script that can be resolved from "${scriptName}"`
),
ref: 'missing-script',
}
})
}
const command = [script, args].join(' ').trim()
const log = getLogger(getLogLevel(options))
log.info(colors.gray('p-s executing: ') + colors.green(command))
return spawn(command, {stdio: 'inherit', env: getEnv()})
let child
const promise = new Promise((resolve, reject) => {
child = spawn(command, {stdio: 'inherit', env: getEnv()})

child.on('error', error => {
child = null
reject({
message: colors.red(
`The script called "${scriptName}" which runs "${command}" emitted an error`
),
ref: 'emitted-an-error',
error,
})
})

child.on('close', code => {
child = null
if (code === NON_ERROR) {
resolve(code)
} else {
reject({
message: colors.red(
`The script called "${scriptName}" which runs "${command}" failed with exit code ${code}`
),
ref: 'failed-with-exit-code',
code,
})
}
})
})

promise.abort = function abort() {
if (child !== null) {
child.kill()
child = null
}
}

return promise
}

function getLogLevel({silent, logLevel}) {
Expand Down
Loading

0 comments on commit c911c67

Please sign in to comment.