Skip to content

Commit

Permalink
fix: surface command exit code on kill (#170)
Browse files Browse the repository at this point in the history
  • Loading branch information
dsherret authored Aug 5, 2023
1 parent b76463d commit c347674
Show file tree
Hide file tree
Showing 4 changed files with 48 additions and 25 deletions.
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -225,7 +225,7 @@ await $`echo ${text}`; // will output `> echo example` before running the comman

### Timeout a command

This will exit with code 124.
This will exit with code 124 after 1 second.

```ts
// timeout a command after a specified time
Expand Down
37 changes: 27 additions & 10 deletions mod.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1534,8 +1534,8 @@ Deno.test("signal listening in registered commands", async () => {
function listener(signal: Deno.Signal) {
if (signal === "SIGKILL") {
resolve({
kind: "exit",
code: 1,
kind: "continue",
code: 135,
});
handler.signal.removeListener(listener);
} else {
Expand All @@ -1548,15 +1548,32 @@ Deno.test("signal listening in registered commands", async () => {
});
const $ = build$({ commandBuilder });

const child = $`listen`.stderr("piped").spawn();
await $.sleep(5); // let the command start up
child.kill("SIGINT");
child.kill("SIGBREAK");
child.kill("SIGKILL");
{
const child = $`listen`.stderr("piped").spawn();
await $.sleep(5); // let the command start up
child.kill("SIGINT");
child.kill("SIGBREAK");
child.kill("SIGKILL");

const result = await child;
assertEquals(result.code, 135);
assertEquals(result.stderr, "SIGINT\nSIGBREAK\n");
}

const result = await child;
assertEquals(result.code, 1);
assertEquals(result.stderr, "SIGINT\nSIGBREAK\n");
{
// now try killing while running and having a command launch afterwards on failure
const child = $`listen || echo 1`.stderr("piped").spawn();
await $.sleep(5); // let the command start up
child.kill("SIGINT");
child.kill("SIGBREAK");
child.kill("SIGKILL");

const result = await child;
// exit code should be the abort code in this case because it tried
// to launch `echo 1` after the command was killed
assertEquals(result.code, 124);
assertEquals(result.stderr, "SIGINT\nSIGBREAK\n");
}
});

Deno.test("should support setting a command signal", async () => {
Expand Down
32 changes: 20 additions & 12 deletions src/command.ts
Original file line number Diff line number Diff line change
Expand Up @@ -495,8 +495,8 @@ export class CommandChild extends Promise<CommandResult> {

/** Send a signal to the executing command's child process. Note that SIGTERM,
* SIGKILL, SIGABRT, SIGQUIT, SIGINT, or SIGSTOP will cause the entire command
* to be considered "aborted" and will return a 124 exit code, while other signals
* will just be forwarded to the command.
* to be considered "aborted" and if part of a command runs after this has occurred
* it will return a 124 exit code. Other signals will just be forwarded to the command.
*
* Defaults to "SIGTERM".
*/
Expand Down Expand Up @@ -601,7 +601,7 @@ export function parseAndSpawnCommand(state: CommandBuilderState) {
try {
const list = parseCommand(command);
const stdin = takeStdin();
const code = await spawn(list, {
let code = await spawn(list, {
stdin: stdin instanceof ReadableStream ? readerFromStreamReader(stdin.getReader()) : stdin,
stdout,
stderr,
Expand All @@ -611,16 +611,24 @@ export function parseAndSpawnCommand(state: CommandBuilderState) {
exportEnv: state.exportEnv,
signal,
});
if (code !== 0 && !state.noThrow) {
if (stdin instanceof ReadableStream) {
if (!stdin.locked) {
stdin.cancel();
}
if (code !== 0) {
if (timedOut) {
// override the code in the case of a timeout that resulted in a failure
code = 124;
}
if (signal.aborted) {
throw new Error(`${timedOut ? "Timed out" : "Aborted"} with exit code: ${code}`);
} else {
throw new Error(`Exited with code: ${code}`);
if (!state.noThrow) {
if (stdin instanceof ReadableStream) {
if (!stdin.locked) {
stdin.cancel();
}
}
if (timedOut) {
throw new Error(`Timed out with exit code: ${code}`);
} else if (signal.aborted) {
throw new Error(`${timedOut ? "Timed out" : "Aborted"} with exit code: ${code}`);
} else {
throw new Error(`Exited with code: ${code}`);
}
}
}
resolve(
Expand Down
2 changes: 0 additions & 2 deletions src/shell.ts
Original file line number Diff line number Diff line change
Expand Up @@ -638,8 +638,6 @@ async function executeCommandArgs(commandArgs: string[], context: Context): Prom
code: 1,
kind: "exit",
};
} else if (context.signal.aborted) {
return getAbortedResult();
} else {
return resultFromCode(status.code);
}
Expand Down

0 comments on commit c347674

Please sign in to comment.