Add fetch timeouts to Stop hook and health checks
Replace bare fetch() calls with fetchWithTimeout() using Promise.race instead of AbortSignal.timeout() which causes libuv assertion crashes in Bun on Windows. Applies the already-defined HEALTH_CHECK_TIMEOUT_MS (30s/45s) to isWorkerHealthy() and getWorkerVersion(), and HOOK_TIMEOUTS.DEFAULT to the summarize POST. Previously these fetches had no timeout at all, causing the Stop hook to hang for the full hook timeout (120s) when the worker was unreachable. Fixes #963
This commit is contained in:
@@ -7,10 +7,12 @@
|
|||||||
*/
|
*/
|
||||||
|
|
||||||
import type { EventHandler, NormalizedHookInput, HookResult } from '../types.js';
|
import type { EventHandler, NormalizedHookInput, HookResult } from '../types.js';
|
||||||
import { ensureWorkerRunning, getWorkerPort } from '../../shared/worker-utils.js';
|
import { ensureWorkerRunning, getWorkerPort, fetchWithTimeout } from '../../shared/worker-utils.js';
|
||||||
import { logger } from '../../utils/logger.js';
|
import { logger } from '../../utils/logger.js';
|
||||||
import { extractLastMessage } from '../../shared/transcript-parser.js';
|
import { extractLastMessage } from '../../shared/transcript-parser.js';
|
||||||
import { HOOK_EXIT_CODES } from '../../shared/hook-constants.js';
|
import { HOOK_EXIT_CODES, HOOK_TIMEOUTS, getTimeout } from '../../shared/hook-constants.js';
|
||||||
|
|
||||||
|
const SUMMARIZE_TIMEOUT_MS = getTimeout(HOOK_TIMEOUTS.DEFAULT);
|
||||||
|
|
||||||
export const summarizeHandler: EventHandler = {
|
export const summarizeHandler: EventHandler = {
|
||||||
async execute(input: NormalizedHookInput): Promise<HookResult> {
|
async execute(input: NormalizedHookInput): Promise<HookResult> {
|
||||||
@@ -41,15 +43,18 @@ export const summarizeHandler: EventHandler = {
|
|||||||
});
|
});
|
||||||
|
|
||||||
// Send to worker - worker handles privacy check and database operations
|
// Send to worker - worker handles privacy check and database operations
|
||||||
const response = await fetch(`http://127.0.0.1:${port}/api/sessions/summarize`, {
|
const response = await fetchWithTimeout(
|
||||||
method: 'POST',
|
`http://127.0.0.1:${port}/api/sessions/summarize`,
|
||||||
headers: { 'Content-Type': 'application/json' },
|
{
|
||||||
body: JSON.stringify({
|
method: 'POST',
|
||||||
contentSessionId: sessionId,
|
headers: { 'Content-Type': 'application/json' },
|
||||||
last_assistant_message: lastAssistantMessage
|
body: JSON.stringify({
|
||||||
})
|
contentSessionId: sessionId,
|
||||||
// Note: Removed signal to avoid Windows Bun cleanup issue (libuv assertion)
|
last_assistant_message: lastAssistantMessage
|
||||||
});
|
}),
|
||||||
|
},
|
||||||
|
SUMMARIZE_TIMEOUT_MS
|
||||||
|
);
|
||||||
|
|
||||||
if (!response.ok) {
|
if (!response.ok) {
|
||||||
// Return standard response even on failure (matches original behavior)
|
// Return standard response even on failure (matches original behavior)
|
||||||
|
|||||||
@@ -10,6 +10,21 @@ const MARKETPLACE_ROOT = path.join(homedir(), '.claude', 'plugins', 'marketplace
|
|||||||
// Named constants for health checks
|
// Named constants for health checks
|
||||||
const HEALTH_CHECK_TIMEOUT_MS = getTimeout(HOOK_TIMEOUTS.HEALTH_CHECK);
|
const HEALTH_CHECK_TIMEOUT_MS = getTimeout(HOOK_TIMEOUTS.HEALTH_CHECK);
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Fetch with a timeout using Promise.race instead of AbortSignal.
|
||||||
|
* AbortSignal.timeout() causes a libuv assertion crash in Bun on Windows,
|
||||||
|
* so we use a racing setTimeout pattern that avoids signal cleanup entirely.
|
||||||
|
* The orphaned fetch is harmless since the process exits shortly after.
|
||||||
|
*/
|
||||||
|
export function fetchWithTimeout(url: string, init: RequestInit = {}, timeoutMs: number): Promise<Response> {
|
||||||
|
return Promise.race([
|
||||||
|
fetch(url, init),
|
||||||
|
new Promise<never>((_, reject) =>
|
||||||
|
setTimeout(() => reject(new Error(`Request timed out after ${timeoutMs}ms`)), timeoutMs)
|
||||||
|
),
|
||||||
|
]);
|
||||||
|
}
|
||||||
|
|
||||||
// Cache to avoid repeated settings file reads
|
// Cache to avoid repeated settings file reads
|
||||||
let cachedPort: number | null = null;
|
let cachedPort: number | null = null;
|
||||||
let cachedHost: string | null = null;
|
let cachedHost: string | null = null;
|
||||||
@@ -65,8 +80,9 @@ export function clearPortCache(): void {
|
|||||||
*/
|
*/
|
||||||
async function isWorkerHealthy(): Promise<boolean> {
|
async function isWorkerHealthy(): Promise<boolean> {
|
||||||
const port = getWorkerPort();
|
const port = getWorkerPort();
|
||||||
// Note: Removed AbortSignal.timeout to avoid Windows Bun cleanup issue (libuv assertion)
|
const response = await fetchWithTimeout(
|
||||||
const response = await fetch(`http://127.0.0.1:${port}/api/health`);
|
`http://127.0.0.1:${port}/api/health`, {}, HEALTH_CHECK_TIMEOUT_MS
|
||||||
|
);
|
||||||
return response.ok;
|
return response.ok;
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -84,8 +100,9 @@ function getPluginVersion(): string {
|
|||||||
*/
|
*/
|
||||||
async function getWorkerVersion(): Promise<string> {
|
async function getWorkerVersion(): Promise<string> {
|
||||||
const port = getWorkerPort();
|
const port = getWorkerPort();
|
||||||
// Note: Removed AbortSignal.timeout to avoid Windows Bun cleanup issue (libuv assertion)
|
const response = await fetchWithTimeout(
|
||||||
const response = await fetch(`http://127.0.0.1:${port}/api/version`);
|
`http://127.0.0.1:${port}/api/version`, {}, HEALTH_CHECK_TIMEOUT_MS
|
||||||
|
);
|
||||||
if (!response.ok) {
|
if (!response.ok) {
|
||||||
throw new Error(`Failed to get worker version: ${response.status}`);
|
throw new Error(`Failed to get worker version: ${response.status}`);
|
||||||
}
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user