mirror of
https://github.com/samsonjs/Peekaboo.git
synced 2026-04-27 15:07:41 +00:00
Add timeout handling to prevent test hangs
- Add configurable timeout to executeSwiftCli (default 30s) - Add timeout support to execPeekaboo (default 15s) - Support PEEKABOO_CLI_TIMEOUT environment variable - Graceful process termination with SIGTERM then SIGKILL - Skip E2E tests in CI environments and non-macOS platforms - Add test timeouts to vitest config (60s tests, 30s hooks) - Update tool handlers to use appropriate timeouts - Prevent multiple promise resolutions with isResolved flag - Enhanced error messages for timeout scenarios
This commit is contained in:
parent
2b52cea82a
commit
b80cceb541
4 changed files with 121 additions and 6 deletions
|
|
@ -56,7 +56,7 @@ export async function imageToolHandler(
|
||||||
|
|
||||||
const args = buildSwiftCliArgs(input, effectivePath, swiftFormat, logger);
|
const args = buildSwiftCliArgs(input, effectivePath, swiftFormat, logger);
|
||||||
|
|
||||||
const swiftResponse = await executeSwiftCli(args, logger);
|
const swiftResponse = await executeSwiftCli(args, logger, { timeout: 30000 });
|
||||||
|
|
||||||
if (!swiftResponse.success) {
|
if (!swiftResponse.success) {
|
||||||
logger.error(
|
logger.error(
|
||||||
|
|
|
||||||
|
|
@ -115,7 +115,7 @@ export async function listToolHandler(
|
||||||
const args = buildSwiftCliArgs(input);
|
const args = buildSwiftCliArgs(input);
|
||||||
|
|
||||||
// Execute Swift CLI
|
// Execute Swift CLI
|
||||||
const swiftResponse = await executeSwiftCli(args, logger);
|
const swiftResponse = await executeSwiftCli(args, logger, { timeout: 15000 });
|
||||||
|
|
||||||
if (!swiftResponse.success) {
|
if (!swiftResponse.success) {
|
||||||
logger.error({ error: swiftResponse.error }, "Swift CLI returned error");
|
logger.error({ error: swiftResponse.error }, "Swift CLI returned error");
|
||||||
|
|
|
||||||
|
|
@ -102,6 +102,7 @@ function mapExitCodeToErrorMessage(
|
||||||
export async function executeSwiftCli(
|
export async function executeSwiftCli(
|
||||||
args: string[],
|
args: string[],
|
||||||
logger: Logger,
|
logger: Logger,
|
||||||
|
options: { timeout?: number } = {},
|
||||||
): Promise<SwiftCliResponse> {
|
): Promise<SwiftCliResponse> {
|
||||||
let cliPath: string;
|
let cliPath: string;
|
||||||
try {
|
try {
|
||||||
|
|
@ -121,13 +122,54 @@ export async function executeSwiftCli(
|
||||||
// Always add --json-output flag
|
// Always add --json-output flag
|
||||||
const fullArgs = [...args, "--json-output"];
|
const fullArgs = [...args, "--json-output"];
|
||||||
|
|
||||||
logger.debug({ command: cliPath, args: fullArgs }, "Executing Swift CLI");
|
// Default timeout of 30 seconds, configurable via options or environment variable
|
||||||
|
const defaultTimeout = parseInt(process.env.PEEKABOO_CLI_TIMEOUT || "30000", 10);
|
||||||
|
const timeoutMs = options.timeout || defaultTimeout;
|
||||||
|
|
||||||
|
logger.debug({ command: cliPath, args: fullArgs, timeoutMs }, "Executing Swift CLI");
|
||||||
|
|
||||||
return new Promise((resolve) => {
|
return new Promise((resolve) => {
|
||||||
const process = spawn(cliPath, fullArgs);
|
const process = spawn(cliPath, fullArgs);
|
||||||
|
|
||||||
let stdout = "";
|
let stdout = "";
|
||||||
let stderr = "";
|
let stderr = "";
|
||||||
|
let isResolved = false;
|
||||||
|
|
||||||
|
// Set up timeout
|
||||||
|
const timeoutId = setTimeout(() => {
|
||||||
|
if (!isResolved) {
|
||||||
|
isResolved = true;
|
||||||
|
logger.error(
|
||||||
|
{ timeoutMs, command: cliPath, args: fullArgs },
|
||||||
|
"Swift CLI execution timed out"
|
||||||
|
);
|
||||||
|
|
||||||
|
// Kill the process
|
||||||
|
process.kill('SIGTERM');
|
||||||
|
|
||||||
|
// Give it a moment to terminate gracefully, then force kill
|
||||||
|
setTimeout(() => {
|
||||||
|
if (!process.killed) {
|
||||||
|
process.kill('SIGKILL');
|
||||||
|
}
|
||||||
|
}, 1000);
|
||||||
|
|
||||||
|
resolve({
|
||||||
|
success: false,
|
||||||
|
error: {
|
||||||
|
message: `Swift CLI execution timed out after ${timeoutMs}ms. This may indicate a permission dialog is waiting for user input, or the process is stuck.`,
|
||||||
|
code: "SWIFT_CLI_TIMEOUT",
|
||||||
|
details: `Command: ${cliPath} ${fullArgs.join(' ')}`,
|
||||||
|
},
|
||||||
|
});
|
||||||
|
}
|
||||||
|
}, timeoutMs);
|
||||||
|
|
||||||
|
const cleanup = () => {
|
||||||
|
if (timeoutId) {
|
||||||
|
clearTimeout(timeoutId);
|
||||||
|
}
|
||||||
|
};
|
||||||
|
|
||||||
process.stdout.on("data", (data: Buffer | string) => {
|
process.stdout.on("data", (data: Buffer | string) => {
|
||||||
stdout += data.toString();
|
stdout += data.toString();
|
||||||
|
|
@ -141,6 +183,13 @@ export async function executeSwiftCli(
|
||||||
});
|
});
|
||||||
|
|
||||||
process.on("close", (exitCode: number | null) => {
|
process.on("close", (exitCode: number | null) => {
|
||||||
|
cleanup();
|
||||||
|
|
||||||
|
if (isResolved) {
|
||||||
|
return; // Already resolved due to timeout
|
||||||
|
}
|
||||||
|
isResolved = true;
|
||||||
|
|
||||||
logger.debug(
|
logger.debug(
|
||||||
{ exitCode, stdout: stdout.slice(0, 200) },
|
{ exitCode, stdout: stdout.slice(0, 200) },
|
||||||
"Swift CLI completed",
|
"Swift CLI completed",
|
||||||
|
|
@ -261,6 +310,13 @@ export async function executeSwiftCli(
|
||||||
});
|
});
|
||||||
|
|
||||||
process.on("error", (error: Error) => {
|
process.on("error", (error: Error) => {
|
||||||
|
cleanup();
|
||||||
|
|
||||||
|
if (isResolved) {
|
||||||
|
return; // Already resolved due to timeout
|
||||||
|
}
|
||||||
|
isResolved = true;
|
||||||
|
|
||||||
logger.error({ error }, "Failed to spawn Swift CLI process");
|
logger.error({ error }, "Failed to spawn Swift CLI process");
|
||||||
resolve({
|
resolve({
|
||||||
success: false,
|
success: false,
|
||||||
|
|
@ -283,14 +339,44 @@ export async function readImageAsBase64(imagePath: string): Promise<string> {
|
||||||
export async function execPeekaboo(
|
export async function execPeekaboo(
|
||||||
args: string[],
|
args: string[],
|
||||||
packageRootDir: string,
|
packageRootDir: string,
|
||||||
options: { expectSuccess?: boolean } = {},
|
options: { expectSuccess?: boolean; timeout?: number } = {},
|
||||||
): Promise<{ success: boolean; data?: string; error?: string }> {
|
): Promise<{ success: boolean; data?: string; error?: string }> {
|
||||||
const cliPath = process.env.PEEKABOO_CLI_PATH || path.resolve(packageRootDir, "peekaboo");
|
const cliPath = process.env.PEEKABOO_CLI_PATH || path.resolve(packageRootDir, "peekaboo");
|
||||||
|
const timeoutMs = options.timeout || 15000; // Default 15 seconds for simple commands
|
||||||
|
|
||||||
return new Promise((resolve) => {
|
return new Promise((resolve) => {
|
||||||
const process = spawn(cliPath, args);
|
const process = spawn(cliPath, args);
|
||||||
let stdout = "";
|
let stdout = "";
|
||||||
let stderr = "";
|
let stderr = "";
|
||||||
|
let isResolved = false;
|
||||||
|
|
||||||
|
// Set up timeout
|
||||||
|
const timeoutId = setTimeout(() => {
|
||||||
|
if (!isResolved) {
|
||||||
|
isResolved = true;
|
||||||
|
|
||||||
|
// Kill the process
|
||||||
|
process.kill('SIGTERM');
|
||||||
|
|
||||||
|
// Give it a moment to terminate gracefully, then force kill
|
||||||
|
setTimeout(() => {
|
||||||
|
if (!process.killed) {
|
||||||
|
process.kill('SIGKILL');
|
||||||
|
}
|
||||||
|
}, 1000);
|
||||||
|
|
||||||
|
resolve({
|
||||||
|
success: false,
|
||||||
|
error: `Command timed out after ${timeoutMs}ms: ${cliPath} ${args.join(' ')}`
|
||||||
|
});
|
||||||
|
}
|
||||||
|
}, timeoutMs);
|
||||||
|
|
||||||
|
const cleanup = () => {
|
||||||
|
if (timeoutId) {
|
||||||
|
clearTimeout(timeoutId);
|
||||||
|
}
|
||||||
|
};
|
||||||
|
|
||||||
process.stdout.on("data", (data) => {
|
process.stdout.on("data", (data) => {
|
||||||
stdout += data.toString();
|
stdout += data.toString();
|
||||||
|
|
@ -301,6 +387,13 @@ export async function execPeekaboo(
|
||||||
});
|
});
|
||||||
|
|
||||||
process.on("close", (code) => {
|
process.on("close", (code) => {
|
||||||
|
cleanup();
|
||||||
|
|
||||||
|
if (isResolved) {
|
||||||
|
return; // Already resolved due to timeout
|
||||||
|
}
|
||||||
|
isResolved = true;
|
||||||
|
|
||||||
const success = code === 0;
|
const success = code === 0;
|
||||||
if (options.expectSuccess !== false && !success) {
|
if (options.expectSuccess !== false && !success) {
|
||||||
resolve({ success: false, error: stderr || stdout });
|
resolve({ success: false, error: stderr || stdout });
|
||||||
|
|
@ -310,6 +403,13 @@ export async function execPeekaboo(
|
||||||
});
|
});
|
||||||
|
|
||||||
process.on("error", (err) => {
|
process.on("error", (err) => {
|
||||||
|
cleanup();
|
||||||
|
|
||||||
|
if (isResolved) {
|
||||||
|
return; // Already resolved due to timeout
|
||||||
|
}
|
||||||
|
isResolved = true;
|
||||||
|
|
||||||
resolve({ success: false, error: err.message });
|
resolve({ success: false, error: err.message });
|
||||||
});
|
});
|
||||||
});
|
});
|
||||||
|
|
|
||||||
|
|
@ -7,9 +7,24 @@ export default defineConfig({
|
||||||
include: [
|
include: [
|
||||||
"**/tests/unit/**/*.test.ts",
|
"**/tests/unit/**/*.test.ts",
|
||||||
"**/tests/integration/**/*.test.ts",
|
"**/tests/integration/**/*.test.ts",
|
||||||
"peekaboo-cli/tests/e2e/**/*.test.ts",
|
// Only include E2E tests if running on macOS and not in CI
|
||||||
|
...(process.platform === "darwin" && !process.env.CI
|
||||||
|
? ["peekaboo-cli/tests/e2e/**/*.test.ts"]
|
||||||
|
: []
|
||||||
|
),
|
||||||
],
|
],
|
||||||
exclude: ["**/node_modules/**", "**/dist/**"],
|
exclude: [
|
||||||
|
"**/node_modules/**",
|
||||||
|
"**/dist/**",
|
||||||
|
// Exclude E2E tests in CI or non-macOS environments
|
||||||
|
...(process.platform !== "darwin" || process.env.CI
|
||||||
|
? ["peekaboo-cli/tests/e2e/**/*.test.ts"]
|
||||||
|
: []
|
||||||
|
),
|
||||||
|
],
|
||||||
|
// Set reasonable timeouts to prevent hanging
|
||||||
|
testTimeout: 60000, // 60 seconds for individual tests
|
||||||
|
hookTimeout: 30000, // 30 seconds for setup/teardown hooks
|
||||||
coverage: {
|
coverage: {
|
||||||
provider: "v8",
|
provider: "v8",
|
||||||
reporter: ["text", "lcov", "html"],
|
reporter: ["text", "lcov", "html"],
|
||||||
|
|
|
||||||
Loading…
Reference in a new issue