From 4716d315b737896c865ab23cbd041dfb9e3aa0c7 Mon Sep 17 00:00:00 2001 From: fireblue Date: Mon, 9 Mar 2026 20:55:50 +0800 Subject: [PATCH] feat: improve spawn error handling and reporting across full stack (#249) * feat: improve spawn error handling and reporting across full stack - Return error result instead of throwing in apiMachine spawn handler - Add lastSpawnError field to RunnerState for persistent error tracking - Add error awaiter system for early process exit/error detection before webhook - Build detailed webhook failure messages with exit code, signal, and stderr tail - Report spawn outcomes to hub via runner state updates - Handle more spawn result types in rpcGateway with better error messages - Display runner last spawn error in web UI (NewSession & SpawnSession) - Extract shared formatRunnerSpawnError utility to avoid duplication Co-Authored-By: Claude Opus 4.6 * fix(cli): narrow spawnResult type check to fix TS2339 error Use `type === 'error'` instead of `type !== 'success'` to properly narrow the discriminated union, allowing TypeScript to infer errorMessage. Co-Authored-By: Claude Opus 4.6 --------- Co-authored-by: Claude Opus 4.6 --- cli/src/api/apiMachine.ts | 2 +- cli/src/api/types.ts | 9 +- cli/src/runner/run.ts | 152 +++++++++++++++++++++++- hub/src/sync/rpcGateway.ts | 20 +++- web/src/components/NewSession/index.tsx | 15 +++ web/src/components/SpawnSession.tsx | 11 ++ web/src/types/api.ts | 17 +++ web/src/utils/formatRunnerSpawnError.ts | 15 +++ 8 files changed, 234 insertions(+), 7 deletions(-) create mode 100644 web/src/utils/formatRunnerSpawnError.ts diff --git a/cli/src/api/apiMachine.ts b/cli/src/api/apiMachine.ts index e38d0889..8c6ea7a0 100644 --- a/cli/src/api/apiMachine.ts +++ b/cli/src/api/apiMachine.ts @@ -127,7 +127,7 @@ export class ApiMachineClient { case 'requestToApproveDirectoryCreation': return { type: 'requestToApproveDirectoryCreation', directory: result.directory } case 'error': - throw new Error(result.errorMessage) + return { type: 'error', errorMessage: result.errorMessage } } }) diff --git a/cli/src/api/types.ts b/cli/src/api/types.ts index 3ca1b222..f340a453 100644 --- a/cli/src/api/types.ts +++ b/cli/src/api/types.ts @@ -43,7 +43,14 @@ export const RunnerStateSchema = z.object({ httpPort: z.number().optional(), startedAt: z.number().optional(), shutdownRequestedAt: z.number().optional(), - shutdownSource: z.union([z.enum(['mobile-app', 'cli', 'os-signal', 'unknown']), z.string()]).optional() + shutdownSource: z.union([z.enum(['mobile-app', 'cli', 'os-signal', 'unknown']), z.string()]).optional(), + lastSpawnError: z.object({ + message: z.string(), + pid: z.number().optional(), + exitCode: z.number().nullable().optional(), + signal: z.string().nullable().optional(), + at: z.number() + }).nullable().optional() }) export type RunnerState = z.infer diff --git a/cli/src/runner/run.ts b/cli/src/runner/run.ts index 3ae5ac44..058d2743 100644 --- a/cli/src/runner/run.ts +++ b/cli/src/runner/run.ts @@ -127,6 +127,20 @@ export async function startRunner(): Promise { // Session spawning awaiter system const pidToAwaiter = new Map void>(); + const pidToErrorAwaiter = new Map void>(); + type SpawnFailureDetails = { + message: string + pid?: number + exitCode?: number | null + signal?: NodeJS.Signals | null + }; + let reportSpawnOutcomeToHub: ((outcome: { type: 'success' } | { type: 'error'; details: SpawnFailureDetails }) => void) | null = null; + const formatSpawnError = (error: unknown): string => { + if (error instanceof Error) { + return error.message; + } + return String(error); + }; // Helper functions const getCurrentChildren = () => Array.from(pidToTrackedSession.values()); @@ -157,6 +171,7 @@ export async function startRunner(): Promise { const awaiter = pidToAwaiter.get(pid); if (awaiter) { pidToAwaiter.delete(pid); + pidToErrorAwaiter.delete(pid); awaiter(existingSession); logger.debug(`[RUNNER RUN] Resolved session awaiter for PID ${pid}`); } @@ -383,17 +398,66 @@ export async function startRunner(): Promise { stderrTail = appendTail(stderrTail, data); }); + let spawnErrorBeforePidCheck: Error | null = null; + const captureSpawnErrorBeforePidCheck = (error: Error) => { + spawnErrorBeforePidCheck = error; + }; + happyProcess.once('error', captureSpawnErrorBeforePidCheck); + if (!happyProcess.pid) { - logger.debug('[RUNNER RUN] Failed to spawn process - no PID returned'); + // Allow the async 'error' event to fire before we read it + await new Promise((resolve) => setImmediate(resolve)); + const details = [`cwd=${spawnDirectory}`]; + if (spawnErrorBeforePidCheck) { + details.push(formatSpawnError(spawnErrorBeforePidCheck)); + } + const errorMessage = `Failed to spawn HAPI process - no PID returned (${details.join('; ')})`; + logger.debug('[RUNNER RUN] Failed to spawn process - no PID returned', spawnErrorBeforePidCheck ?? null); + reportSpawnOutcomeToHub?.({ + type: 'error', + details: { + message: errorMessage + } + }); await maybeCleanupWorktree('no-pid'); return { type: 'error', - errorMessage: 'Failed to spawn HAPI process - no PID returned' + errorMessage }; } + happyProcess.removeListener('error', captureSpawnErrorBeforePidCheck); const pid = happyProcess.pid; logger.debug(`[RUNNER RUN] Spawned process with PID ${pid}`); + let observedExitCode: number | null = null; + let observedExitSignal: NodeJS.Signals | null = null; + const buildWebhookFailureMessage = (reason: 'timeout' | 'exit-before-webhook' | 'process-error-before-webhook'): string => { + let message = ''; + if (reason === 'exit-before-webhook') { + message = `Session process exited before webhook for PID ${pid}`; + } else if (reason === 'process-error-before-webhook') { + message = `Session process error before webhook for PID ${pid}`; + } else { + message = `Session webhook timeout for PID ${pid}`; + } + + if (observedExitCode !== null || observedExitSignal) { + if (observedExitCode !== null) { + message += ` (exit code ${observedExitCode})`; + } else { + message += ` (signal ${observedExitSignal})`; + } + } + + const trimmedTail = stderrTail.trim(); + if (trimmedTail) { + const compactTail = trimmedTail.replace(/\s+/g, ' '); + const tailForMessage = compactTail.length > 800 ? compactTail.slice(-800) : compactTail; + message += `. stderr: ${tailForMessage}`; + } + + return message; + }; const trackedSession: TrackedSession = { startedBy: 'runner', @@ -406,15 +470,29 @@ export async function startRunner(): Promise { pidToTrackedSession.set(pid, trackedSession); happyProcess.on('exit', (code, signal) => { + observedExitCode = typeof code === 'number' ? code : null; + observedExitSignal = signal ?? null; logger.debug(`[RUNNER RUN] Child PID ${pid} exited with code ${code}, signal ${signal}`); if (code !== 0 || signal) { logStderrTail(); } + const errorAwaiter = pidToErrorAwaiter.get(pid); + if (errorAwaiter) { + pidToErrorAwaiter.delete(pid); + pidToAwaiter.delete(pid); + errorAwaiter(buildWebhookFailureMessage('exit-before-webhook')); + } onChildExited(pid); }); happyProcess.on('error', (error) => { logger.debug(`[RUNNER RUN] Child process error:`, error); + const errorAwaiter = pidToErrorAwaiter.get(pid); + if (errorAwaiter) { + pidToErrorAwaiter.delete(pid); + pidToAwaiter.delete(pid); + errorAwaiter(buildWebhookFailureMessage('process-error-before-webhook')); + } onChildExited(pid); }); @@ -425,11 +503,12 @@ export async function startRunner(): Promise { // Set timeout for webhook const timeout = setTimeout(() => { pidToAwaiter.delete(pid); + pidToErrorAwaiter.delete(pid); logger.debug(`[RUNNER RUN] Session webhook timeout for PID ${pid}`); logStderrTail(); resolve({ type: 'error', - errorMessage: `Session webhook timeout for PID ${pid}` + errorMessage: buildWebhookFailureMessage('timeout') }); // 15 second timeout - I have seen timeouts on 10 seconds // even though session was still created successfully in ~2 more seconds @@ -438,21 +517,46 @@ export async function startRunner(): Promise { // Register awaiter pidToAwaiter.set(pid, (completedSession) => { clearTimeout(timeout); + pidToErrorAwaiter.delete(pid); logger.debug(`[RUNNER RUN] Session ${completedSession.happySessionId} fully spawned with webhook`); resolve({ type: 'success', sessionId: completedSession.happySessionId! }); }); + pidToErrorAwaiter.set(pid, (errorMessage) => { + clearTimeout(timeout); + resolve({ + type: 'error', + errorMessage + }); + }); }); - if (spawnResult.type !== 'success') { + if (spawnResult.type === 'error') { + reportSpawnOutcomeToHub?.({ + type: 'error', + details: { + message: spawnResult.errorMessage, + pid, + exitCode: observedExitCode, + signal: observedExitSignal + } + }); await maybeCleanupWorktree('spawn-error'); + } else { + reportSpawnOutcomeToHub?.({ type: 'success' }); } return spawnResult; } catch (error) { const errorMessage = error instanceof Error ? error.message : String(error); logger.debug('[RUNNER RUN] Failed to spawn session:', error); await maybeCleanupWorktree('exception'); + reportSpawnOutcomeToHub?.({ + type: 'error', + details: { + message: `Failed to spawn session: ${errorMessage}` + } + }); return { type: 'error', errorMessage: `Failed to spawn session: ${errorMessage}` @@ -500,6 +604,8 @@ export async function startRunner(): Promise { const onChildExited = (pid: number) => { logger.debug(`[RUNNER RUN] Removing exited process PID ${pid} from tracking`); pidToTrackedSession.delete(pid); + pidToAwaiter.delete(pid); + pidToErrorAwaiter.delete(pid); }; // Start control server @@ -569,6 +675,44 @@ export async function startRunner(): Promise { // Connect to server apiMachine.connect(); + reportSpawnOutcomeToHub = (outcome) => { + void apiMachine.updateRunnerState((state: RunnerState | null) => { + const baseState: RunnerState = state + ? { ...state } + : { status: 'running' }; + + if (typeof baseState.pid !== 'number') { + baseState.pid = process.pid; + } + if (typeof baseState.httpPort !== 'number') { + baseState.httpPort = controlPort; + } + if (typeof baseState.startedAt !== 'number') { + baseState.startedAt = Date.now(); + } + + if (outcome.type === 'success') { + return { + ...baseState, + lastSpawnError: null + }; + } + + return { + ...baseState, + lastSpawnError: { + message: outcome.details.message, + pid: outcome.details.pid, + exitCode: outcome.details.exitCode ?? null, + signal: outcome.details.signal ?? null, + at: Date.now() + } + }; + }).catch((error) => { + logger.debug('[RUNNER RUN] Failed to update runner state with spawn outcome', error); + }); + }; + // Every 60 seconds: // 1. Prune stale sessions // 2. Check if runner needs update diff --git a/hub/src/sync/rpcGateway.ts b/hub/src/sync/rpcGateway.ts index 3aca3e20..b36e8391 100644 --- a/hub/src/sync/rpcGateway.ts +++ b/hub/src/sync/rpcGateway.ts @@ -127,8 +127,26 @@ export class RpcGateway { if (obj.type === 'error' && typeof obj.errorMessage === 'string') { return { type: 'error', message: obj.errorMessage } } + if (obj.type === 'requestToApproveDirectoryCreation' && typeof obj.directory === 'string') { + return { type: 'error', message: `Directory creation requires approval: ${obj.directory}` } + } + if (typeof obj.error === 'string') { + return { type: 'error', message: obj.error } + } + if (obj.type !== 'success' && typeof obj.message === 'string') { + return { type: 'error', message: obj.message } + } } - return { type: 'error', message: 'Unexpected spawn result' } + const details = typeof result === 'string' + ? result + : (() => { + try { + return JSON.stringify(result) + } catch { + return String(result) + } + })() + return { type: 'error', message: `Unexpected spawn result: ${details}` } } catch (error) { return { type: 'error', message: error instanceof Error ? error.message : String(error) } } diff --git a/web/src/components/NewSession/index.tsx b/web/src/components/NewSession/index.tsx index 08fd8156..60efd325 100644 --- a/web/src/components/NewSession/index.tsx +++ b/web/src/components/NewSession/index.tsx @@ -21,6 +21,7 @@ import { } from './preferences' import { SessionTypeSelector } from './SessionTypeSelector' import { YoloToggle } from './YoloToggle' +import { formatRunnerSpawnError } from '../../utils/formatRunnerSpawnError' export function NewSession(props: { api: ApiClient @@ -82,6 +83,15 @@ export function NewSession(props: { } }, [props.machines, machineId, getLastUsedMachineId, getRecentPaths]) + const selectedMachine = useMemo( + () => (machineId ? props.machines.find((machine) => machine.id === machineId) ?? null : null), + [machineId, props.machines] + ) + const runnerSpawnError = useMemo( + () => formatRunnerSpawnError(selectedMachine), + [selectedMachine] + ) + const recentPaths = useMemo( () => getRecentPaths(machineId), [getRecentPaths, machineId] @@ -247,6 +257,11 @@ export function NewSession(props: { isDisabled={isFormDisabled} onChange={handleMachineChange} /> + {runnerSpawnError ? ( +
+ Runner last spawn error: {runnerSpawnError} +
+ ) : null} getMachineTitle(props.machine), [props.machine]) + const runnerSpawnError = useMemo( + () => formatRunnerSpawnError(props.machine), + [props.machine?.runnerState?.lastSpawnError] + ) async function spawn() { const trimmed = directory.trim() @@ -144,6 +149,12 @@ export function SpawnSession(props: { + {runnerSpawnError ? ( +
+ Runner last spawn error: {runnerSpawnError} +
+ ) : null} + {(error ?? spawnError) ? (
{error ?? spawnError} diff --git a/web/src/types/api.ts b/web/src/types/api.ts index 57d720ae..14a7b6c6 100644 --- a/web/src/types/api.ts +++ b/web/src/types/api.ts @@ -42,6 +42,22 @@ export type DecryptedMessage = ProtocolDecryptedMessage & { originalText?: string } +export type RunnerState = { + status?: string + pid?: number + httpPort?: number + startedAt?: number + shutdownRequestedAt?: number + shutdownSource?: string + lastSpawnError?: { + message: string + pid?: number + exitCode?: number | null + signal?: string | null + at: number + } | null +} + export type Machine = { id: string active: boolean @@ -51,6 +67,7 @@ export type Machine = { happyCliVersion: string displayName?: string } | null + runnerState?: RunnerState | null } export type AuthResponse = { diff --git a/web/src/utils/formatRunnerSpawnError.ts b/web/src/utils/formatRunnerSpawnError.ts new file mode 100644 index 00000000..a0e0c40f --- /dev/null +++ b/web/src/utils/formatRunnerSpawnError.ts @@ -0,0 +1,15 @@ +import type { Machine } from '../types/api' + +export function formatRunnerSpawnError(machine: Machine | null): string | null { + const lastSpawnError = machine?.runnerState?.lastSpawnError + if (!lastSpawnError?.message) { + return null + } + + const at = typeof lastSpawnError.at === 'number' + ? new Date(lastSpawnError.at).toLocaleString() + : null + return at + ? `${lastSpawnError.message} (${at})` + : lastSpawnError.message +}