Merge pull request #3947 from MoerAI/fix/process-cleanup-opt-out-env
fix(background-agent): add OMO_DISABLE_PROCESS_CLEANUP env opt-out for global handlers (fixes #3856)
This commit is contained in:
@@ -235,6 +235,103 @@ describe("#given process cleanup registration", () => {
|
|||||||
})
|
})
|
||||||
})
|
})
|
||||||
|
|
||||||
|
describe("#given OMO_DISABLE_PROCESS_CLEANUP env var", () => {
|
||||||
|
let originalEnvValue: string | undefined
|
||||||
|
|
||||||
|
beforeEach(() => {
|
||||||
|
originalEnvValue = process.env.OMO_DISABLE_PROCESS_CLEANUP
|
||||||
|
})
|
||||||
|
|
||||||
|
afterEach(() => {
|
||||||
|
if (originalEnvValue === undefined) {
|
||||||
|
delete process.env.OMO_DISABLE_PROCESS_CLEANUP
|
||||||
|
} else {
|
||||||
|
process.env.OMO_DISABLE_PROCESS_CLEANUP = originalEnvValue
|
||||||
|
}
|
||||||
|
})
|
||||||
|
|
||||||
|
test("#given env var is set to 1 #when registerManagerForCleanup runs #then uncaughtException handler is NOT registered", () => {
|
||||||
|
const uncaughtExceptionListenersBefore = process.listeners("uncaughtException")
|
||||||
|
const unhandledRejectionListenersBefore = process.listeners("unhandledRejection")
|
||||||
|
process.env.OMO_DISABLE_PROCESS_CLEANUP = "1"
|
||||||
|
const manager = { shutdown: mock(() => {}) }
|
||||||
|
registeredManagers.push(manager)
|
||||||
|
|
||||||
|
registerManagerForCleanup(manager)
|
||||||
|
|
||||||
|
expect(process.listeners("uncaughtException")).toHaveLength(uncaughtExceptionListenersBefore.length)
|
||||||
|
expect(process.listeners("unhandledRejection")).toHaveLength(unhandledRejectionListenersBefore.length)
|
||||||
|
})
|
||||||
|
|
||||||
|
test("#given env var is set to true #when registerManagerForCleanup runs #then handlers are NOT registered", () => {
|
||||||
|
const uncaughtExceptionListenersBefore = process.listeners("uncaughtException")
|
||||||
|
process.env.OMO_DISABLE_PROCESS_CLEANUP = "true"
|
||||||
|
const manager = { shutdown: mock(() => {}) }
|
||||||
|
registeredManagers.push(manager)
|
||||||
|
|
||||||
|
registerManagerForCleanup(manager)
|
||||||
|
|
||||||
|
expect(process.listeners("uncaughtException")).toHaveLength(uncaughtExceptionListenersBefore.length)
|
||||||
|
})
|
||||||
|
|
||||||
|
test("#given env var is set to 0 #when registerManagerForCleanup runs #then handlers ARE registered", () => {
|
||||||
|
const uncaughtExceptionListenersBefore = process.listeners("uncaughtException")
|
||||||
|
process.env.OMO_DISABLE_PROCESS_CLEANUP = "0"
|
||||||
|
const manager = { shutdown: mock(() => {}) }
|
||||||
|
registeredManagers.push(manager)
|
||||||
|
|
||||||
|
registerManagerForCleanup(manager)
|
||||||
|
|
||||||
|
expect(process.listeners("uncaughtException")).toHaveLength(uncaughtExceptionListenersBefore.length + 1)
|
||||||
|
})
|
||||||
|
|
||||||
|
test("#given env var is unset #when registerManagerForCleanup runs #then handlers ARE registered", () => {
|
||||||
|
const uncaughtExceptionListenersBefore = process.listeners("uncaughtException")
|
||||||
|
delete process.env.OMO_DISABLE_PROCESS_CLEANUP
|
||||||
|
const manager = { shutdown: mock(() => {}) }
|
||||||
|
registeredManagers.push(manager)
|
||||||
|
|
||||||
|
registerManagerForCleanup(manager)
|
||||||
|
|
||||||
|
expect(process.listeners("uncaughtException")).toHaveLength(uncaughtExceptionListenersBefore.length + 1)
|
||||||
|
})
|
||||||
|
|
||||||
|
test("#given env var is set #when signals fire #then SIGINT/SIGTERM/beforeExit/exit handlers still run cleanup", () => {
|
||||||
|
const exitListenersBefore = process.listeners("exit")
|
||||||
|
process.env.OMO_DISABLE_PROCESS_CLEANUP = "yes"
|
||||||
|
const shutdown = mock(() => {})
|
||||||
|
const manager = { shutdown }
|
||||||
|
registeredManagers.push(manager)
|
||||||
|
|
||||||
|
registerManagerForCleanup(manager)
|
||||||
|
const exitListener = getNewListener("exit", exitListenersBefore)
|
||||||
|
exitListener()
|
||||||
|
|
||||||
|
expect(shutdown).toHaveBeenCalledTimes(1)
|
||||||
|
})
|
||||||
|
|
||||||
|
test("#given env var is set AND process emits uncaughtException #when event fires #then manager shutdown is NOT invoked by our handler", async () => {
|
||||||
|
process.env.OMO_DISABLE_PROCESS_CLEANUP = "1"
|
||||||
|
const exitSpy = spyOn(process, "exit").mockImplementation((() => undefined) as never)
|
||||||
|
const shutdown = mock(() => {})
|
||||||
|
const manager = { shutdown }
|
||||||
|
registeredManagers.push(manager)
|
||||||
|
|
||||||
|
try {
|
||||||
|
registerManagerForCleanup(manager)
|
||||||
|
|
||||||
|
// Other listeners on uncaughtException may exist (e.g. node default).
|
||||||
|
// We assert that OUR handler did not run cleanup.
|
||||||
|
process.emit("uncaughtException", new Error("boom"))
|
||||||
|
await flushMicrotasks()
|
||||||
|
|
||||||
|
expect(shutdown).not.toHaveBeenCalled()
|
||||||
|
} finally {
|
||||||
|
exitSpy.mockRestore()
|
||||||
|
}
|
||||||
|
})
|
||||||
|
})
|
||||||
|
|
||||||
describe("#given uncaught exception and rejection cleanup", () => {
|
describe("#given uncaught exception and rejection cleanup", () => {
|
||||||
test("#given manager registered AND process emits uncaughtException #when event fires #then manager shuts down before process exits", async () => {
|
test("#given manager registered AND process emits uncaughtException #when event fires #then manager shuts down before process exits", async () => {
|
||||||
const exitSpy = spyOn(process, "exit").mockImplementation((() => undefined) as never)
|
const exitSpy = spyOn(process, "exit").mockImplementation((() => undefined) as never)
|
||||||
|
|||||||
@@ -3,6 +3,26 @@ import { log } from "../../shared"
|
|||||||
type ProcessCleanupSignal = NodeJS.Signals | "beforeExit" | "exit"
|
type ProcessCleanupSignal = NodeJS.Signals | "beforeExit" | "exit"
|
||||||
type ProcessCleanupErrorEvent = "uncaughtException" | "unhandledRejection"
|
type ProcessCleanupErrorEvent = "uncaughtException" | "unhandledRejection"
|
||||||
|
|
||||||
|
/**
|
||||||
|
* When set to a truthy value (1/true/yes/on), suppresses the global
|
||||||
|
* uncaughtException / unhandledRejection handlers that force-exit the host
|
||||||
|
* process. Use this when the plugin is installed but background-agent tasks
|
||||||
|
* are not actively in use, to avoid OpenCode dying on transient streaming
|
||||||
|
* errors propagated as unhandled rejections (see issue #3856).
|
||||||
|
*
|
||||||
|
* Signal handlers (SIGINT/SIGTERM/SIGBREAK/beforeExit/exit) remain registered
|
||||||
|
* because they are needed for graceful shutdown of any in-flight cleanup
|
||||||
|
* targets that were registered before the user noticed the issue.
|
||||||
|
*/
|
||||||
|
const PROCESS_CLEANUP_DISABLE_ENV = "OMO_DISABLE_PROCESS_CLEANUP"
|
||||||
|
const TRUTHY_ENV_VALUES = new Set(["1", "true", "yes", "on"])
|
||||||
|
|
||||||
|
function isProcessCleanupErrorHandlersDisabled(): boolean {
|
||||||
|
const raw = process.env[PROCESS_CLEANUP_DISABLE_ENV]
|
||||||
|
if (!raw) return false
|
||||||
|
return TRUTHY_ENV_VALUES.has(raw.trim().toLowerCase())
|
||||||
|
}
|
||||||
|
|
||||||
/** @internal test-only seam: prevents process.exitCode from contaminating bun test runner */
|
/** @internal test-only seam: prevents process.exitCode from contaminating bun test runner */
|
||||||
let _scheduleForcedExitEnabled = true
|
let _scheduleForcedExitEnabled = true
|
||||||
|
|
||||||
@@ -116,6 +136,15 @@ export function registerManagerForCleanup(manager: CleanupTarget): void {
|
|||||||
}
|
}
|
||||||
registerSignal("beforeExit", false)
|
registerSignal("beforeExit", false)
|
||||||
registerSignal("exit", false)
|
registerSignal("exit", false)
|
||||||
|
|
||||||
|
if (isProcessCleanupErrorHandlersDisabled()) {
|
||||||
|
log(
|
||||||
|
`[background-agent] ${PROCESS_CLEANUP_DISABLE_ENV} is set; skipping global uncaughtException/unhandledRejection handler registration. `
|
||||||
|
+ "Signal handlers (SIGINT/SIGTERM/beforeExit/exit) remain active.",
|
||||||
|
)
|
||||||
|
return
|
||||||
|
}
|
||||||
|
|
||||||
cleanupErrorHandlers.set("uncaughtException", registerErrorEvent("uncaughtException", cleanupAll))
|
cleanupErrorHandlers.set("uncaughtException", registerErrorEvent("uncaughtException", cleanupAll))
|
||||||
cleanupErrorHandlers.set("unhandledRejection", registerErrorEvent("unhandledRejection", cleanupAll))
|
cleanupErrorHandlers.set("unhandledRejection", registerErrorEvent("unhandledRejection", cleanupAll))
|
||||||
}
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user