Skip to content

Commit 179ebc4

Browse files
antitoxicantitoxic
authored
fix(run): make non-recursive --no-bail exit non-zero when a script fails (#12263)
When running a non-recursive `pnpm run --no-bail` that matches multiple scripts (e.g. via a `/regex/` selector), pnpm always exited with code `0` regardless of whether any script failed. This is inconsistent with recursive runs, which aggregate failures and exit non-zero at the end (via `throwOnCommandFail`). This PR fixes `--no-bail` directly so its exit-code behavior is consistent across recursive and non-recursive runs, as requested in #8013: - `--no-bail` still runs every matched script to completion (it no longer short-circuits on the first failure — execution switched from `Promise.all` to `Promise.allSettled`). - After all scripts settle, the command exits with a non-zero exit code (`ERR_PNPM_RUN_FAILED`) if any of them failed. This is a behavior change: previously a non-recursive `pnpm run --no-bail` with a failing script exited `0`. No new flag is introduced — per the issue discussion, a separate flag "would just add confusion without benefit". Closes #8013
1 parent e85aea2 commit 179ebc4

3 files changed

Lines changed: 98 additions & 17 deletions

File tree

‎.changeset/no-bail-exit-code.md‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
---
2+
"@pnpm/exec.commands": minor
3+
"pnpm": minor
4+
---
5+
6+
`pnpm run --no-bail` now exits with a non-zero exit code when any of the executed scripts fail, while still running every matched script to completion. This makes the exit-code behavior of `--no-bail` consistent between recursive and non-recursive runs (recursive runs already failed at the end). Previously, a non-recursive `pnpm run --no-bail` always exited with code 0, even when a script failed [#8013](https://github.com/pnpm/pnpm/issues/8013).

‎exec/commands/src/run.ts‎

Lines changed: 26 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -137,7 +137,7 @@ For options that may be used with `-r`, see "pnpm help recursive"',
137137
shortAlias: '-r',
138138
},
139139
{
140-
description: 'The command will exit with a 0 exit code even if the script fails',
140+
description: 'Continue running the remaining scripts even if one of them fails, instead of aborting on the first failure. The command still exits with a non-zero exit code if any script failed',
141141
name: '--no-bail',
142142
},
143143
IF_PRESENT_OPTION_HELP,
@@ -292,20 +292,34 @@ so you may run "pnpm -w run ${scriptName}"`,
292292
...makeNodeRequireOption(pnpPath),
293293
}
294294
}
295-
try {
296-
const limitRun = pLimit(concurrency)
295+
const limitRun = pLimit(concurrency)
297296

298-
const runScriptOptions: RunScriptOptions = {
299-
enablePrePostScripts: opts.enablePrePostScripts ?? false,
300-
syncInjectedDepsAfterScripts: opts.syncInjectedDepsAfterScripts,
301-
workspaceDir: opts.workspaceDir,
302-
}
303-
const _runScript = runScript.bind(null, { manifest, lifecycleOpts, runScriptOptions, passedThruArgs })
297+
const runScriptOptions: RunScriptOptions = {
298+
enablePrePostScripts: opts.enablePrePostScripts ?? false,
299+
syncInjectedDepsAfterScripts: opts.syncInjectedDepsAfterScripts,
300+
workspaceDir: opts.workspaceDir,
301+
}
302+
const _runScript = runScript.bind(null, { manifest, lifecycleOpts, runScriptOptions, passedThruArgs })
304303

304+
if (opts.bail !== false) {
305305
await Promise.all(specifiedScripts.map(script => limitRun(() => _runScript(script))))
306-
} catch (err: unknown) {
307-
if (opts.bail !== false) {
308-
throw err
306+
} else {
307+
const results = await Promise.allSettled(
308+
specifiedScripts.map(script => limitRun(() => _runScript(script)))
309+
)
310+
const failures = results
311+
.map((result, index) => ({ result, script: specifiedScripts[index] }))
312+
.filter((entry): entry is { result: PromiseRejectedResult, script: string } => entry.result.status === 'rejected')
313+
if (failures.length > 0) {
314+
throw new PnpmError(
315+
'RUN_FAILED',
316+
`Some scripts failed: ${failures.length} of ${specifiedScripts.length}`,
317+
{
318+
hint: failures
319+
.map(({ script, result }) => `${script}: ${result.reason?.message ?? String(result.reason)}`)
320+
.join('\n'),
321+
}
322+
)
309323
}
310324
}
311325
return undefined

‎exec/commands/test/index.ts‎

Lines changed: 66 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
/// <reference path="../../../__typings__/index.d.ts" />
22
import fs from 'node:fs'
33
import path from 'node:path'
4+
import util from 'node:util'
45

56
import { expect, test } from '@jest/globals'
67
import type { PnpmError } from '@pnpm/error'
@@ -56,7 +57,7 @@ test('pnpm run: returns correct exit code', async () => {
5657
expect(err.errno).toBe(1)
5758
})
5859

59-
test('pnpm run --no-bail never fails', async () => {
60+
test('pnpm run --no-bail runs the script to completion but still exits non-zero on failure', async () => {
6061
prepare({
6162
scripts: {
6263
exit1: 'node recordArgs && exit 1',
@@ -65,6 +66,69 @@ test('pnpm run --no-bail never fails', async () => {
6566
fs.writeFileSync('args.json', '[]', 'utf8')
6667
fs.writeFileSync('recordArgs.js', RECORD_ARGS_FILE, 'utf8')
6768

69+
let err: unknown
70+
try {
71+
await run.handler({
72+
...DEFAULT_OPTS,
73+
bin: 'node_modules/.bin',
74+
bail: false,
75+
dir: process.cwd(),
76+
extraBinPaths: [],
77+
extraEnv: {},
78+
pnpmHomeDir: '',
79+
}, ['exit1'])
80+
} catch (_err: unknown) {
81+
err = _err
82+
}
83+
84+
expect(util.types.isNativeError(err)).toBe(true)
85+
expect((err as PnpmError).code).toBe('ERR_PNPM_RUN_FAILED')
86+
87+
const { default: args } = await import(path.resolve('args.json'))
88+
expect(args).toStrictEqual([[]])
89+
})
90+
91+
test('pnpm run with regex and --no-bail runs every matched script but exits non-zero when one fails', async () => {
92+
prepare({
93+
scripts: {
94+
'lint:a': 'node -e "require(\'fs\').writeFileSync(\'lint-a.txt\', \'a\')"',
95+
'lint:b': 'node -e "require(\'fs\').writeFileSync(\'lint-b.txt\', \'b\'); process.exit(1)"',
96+
'lint:c': 'node -e "require(\'fs\').writeFileSync(\'lint-c.txt\', \'c\')"',
97+
},
98+
})
99+
100+
let err: unknown
101+
try {
102+
await run.handler({
103+
...DEFAULT_OPTS,
104+
bin: 'node_modules/.bin',
105+
bail: false,
106+
dir: process.cwd(),
107+
extraBinPaths: [],
108+
extraEnv: {},
109+
pnpmHomeDir: '',
110+
}, ['/^lint:/'])
111+
} catch (_err: unknown) {
112+
err = _err
113+
}
114+
115+
expect(util.types.isNativeError(err)).toBe(true)
116+
expect((err as PnpmError).code).toBe('ERR_PNPM_RUN_FAILED')
117+
118+
// Every matched script ran to completion, even though lint:b failed.
119+
expect(fs.readFileSync('lint-a.txt', 'utf8')).toBe('a')
120+
expect(fs.readFileSync('lint-b.txt', 'utf8')).toBe('b')
121+
expect(fs.readFileSync('lint-c.txt', 'utf8')).toBe('c')
122+
})
123+
124+
test('pnpm run with regex and --no-bail exits zero when all matched scripts pass', async () => {
125+
prepare({
126+
scripts: {
127+
'lint:a': 'node -e "process.exit(0)"',
128+
'lint:b': 'node -e "process.exit(0)"',
129+
},
130+
})
131+
68132
await run.handler({
69133
...DEFAULT_OPTS,
70134
bin: 'node_modules/.bin',
@@ -73,10 +137,7 @@ test('pnpm run --no-bail never fails', async () => {
73137
extraBinPaths: [],
74138
extraEnv: {},
75139
pnpmHomeDir: '',
76-
}, ['exit1'])
77-
78-
const { default: args } = await import(path.resolve('args.json'))
79-
expect(args).toStrictEqual([[]])
140+
}, ['/^lint:/'])
80141
})
81142

82143
const RECORD_ARGS_FILE = 'require(\'fs\').writeFileSync(\'args.json\', JSON.stringify(require(\'./args.json\').concat([process.argv.slice(2)])), \'utf8\')'

0 commit comments

Comments
 (0)