Skip to content

Commit e8f704e

Browse files
committed
Refactor: Move abort signal handling into streamProcessToLogger
Rather than having callers manually set up abort handlers and cleanup, streamProcessToLogger now accepts an optional abortSignal and returns a cleanup function. This centralizes the abort handling logic and makes call sites cleaner. Benefits: - Reduces boilerplate at call sites - Ensures consistent cleanup (no risk of forgetting removeEventListener) - Single place to maintain abort handling logic Addresses code review feedback.
1 parent 7855eaa commit e8f704e

File tree

2 files changed

+26
-12
lines changed

2 files changed

+26
-12
lines changed

src/runtime/SSHRuntime.ts

Lines changed: 4 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -563,16 +563,10 @@ export class SSHRuntime implements Runtime {
563563
log.debug(`Creating bundle: ${command}`);
564564
const proc = spawn("bash", ["-c", command]);
565565

566-
// Handle abort signal
567-
const abortHandler = () => {
568-
proc.kill();
569-
reject(new Error("Bundle creation aborted"));
570-
};
571-
abortSignal?.addEventListener("abort", abortHandler);
572-
573-
streamProcessToLogger(proc, initLogger, {
566+
const cleanup = streamProcessToLogger(proc, initLogger, {
574567
logStdout: false,
575568
logStderr: true,
569+
abortSignal,
576570
});
577571

578572
let stderr = "";
@@ -581,7 +575,7 @@ export class SSHRuntime implements Runtime {
581575
});
582576

583577
proc.on("close", (code) => {
584-
abortSignal?.removeEventListener("abort", abortHandler);
578+
cleanup();
585579
if (abortSignal?.aborted) {
586580
reject(new Error("Bundle creation aborted"));
587581
} else if (code === 0) {
@@ -592,7 +586,7 @@ export class SSHRuntime implements Runtime {
592586
});
593587

594588
proc.on("error", (err) => {
595-
abortSignal?.removeEventListener("abort", abortHandler);
589+
cleanup();
596590
reject(err);
597591
});
598592
});

src/runtime/streamProcess.ts

Lines changed: 22 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ import type { InitLogger } from "./Runtime";
1616
* @param process Child process to stream from
1717
* @param initLogger Logger to stream output to
1818
* @param options Configuration for which streams to log
19+
* @returns Cleanup function to remove abort signal listener (call this in process close/error handlers)
1920
*/
2021
export function streamProcessToLogger(
2122
process: ChildProcess,
@@ -27,15 +28,27 @@ export function streamProcessToLogger(
2728
logStderr?: boolean;
2829
/** Optional: Command string to log before streaming starts */
2930
command?: string;
31+
/** Optional: Abort signal to kill process on cancellation */
32+
abortSignal?: AbortSignal;
3033
}
31-
): void {
32-
const { logStdout = false, logStderr = true, command } = options ?? {};
34+
): () => void {
35+
const { logStdout = false, logStderr = true, command, abortSignal } = options ?? {};
3336

3437
// Log the command being executed (if provided)
3538
if (command) {
3639
initLogger.logStep(`Executing: ${command}`);
3740
}
3841

42+
// Set up abort signal handler
43+
const abortHandler = abortSignal
44+
? () => {
45+
process.kill();
46+
}
47+
: null;
48+
if (abortHandler && abortSignal) {
49+
abortSignal.addEventListener("abort", abortHandler);
50+
}
51+
3952
// Drain stdout (prevent pipe overflow)
4053
if (process.stdout) {
4154
process.stdout.on("data", (data: Buffer) => {
@@ -65,4 +78,11 @@ export function streamProcessToLogger(
6578
// Otherwise drain silently to prevent buffer overflow
6679
});
6780
}
81+
82+
// Return cleanup function to remove abort listener
83+
return () => {
84+
if (abortHandler && abortSignal) {
85+
abortSignal.removeEventListener("abort", abortHandler);
86+
}
87+
};
6888
}

0 commit comments

Comments
 (0)