fix(app-boot): keep the fail-loud exit fatal while the terminal is released
Review of the previous commit found two defects in the release path, both reproduced against the implementation: - The timeout guarding a never-settling release was unref'ed. An unhandledRejection listener suppresses Node's default fatal exit, so with nothing else referenced the process reached an empty event loop and exited 0 on the very failure it was reporting. Keep the timer referenced and clear it once the race settles. - The handler uninstalled itself before awaiting the release. A second concurrent rejection then became uncaught and killed the process mid-teardown, stranding exactly the terminal state this restores. Replace the uninstall with a latch: the first rejection is the reported one, and later rejections (teardown's own included) fall through to the pending exit. Add the PTY regression the fake-process tests cannot express: boot the shipped tree over a fixture whose llm-pi-ai providers value is list-shaped, expect exit 1, and assert the captured bytes carry both the diagnostic and ESC[?2004l. Against the pre-fix source the stream ends at ESC[?2004h ESC[>7u ESC[?u ESC[c with no reset and the case fails, so it pins the actual bug. Split the two-shape formatting test into one install per case; a latched handler reports once by design.
This commit is contained in:
@@ -343,10 +343,16 @@ export const FAIL_LOUD_RELEASE_TIMEOUT_MS = 2_000
|
||||
* handler would strand raw mode, bracketed paste, and the keyboard protocol on
|
||||
* the user's shell, and leave an in-flight terminal query's reply to land as
|
||||
* literal text at the next prompt. `release` is the terminal owner's chance to
|
||||
* hand it back; it is awaited under {@link FAIL_LOUD_RELEASE_TIMEOUT_MS}. The
|
||||
* diagnostic is written before the release so the reason survives a disposer
|
||||
* that repaints or clears the screen, and the handler uninstalls itself before
|
||||
* releasing so a rejection from teardown cannot re-enter it.
|
||||
* hand it back; it is awaited under {@link FAIL_LOUD_RELEASE_TIMEOUT_MS}, whose
|
||||
* timer stays referenced so a never-settling disposer cannot let Node reach an
|
||||
* empty event loop and exit 0 instead of failing.
|
||||
*
|
||||
* The diagnostic is written before the release so a hanging or failing disposer
|
||||
* cannot swallow the reason. The handler stays installed while the release runs
|
||||
* — removing it would let a second concurrent rejection become uncaught and kill
|
||||
* the process mid-teardown, stranding exactly the terminal state this restores —
|
||||
* so a latch keeps the first rejection the reported one and lets later
|
||||
* rejections (including the release's own) fall through to the pending exit.
|
||||
* @param binName - the diagnostic prefix on the fatal-failure line.
|
||||
* @param proc - the process slice to register on; tests inject a fake.
|
||||
* @param release - optional teardown awaited before exit, used by a
|
||||
@@ -359,29 +365,33 @@ export function installFailLoud(
|
||||
proc: FailLoudProcess = process,
|
||||
release?: () => Promise<void> | void,
|
||||
): () => void {
|
||||
let exiting = false
|
||||
const handler = (err: unknown): void => {
|
||||
if (assembledActivationRejections.has(err)) return
|
||||
// A release in flight already owns the exit. Swallow later rejections
|
||||
// (teardown's own included) rather than reporting a second failure over the
|
||||
// real one or letting Node kill the process before the terminal is back.
|
||||
if (exiting) return
|
||||
exiting = true
|
||||
proc.stderr.write(`${binName}: fatal load failure: ${err instanceof Error ? err.stack ?? err.message : String(err)}\n`)
|
||||
if (release === undefined) {
|
||||
proc.exit(1)
|
||||
return
|
||||
}
|
||||
// The release runs plugin disposers, which may themselves reject. Without
|
||||
// this the handler would re-enter and report a teardown failure as a second
|
||||
// fatal load failure, hiding the real one.
|
||||
uninstall()
|
||||
void (async () => {
|
||||
let timer: ReturnType<typeof setTimeout> | undefined
|
||||
try {
|
||||
await Promise.race([
|
||||
(async () => release())(),
|
||||
new Promise<void>((resolve) => {
|
||||
setTimeout(resolve, FAIL_LOUD_RELEASE_TIMEOUT_MS).unref()
|
||||
timer = setTimeout(resolve, FAIL_LOUD_RELEASE_TIMEOUT_MS)
|
||||
}),
|
||||
])
|
||||
} catch {
|
||||
// The terminal release failed; the fatal exit below is the outcome that
|
||||
// matters, and no reporter runs after it.
|
||||
}
|
||||
if (timer !== undefined) clearTimeout(timer)
|
||||
proc.exit(1)
|
||||
})()
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user