fix(cipc): stop deadlocking wrapper on agent-browser daemon start
runAb() in scripts/_cipc_service.js resolved only on proc.on('close'),
but agent-browser is a daemon-per-session CLI. When it has to START the
session browser daemon (first call after a reboot, after the daemon died,
or after the app restarts), the new daemon inherits the spawned CLI
stdout pipe. The pipe then never reaches EOF, so Node 'close' event
never fires - even though the CLI exited cleanly with code 0. The 30s
timeout in runAb called proc.kill(SIGTERM), but the process had already
exited, so the kill was a no-op and the promise never settled. The
wrapper serialised mutex then held inFlight forever, every later request
queued behind it, and CIPC search silently 500-ed for days with no log
line to show why - which is what made this look like a flaky / restart-
only issue.
Fixes:
- runAb now settles on 'exit' (with a 250ms flush grace for stdio to
drain), escalates the timeout kill to SIGKILL, and guarantees an
unconditional forced settle if neither fires. The promise can no
longer remain pending.
- app/api/cipc/search/route.ts: ensureWrapperService() now probes port
6002 on every search and respawns the wrapper (rate-limited to once
per 30s) if it is down. Hot reloads still no-op once a wrapper is up,
but a dead wrapper now self-heals instead of 500-ing forever.
Verified end to end: killed the browser daemon and the wrapper, started
a fresh wrapper, its first isWarm() spawned a new daemon (the previous
deadlock path) and returned in under 1s. A real enterprise search of
K2018435959 came back in 31.7s on the cold path and 50.9s on the warm
path with full company + director data.
This commit is contained in:
parent
b3eaeb7287
commit
92dcae5122
2 changed files with 54 additions and 18 deletions
|
|
@ -57,11 +57,19 @@ const SCRIPTS_DIR = path.join(process.cwd(), "scripts");
|
|||
// fails the build and 500s the entire app. Route handlers are compiled for
|
||||
// the Node runtime, so the builtins resolve correctly.
|
||||
//
|
||||
// Guarded by a module-level flag + a port probe, so repeated searches
|
||||
// never spawn duplicates (including across dev-server hot reloads).
|
||||
// Probes port 6002 on every search and respawns the wrapper (rate-limited)
|
||||
// if it is down. The probe means the route self-heals: if the wrapper dies
|
||||
// (or the agent-browser session it owns hangs) the next search brings it
|
||||
// back instead of silently timing out at 165 s and 500-ing forever. Hot
|
||||
// reloads in `next dev` are also fine — once a wrapper is up, the probe
|
||||
// returns true and we no-op.
|
||||
// Set CIPC_AUTOSTART=0 to disable.
|
||||
const WRAPPER_PORT = 6002;
|
||||
let autostartAttempted = false;
|
||||
// Cooldown between respawns when the port is closed. A single bad spawn
|
||||
// (e.g. the wrapper crashed on startup) must not trigger a respawn loop
|
||||
// that pegs the CPU.
|
||||
let lastWrapperSpawnMs = 0;
|
||||
const WRAPPER_RESPAWN_COOLDOWN_MS = 30_000;
|
||||
|
||||
function portIsOpen(port: number, ms: number): Promise<boolean> {
|
||||
// Local require rather than the module-level `net` const declared further
|
||||
|
|
@ -81,13 +89,17 @@ function portIsOpen(port: number, ms: number): Promise<boolean> {
|
|||
}
|
||||
|
||||
async function ensureWrapperService(): Promise<void> {
|
||||
if (autostartAttempted) return;
|
||||
autostartAttempted = true;
|
||||
if (process.env.CIPC_AUTOSTART === "0") return;
|
||||
try {
|
||||
// Already running? Leave it alone.
|
||||
if (await portIsOpen(WRAPPER_PORT, 700)) return;
|
||||
|
||||
// Port is closed — respawn, but rate-limited so a startup failure
|
||||
// can't turn into a tight respawn loop.
|
||||
const now = Date.now();
|
||||
if (now - lastWrapperSpawnMs < WRAPPER_RESPAWN_COOLDOWN_MS) return;
|
||||
lastWrapperSpawnMs = now;
|
||||
|
||||
const scriptPath = path.join(SCRIPTS_DIR, "_cipc_service.js");
|
||||
if (!fs.existsSync(scriptPath)) {
|
||||
console.warn("[cipc] _cipc_service.js not found — skipping autostart");
|
||||
|
|
|
|||
|
|
@ -92,30 +92,54 @@ function runAb(args, timeoutMs = 30_000) {
|
|||
// Runs a single agent-browser command by spawning the .exe directly
|
||||
// (no cmd.exe wrapper), which avoids quoting surprises on Windows.
|
||||
// Returns trimmed stdout; throws on timeout or spawn error.
|
||||
//
|
||||
// Why we settle on 'exit' rather than 'close': agent-browser is a
|
||||
// daemon-per-session CLI. When it has to START the session's browser
|
||||
// daemon (first call after a reboot, or after the daemon died), the new
|
||||
// daemon inherits the spawned CLI's stdout pipe. The pipe then never
|
||||
// reaches EOF, so Node's 'close' event never fires — even though the CLI
|
||||
// exited cleanly with code 0. Listening for 'close' used to permanently
|
||||
// wedge this mutex, so every later request queued forever and CIPC search
|
||||
// 500-ed for days with no log line to explain it. We now settle on 'exit'
|
||||
// after a short flush grace for stdio to drain, escalate the timeout kill
|
||||
// to SIGKILL, and guarantee an unconditional forced settle if neither fires
|
||||
// — so the promise cannot remain pending.
|
||||
return new Promise((resolve, reject) => {
|
||||
let stdout = '', stderr = '', killed = false;
|
||||
let stdout = '', stderr = '', settled = false, killEscalate = null;
|
||||
let proc;
|
||||
try {
|
||||
proc = spawn(AGENT_BROWSER_EXE, ['--session', SESSION, ...args], {
|
||||
windowsHide: true,
|
||||
});
|
||||
} catch (e) { return reject(e); }
|
||||
const finish = (fn, val) => {
|
||||
if (settled) return;
|
||||
settled = true;
|
||||
clearTimeout(timer);
|
||||
if (killEscalate) clearTimeout(killEscalate);
|
||||
fn(val);
|
||||
};
|
||||
const timer = setTimeout(() => {
|
||||
killed = true;
|
||||
try { proc.kill('SIGTERM'); } catch {}
|
||||
// SIGKILL maps to TerminateProcess on Windows and is a no-op if the
|
||||
// process has already exited, so we also schedule an unconditional
|
||||
// forced settle shortly after.
|
||||
try { proc.kill('SIGKILL'); } catch {}
|
||||
killEscalate = setTimeout(
|
||||
() => finish(reject, new Error(`agent-browser timeout (${timeoutMs}ms)`)),
|
||||
2000
|
||||
);
|
||||
}, timeoutMs);
|
||||
proc.stdout.on('data', (d) => (stdout += d));
|
||||
proc.stderr.on('data', (d) => (stderr += d));
|
||||
proc.on('error', (e) => {
|
||||
clearTimeout(timer);
|
||||
reject(e);
|
||||
});
|
||||
proc.on('close', (code) => {
|
||||
clearTimeout(timer);
|
||||
if (killed) return reject(new Error('agent-browser timeout'));
|
||||
// agent-browser returns 0 even on bad JSON-eval results — the
|
||||
// actual error message is encoded in the JSON-stringified stdout.
|
||||
resolve({ stdout: stdout.trim(), stderr: stderr.trim(), code });
|
||||
proc.on('error', (e) => finish(reject, e));
|
||||
proc.on('exit', (code) => {
|
||||
// 'close' can never fire reliably here (see header); give stdio a
|
||||
// brief moment to flush, then settle. agent-browser returns 0 even
|
||||
// on bad JSON-eval results — the actual error is in the stdout.
|
||||
setTimeout(
|
||||
() => finish(resolve, { stdout: stdout.trim(), stderr: stderr.trim(), code }),
|
||||
250
|
||||
);
|
||||
});
|
||||
});
|
||||
}
|
||||
|
|
|
|||
Loading…
Reference in a new issue