fix(background-agent): keep cleanup error listener active
Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent) Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>
This commit is contained in:
@@ -435,6 +435,34 @@ describe("#given process cleanup registration", () => {
|
||||
}
|
||||
})
|
||||
|
||||
test("#given repeated uncaughtException events #when manager is registered #then listener stays installed and host is not forced to exit", async () => {
|
||||
const uncaughtExceptionListenersBefore = process.listeners("uncaughtException")
|
||||
const exitSpy = spyOn(process, "exit").mockImplementation((() => undefined) as never)
|
||||
const shutdown = mock(() => {})
|
||||
const manager = { shutdown }
|
||||
registeredManagers.push(manager)
|
||||
__enableScheduledForcedExitForTesting()
|
||||
|
||||
try {
|
||||
registerManagerForCleanup(manager)
|
||||
|
||||
process.emit("uncaughtException", new Error("first transient MCP failure"))
|
||||
process.emit("uncaughtException", new Error("second transient MCP failure"))
|
||||
await flushMicrotasks()
|
||||
|
||||
expect(process.listeners("uncaughtException")).toHaveLength(
|
||||
uncaughtExceptionListenersBefore.length + 1,
|
||||
)
|
||||
expect(shutdown).not.toHaveBeenCalled()
|
||||
expect(exitSpy).not.toHaveBeenCalled()
|
||||
expect(process.exitCode).toBe(0)
|
||||
} finally {
|
||||
exitSpy.mockRestore()
|
||||
__disableScheduledForcedExitForTesting()
|
||||
process.exitCode = 0
|
||||
}
|
||||
})
|
||||
|
||||
test("#given a manager registered AND process emits 'exit' #then cleanup still runs (signal path remains the real shutdown gate)", () => {
|
||||
const exitListenersBefore = process.listeners("exit")
|
||||
const shutdown = mock(() => {})
|
||||
|
||||
@@ -115,16 +115,22 @@ function registerErrorEvent(
|
||||
// regardless of cause, so cleanup is not skipped when the host genuinely
|
||||
// dies.
|
||||
//
|
||||
// We still detach the listener before logging so a re-emit from inside
|
||||
// `log()` (e.g. EPIPE while writing to a broken pipe during shutdown)
|
||||
// cannot recurse and produce the 100+ GB log explosion that #3856-era
|
||||
// regressions caused.
|
||||
// Keep the listener installed after logging. Desktop sidecars can emit more
|
||||
// than one transient error during MCP startup or provider reconnects; if we
|
||||
// detach after the first event, the second uncaught exception falls through
|
||||
// to Node's default process termination path and reproduces the exit-code-1
|
||||
// crash from #4128. A local re-entry guard still prevents `log()` failures
|
||||
// (for example EPIPE while writing during shutdown) from recursing into the
|
||||
// 100+ GB log explosion that #3856-era regressions caused.
|
||||
let logging = false
|
||||
const listener = (error: unknown) => {
|
||||
process.off(signal, listener)
|
||||
if (logging) return
|
||||
logging = true
|
||||
log(
|
||||
`[background-agent] ${signal} observed; keeping host alive and skipping cleanup (signal handlers run on real shutdown)`,
|
||||
describeProcessCleanupError(error),
|
||||
)
|
||||
logging = false
|
||||
}
|
||||
process.on(signal, listener)
|
||||
return listener
|
||||
|
||||
Reference in New Issue
Block a user