Skip to content

Commit 68039e0

Browse files
authored
fix(cli): propagate proper exit status code when running yarnPath binary (#4798)
**What's the problem this PR addresses?** Fixes a scenario w/ Yarn CLI where a non-zero exit code is ignored (and `exitCode = 1` is used instead), when running a command using `yarn` binary specified by `yarnPath`. **How did you fix it?** Apparently [current implementation](https://github.com/yarnpkg/berry/blob/a66e528506168f7bfaa4fa9fc02b8edb226e47d4/packages/yarnpkg-cli/sources/main.ts#L107-L111) uses a `error.code` property, which does not exist on the `Error` that the underlying [`child_process.execFileSync()`](https://nodejs.org/api/child_process.html#child_processexecfilesyncfile-args-options) throws, when a child process exits w/ a non-zero exit status. The `error.status` should be used instead, as per Node.js API documentation: > If the process times out or has a non-zero exit code, this method will throw an [Error](https://nodejs.org/api/errors.html#class-error) that will include the full result of the underlying [child_process.spawnSync()](https://nodejs.org/api/child_process.html#child_processspawnsynccommand-args-options). ``` interface SpawnSyncReturns<T> { pid: number; output: Array<T | null>; stdout: T; stderr: T; status: number | null; signal: NodeJS.Signals | null; error?: Error | undefined; } ``` ... **Checklist** <!--- Don't worry if you miss something, chores are automatically tested. --> <!--- This checklist exists to help you remember doing the chores when you submit a PR. --> <!--- Put an `x` in all the boxes that apply. --> - [x] I have read the [Contributing Guide](https://yarnpkg.com/advanced/contributing). <!-- See https://yarnpkg.com/advanced/contributing#preparing-your-pr-to-be-released for more details. --> <!-- Check with `yarn version check` and fix with `yarn version check -i` --> - [x] I have set the packages that need to be released for my changes to be effective. <!-- The "Testing chores" workflow validates that your PR follows our guidelines. --> <!-- If it doesn't pass, click on it to see details as to what your PR might be missing. --> - [x] I will check that all automated PR checks pass before the PR gets reviewed.
1 parent bb0ad0d commit 68039e0

3 files changed

Lines changed: 46 additions & 2 deletions

File tree

.yarn/versions/e0fbb587.yml

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
releases:
2+
"@yarnpkg/cli": patch
3+
4+
declined:
5+
- "@yarnpkg/plugin-compat"
6+
- "@yarnpkg/plugin-constraints"
7+
- "@yarnpkg/plugin-dlx"
8+
- "@yarnpkg/plugin-essentials"
9+
- "@yarnpkg/plugin-init"
10+
- "@yarnpkg/plugin-interactive-tools"
11+
- "@yarnpkg/plugin-nm"
12+
- "@yarnpkg/plugin-npm-cli"
13+
- "@yarnpkg/plugin-pack"
14+
- "@yarnpkg/plugin-patch"
15+
- "@yarnpkg/plugin-pnp"
16+
- "@yarnpkg/plugin-pnpm"
17+
- "@yarnpkg/plugin-stage"
18+
- "@yarnpkg/plugin-typescript"
19+
- "@yarnpkg/plugin-version"
20+
- "@yarnpkg/plugin-workspace-tools"
21+
- "@yarnpkg/builder"
22+
- "@yarnpkg/core"
23+
- "@yarnpkg/doctor"

packages/acceptance-tests/pkg-tests-specs/sources/commands/run.test.js

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,5 @@
1+
import {ppath, xfs} from '@yarnpkg/fslib';
2+
13
describe(`Commands`, () => {
24
for (const [description, args] of [[`with prefix`, [`run`]], [`without prefix`, []]]) {
35
describe(`run ${description}`, () => {
@@ -24,6 +26,25 @@ describe(`Commands`, () => {
2426
code: 42,
2527
});
2628
}));
29+
30+
test(`it should properly forward the script exit codes when calling into another yarnPath binary`, makeTemporaryEnv({
31+
scripts: {
32+
foo: `exit 0`,
33+
},
34+
}, async ({path, run, source}) => {
35+
await xfs.writeFilePromise(ppath.join(path, `yarn-test-secondary-binary.js`), [
36+
`#!/usr/bin/env node`,
37+
`process.exit(42);`,
38+
].join(`\n`));
39+
40+
await run(`install`);
41+
42+
await expect(run(...args, `foo`, {
43+
yarnPath: `./yarn-test-secondary-binary.js`,
44+
})).rejects.toMatchObject({
45+
code: 42,
46+
});
47+
}));
2748
});
2849
}
2950

packages/yarnpkg-cli/sources/lib.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
import {Configuration, CommandContext, PluginConfiguration, TelemetryManager, semverUtils, miscUtils, YarnVersion} from '@yarnpkg/core';
22
import {PortablePath, npath, ppath, xfs} from '@yarnpkg/fslib';
3-
import {execFileSync} from 'child_process';
3+
import {execFileSync, type SpawnSyncReturns} from 'child_process';
44
import {isCI} from 'ci-info';
55
import {Cli, UsageError} from 'clipanion';
66

@@ -83,7 +83,7 @@ function runYarnPath(cli: YarnCli, argv: Array<string>, {yarnPath}: {yarnPath: P
8383
try {
8484
execFileSync(process.execPath, [npath.fromPortablePath(yarnPath), ...argv], yarnPathExecOptions);
8585
} catch (err) {
86-
return (err.code as number) ?? 1;
86+
return (err as SpawnSyncReturns<void>).status ?? 1;
8787
}
8888

8989
return 0;

0 commit comments

Comments
 (0)