Skip to content

fix: propagate SIGINT exit status on POSIX #586

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
lawrence3699 wants to merge 1 commit into
base: main
Choose a base branch
from
+15 −4
Open

fix: propagate SIGINT exit status on POSIX #586

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 4 additions & 2 deletions bin/index.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -185,7 +185,7 @@
expect(exit.code).toBeGreaterThan(0);
});

it('is of success when a SIGINT is sent', async () => {
it('propagates SIGINT on POSIX when interrupted', async () => {
// Windows doesn't support sending signals like on POSIX platforms.
// However, in a console, processes can be interrupted with CTRL+C (like a SIGINT).
// This is what we simulate here with the help of a wrapper application.
Expand All @@ -204,7 +204,9 @@
const lines = await child.getLogLines();
const exit = await child.exit;

expect(exit.code).toBe(0);
expect(exit).toMatchObject(
isWindows ? { code: 0, signal: null } : { code: null, signal: 'SIGINT' },
);
expect(lines).toContainEqual(
expect.stringMatching(
createKillMessage(
Expand Down Expand Up @@ -337,7 +339,7 @@

describe('--kill-others', () => {
describe('kills on success', () => {
it.each(['--kill-others', '-k'])('%s', async (arg) => {

Check failure on line 342 in bin/index.spec.ts

View workflow job for this annotation

GitHub Actions / Test (Node.js 24, Windows)

bin/index.spec.ts > --kill-others > kills on success > -k 

Error: Test timed out in 5000ms.
If this is a long-running test, pass a timeout value as the last argument or configure it globally with "testTimeout".
 ❯ bin/index.spec.ts:342:40
const lines = await run(
`${arg} "node __fixtures__/sleep.js 10" "exit 0"`,
).getLogLines();
Expand Down
13 changes: 11 additions & 2 deletions bin/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -225,6 +225,15 @@ assertDeprecated(
'Use commas as name separators instead.',
);

let interruptedBySigint = false;
if (process.platform !== 'win32') {
// On POSIX, exit with SIGINT after children finish so shell callers don't treat Ctrl+C as
// a successful run.
process.once('SIGINT', () => {
interruptedBySigint = true;
});
}

// Get names of commands by the specified separator
const names = (args.names || '').split(args.nameSeparator);

Expand Down Expand Up @@ -269,6 +278,6 @@ concurrently(
additionalArguments: args.passthroughArguments ? additionalArguments : undefined,
},
).result.then(
() => process.exit(0),
() => process.exit(1),
() => (interruptedBySigint ? process.kill(process.pid, 'SIGINT') : process.exit(0)),
() => (interruptedBySigint ? process.kill(process.pid, 'SIGINT') : process.exit(1)),
);
Loading